feat(complete): group options by tag (in zsh)#6334
feat(complete): group options by tag (in zsh)#6334bobrippling wants to merge 2 commits intoclap-rs:masterfrom
zsh)#6334Conversation
e5a4b4d to
629cac1
Compare
zsh)zsh)
65f0b45 to
b161c88
Compare
b161c88 to
0609ae5
Compare
zsh)zsh)
| if [[ "$value" == */ ]]; then | ||
| local dir_no_slash="${value%/}" | ||
| if [[ "$completion" == *:* ]]; then | ||
| local desc="${completion#*:}" | ||
| if [[ "$value" == *:* ]]; then | ||
| local desc="${value#*:}" | ||
| dirs+=("$dir_no_slash:$desc") | ||
| else | ||
| dirs+=("$dir_no_slash") | ||
| fi | ||
| else | ||
| other+=("$completion") | ||
| if (( ${+tag_map["$tag"]} )); then # key exists? | ||
| tag_map["$tag"]+=$'\n'"$value" | ||
| else | ||
| tag_map["$tag"]="$value" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Does this mean we aren't getting the / handling if a tag is present?
There was a problem hiding this comment.
The opposite in fact - if a / is there, we'll handle it (since we test that first) and only perform the option grouping for remaining options
There was a problem hiding this comment.
Items with / should still have grouping though
| } | ||
| write!( | ||
| buf, | ||
| "{}:", |
There was a problem hiding this comment.
I suppose we have two options:
- Pick a different separator - but what if that separator is in the tag?
- Avoid
:in tags, either by filtering them out at this point, or making it part of the tag requirement
What do you think?
There was a problem hiding this comment.
We can pick an unlikely separator, like _SEP_
There was a problem hiding this comment.
Thinking about it, I think we can avoid this entirely - I'll use the escape_value() function which looks to be for precisely this.
eb40761 to
eeee9ab
Compare
eeee9ab to
db855d8
Compare
db855d8 to
9e85b97
Compare
| % zstyle ':completion:*' group-name '' | ||
| % zstyle ':completion:*:descriptions' format '%d' | ||
| % exhaustive - | ||
| "Options" options |
There was a problem hiding this comment.
This is redundant. In --help, we just list the header. We should likely do the same here as well.
| "zstyle ':completion:*' group-name ''", | ||
| "zstyle ':completion:*:descriptions' format '%d'", |
There was a problem hiding this comment.
The format style is needed to have zsh show group descriptions - for example:
% ls -<TAB>
# ^ no option completions
% autoload -Uz compinit
% compinit
% ls -<TAB>
-@ -- display extended attribute keys and sizes in long listing
-1 -- single column output
-a -- list entries starting with .
-A -- list all except . and ..
...
# ^ options are shown
% zstyle ':completion:*:descriptions' format '%d'
% ls -<TAB>
option # <----
-@ -- display extended attribute keys and sizes in long listing
-1 -- single column output
-a -- list entries starting with .
-A -- list all except . and ..
...
# ^ note that the "description" header is now shown, in this case a plain "option"(group-name isn't needed so I've dropped it)
This allows zsh to group completions by whether they're global, and then by their tag. For example:
Closes #6320
Outstanding Questions
Options name
completing "Options" optionsdoesn't seem particulary nice, should we filter out the default"Options"tag?clap/clap_complete/src/engine/complete.rs
Lines 532 to 534 in 1420275
when generating the completions in
shells.rsTests
My new test is currently failing because it gets the zsh initial login setup:
I'm not sure why my test gets this but the others don't