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

Support multiple values in native completions #5601

Merged
merged 4 commits into from
Jul 31, 2024

Conversation

shannmu
Copy link
Contributor

@shannmu shannmu commented Jul 26, 2024

Related issue #3921

Work in this PR

  • Support multi-values for both short flag and long flag
    What we can complete now?
    dynamic --flag bar1 [TAB]
    
    It will completebar1 bar2 bar3.
    Also,
    dynamic -F bar1 [TAB]
    
    will completebar1 bar2 bar3.
  • Support multi-values for positional argument with num_args(N) which N is fixed.
    dynamic pos_a [TAB]
    
    will complete pos_a pos_b pos_c
    dynamic -- pos_a [TAB]
    
    will complete pos_a pos_b pos_c

What may be is left

  • Support multi-values for both short flag and long flag with format as follows
    dynamic --flag=bar1 [TAB]
    dynamic -F=bar1 [TAB]
    
  • Support multi-values for positional argument with last or trailing_var_arg

@shannmu
Copy link
Contributor Author

shannmu commented Jul 26, 2024

I can pass the clap_complete/tests/testsuite/bash.rs::complete test on my PC. I'm not sure if the CI failed because of my code changes.

@shannmu
Copy link
Contributor Author

shannmu commented Jul 30, 2024

Thank you for your review! Sorry for the late response. I have made the changes to the code. During the modification process, I discovered a small error. For ParseState::Pos(usize), using only a single usize to record the takes_num_args of that positional argument is insufficient. There is a command line like this:

dynamic --flag bar -- pos_1 pos_2 pos_2 pos_2

The original incorrect logic would treat pos_1 as the first value of the positional argument with index=2, leading to a count error for index=2. Therefore, I changed ParseState::Pos(usize) to ParseState::Pos((usize, usize)). The first usize represents the pos_index of the current argument, and the second usize represents the count of the positional argument the current parameter is. This way, we can avoid the original error.

@epage
Copy link
Member

epage commented Jul 30, 2024

. There is a command line like this:

If we've identified a bug like this during development, it means we should probably add a test case for it.

@epage epage merged commit ca2265c into clap-rs:master Jul 31, 2024
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants