Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seem to be unable to pass multiple args to mpv with '-e' opt. #424

Closed
talzahr opened this issue Jan 27, 2022 · 8 comments
Closed

Seem to be unable to pass multiple args to mpv with '-e' opt. #424

talzahr opened this issue Jan 27, 2022 · 8 comments
Assignees
Labels
priority 2: medium Default for bugs type: bug something isn't working
Milestone

Comments

@talzahr
Copy link

talzahr commented Jan 27, 2022

Metadata (please complete the following information)
Version: 1.4.1 (ran a git pull about an hour before posting)
OS: Arch Linux kernel 5.16.2
Shell: Bash
Anime: itai no wa iya nano de... (non-dub)
mpv version: 0.34.1 w/ MPRIS plugin v. 0.6
youtube-dl version: 2021.12.17

Describe the bug
Tried multiple option syntax to pass the three options below to mpv, however the player refuses to load when there's more than 1 argument.

Steps To Reproduce
./ani-cli -e --brightness=-15 --gamma=-15 --contrast=15 (results in usage text)
./ani-cli -e "--brightness=-15 --gamma=-15 --contrast=15" (no errors appear, but player does not load)
./ani-cli -e '--brightness=-15 --gamma=-15 --contrast=15' (same as above)
./ani-cli -e --brightness=-15 -e --gamma=-15 -e --contrast=15 (only 1 arg is passed)

Expected behavior
Pass multiple args to mpv

Screenshots (if applicable)

Additional context
Not a huge thing really as I can just put it in a config or modify $player_arguments directly in ani-cli but it would be nice to have. Thanks, awesome program

@talzahr talzahr added priority 2: medium Default for bugs type: bug something isn't working labels Jan 27, 2022
@71zenith
Copy link
Collaborator

The only way I can get it working is by removing the quotes where it is passes to the player

@pystardust
Copy link
Owner

nohup "$player_fn" "$player_arguments" --referrer="$dpage_link" "$video_url" > /dev/null 2>&1 &

This is what we have currently

nohup "$player_fn" $player_arguments --referrer="$dpage_link" "$video_url" > /dev/null 2>&1 &

we need this instead.

PROS: since we are having variable without quotes, word splitting would occur causing multiple arguments to be split by spaces.
CONS: if we have a argument with space in it example

ani-cli -e '--sub-file="some file.ext"'

This would result in 2 arguments due to word splitting causing an issue

The best way to be implemented would be do parse each argument separately with -e arg1 -e arg2 and handle this in the script

@port19x port19x added this to the v1.6 milestone Jan 27, 2022
@port19x
Copy link
Collaborator

port19x commented Jan 27, 2022

nohup "$player_fn" "$player_arguments" --referrer="$dpage_link" "$video_url" > /dev/null 2>&1 &

This is what we have currently

nohup "$player_fn" $player_arguments --referrer="$dpage_link" "$video_url" > /dev/null 2>&1 &

we need this instead.

PROS: since we are having variable without quotes, word splitting would occur causing multiple arguments to be split by spaces. CONS: if we have a argument with space in it example

ani-cli -e '--sub-file="some file.ext"'

This would result in 2 arguments due to word splitting causing an issue

The best way to be implemented would be do parse each argument separately with -e arg1 -e arg2 and handle this in the script

I agree that a -e arg1 -e arg2 pattern is the most desireable

@talzahr
Copy link
Author

talzahr commented Jan 28, 2022

Yeah Looks like a good solution with the looped case expression appending the player arguments to an array on each loop pass.

@71zenith
Copy link
Collaborator

Arrays are sadly undefined in POSIX, so can't use those

pystardust added a commit that referenced this issue Jan 28, 2022
This is a partial fix, the issue of variable with spaces remains

Current usage
```
ani-cli -e 'arg1 arg2'
```
@pystardust
Copy link
Owner

This is a partial fix, the issue of variable with spaces remains

Current usage

ani-cli -e 'arg1 arg2'

Shell check complains, but that is the intended behavior we want

@port19x
Copy link
Collaborator

port19x commented Jan 28, 2022

Intentionally reintroduced the bug. Fix it properly before introducing linter issues on master

@71zenith
Copy link
Collaborator

Fixed it in pr #445

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority 2: medium Default for bugs type: bug something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants