feat(clap_complete): Support flags with values --flag barand -f bar in native completions
#5539
Conversation
shannmu
force-pushed
the
option_value
branch
2 times, most recently
from
30d127b
to
d55ea23
Compare
| state = if let Some(opt) = current_cmd.get_arguments().find(|a| { | ||
| a.get_long_and_visible_aliases() | ||
| .map_or(false, |v| v.into_iter().find(|s| *s == flag).is_some()) | ||
| }) { | ||
| match opt.get_action() { |
There was a problem hiding this comment.
what if we did
let opt = ...;
match opt.map(|o| o.get_action()) {
}This would reduce some of the rightward drift and the trailing else that is hard to match up with the if
shannmu
force-pushed
the
option_value
branch
3 times, most recently
from
05ab44a
to
456053d
Compare
shannmu
force-pushed
the
option_value
branch
2 times, most recently
from
4c6e44b
to
46dcc90
Compare
shannmu
changed the title
--flag bar and -f bar completion
|
The cases of -fbar and -f=bar have not been handled yet. |
|
I have added support for clap/clap_complete/src/dynamic/completer.rs Lines 141 to 146 in 469d847 This code didn't consider the case -f=bar[TAB], so it could generate -f=barf in completions. I will fix it later.
|
|
While talked about this in the call, to not lose track of this
Please split that out into another branch for a later PR so we can focus on getting the current PR merged. |
shannmu
changed the title
--flag barand -f bar in native completions
| @@ -148,6 +148,12 @@ fn suggest_argument_value() { | |||
| .short('F') | |||
| .value_parser(["json", "yaml", "toml"]), | |||
| ) | |||
| .arg( | |||
There was a problem hiding this comment.
Please edit the original test commit with these changes
| if is_escaped { | ||
| pos_index += 1; | ||
| state = ParseState::Pos(pos_index); | ||
| } else if arg.is_escape() { | ||
| is_escaped = true; | ||
| } else if let Some(_long) = arg.to_long() { | ||
| } else if let Some(_short) = arg.to_short() { | ||
| state = ParseState::ValueDone; | ||
| } else if let Some((flag, value)) = arg.to_long() { |
There was a problem hiding this comment.
Don't we need to check the state here before any of the branches? There is more nuance to it than that but we can fill that in over time
There was a problem hiding this comment.
Of course, we could check the state here before any of the branches, and I would prefer to do so. This way, ParseState can truly become a type used for state machine rather than just a type that records the current raw_arg state.
Benefits that could potentially come from this (that I can think of for now):
- Easier support for multiple values Support multiple values in native completions #3921 (adding new state and state transitions).
- More precise completion (accurately completing content based on different states).
| let mut flag = false; | ||
| state = if let Some(o) = opt { | ||
| match o.get_action() { |
There was a problem hiding this comment.
let Some(o) = opt else {
break;
};
| if flag { | ||
| break; | ||
| } |
There was a problem hiding this comment.
flag is confusing because they are all flags. I think you mean takes_value?
related issue: #3920
what is left
-fbarand-f=bar--flag=barin bashfoo --bar\toffering both--bar <value>and--bare