bash-completion: don't append '=' to help and clr options - #18339
Conversation
|
fix after: fix before: |
|
|
Thanks for taking a look, and you're right — --help/--h do accept a substring filter argument (e.g. --help=vo), so "takes no argument" in the commit message was inaccurate. I apologise for the poor wording. I'd actually considered that point while writing the patch and settled on the special-casing anyway, but I clearly didn't explain the reasoning well. If I may, here is what led me to keep --help/--h out of the =-appending branch: _mpv_get_args derives value candidates for String-typed options via mpv $1=help | grep -v ':' | awk '{print $1}'. For options that carry a real value list (e.g. --vd, ~289 decoder names) this works nicely. For --help, however, the same pipeline just awk's the first token of each line of the general help prose, which yields: These aren't valid --help= values — they're spellings of other options. So with = appended, Tab offers completions such as --help=--sub-file=, which I felt would be more confusing than helpful. And because the argument is a free-form substring with no finite candidate set, there isn't really anything meaningful for the completer to suggest anyway. That said, I completely understand if you'd rather not special-case --help. I'm happy to go either way:
Whichever you prefer works for me. Thanks again for the review. |
|
The best would be to fix |
|
@guidocella Hi，Is the option added by "help" to complete similar to the following? |
|
It is. |
71e7d1f to
82bba37
Compare
The special-options regex decided which options are completed without
a trailing '=', but it only covered Print options and "not in config
files" flags. As a result, two groups of no-argument options still got
'=' appended:
(1) *-clr / *-help sub-options (e.g. --af-clr, --vo-help) which are
mpv list-type sub-options taking no argument.
(2) --help/--h which is String-typed in --list-options but actually
accepts a substring filter to narrow down the option listing.
Extend the regex to cover (1). For (2), keep '=' appended (it does take
an argument) and route completion through a new Help branch that mirrors
the zsh help-options state: complete bare option names (no "--"
prefix), excluding no-* negations and list-type sub-options
(-add/-append/-clr/-pre/-set/-remove/-toggle). Bare names are needed
because --help=<substring> matches within the option name body.
82bba37 to
7670a76
Compare
3 participants
The special-options regex decided which options are completed without
a trailing '=', but it only covered Print options and "not in config
files" flags. As a result, two groups of no-argument options still got
'=' appended:
Extend the regex to cover (1). For (2), keep '=' appended (it does take
an argument) and route completion through a new Help branch that mirrors
the zsh help-options state: complete bare option names (no "--"
prefix), excluding no-* negations and list-type sub-options
(-add/-append/-clr/-pre/-set/-remove/-toggle). Bare names are needed
because --help= matches within the option name body.