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

Include --help and --version flag in completion #1813

Merged
merged 1 commit into from Oct 4, 2022

Conversation

marckhouzam
Copy link
Collaborator

@marckhouzam marckhouzam commented Sep 21, 2022

Fixes #1786

Thanks to @fnickels's nice work on #1707, I realized why the --help/-h and --version/-v flags where not included in shell completions.

The --help, -h, --version and -v flags are normally added when the execute() function is called on a command. When doing completion we don't call execute() so we need to add these flags explicitly to the command being completed.

Also, when doing shell completion following those flags, this PR disables all completions.
To do so, we keep track of if the --help and --version flags were added by Cobra using a new flag annotation.

To test:

./prog -<TAB>
# Notice that --help and -h are listed with this PR but not without it

./prog --help <TAB>
# Nothing is suggested with this PR but file completion is triggered without the PR

@github-actions github-actions bot added the size/L Denotes a PR that chanes 100-199 lines label Sep 21, 2022
@marckhouzam marckhouzam added kind/bug A bug in cobra; unintended behavior area/shell-completion All shell completions labels Sep 21, 2022
@github-actions github-actions bot removed the area/shell-completion All shell completions label Sep 21, 2022
@marckhouzam
Copy link
Collaborator Author

marckhouzam commented Sep 27, 2022

I realized that the completion logic for this improvement is still lacking. For example: helm status --help <TAB> will still suggest a list of helm releases.

I will need to treat the --help and --version flags as exceptions in Cobra. This actually make sense since these flags are added by Cobra. (Bear with my being a bit OCD about making completion work perfectly 😉 )

I will post an updated version soon.

@jpmcb
Copy link
Collaborator

jpmcb commented Oct 1, 2022

Nice! This all sounds good - give me a ping once this is ready to review! :D

@jpmcb jpmcb self-requested a review October 1, 2022 15:37
@github-actions github-actions bot added size/XL Denotes a PR that exceeds 200 lines. Caution! and removed size/L Denotes a PR that chanes 100-199 lines labels Oct 1, 2022
@github-actions
Copy link

github-actions bot commented Oct 1, 2022

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Fixes spf13#1786

The --help, -h, --version and -v flags are normally added when the
`execute()` function is called on a command.  When doing completion
we don't call `execute()` so we need to add these flags explicitly to
the command being completed.

Also, we disable all further completions if the 'help' or 'version'
flags are present on the command-line.

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
@github-actions
Copy link

github-actions bot commented Oct 1, 2022

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@marckhouzam
Copy link
Collaborator Author

marckhouzam commented Oct 1, 2022

Alright, this is ready @jpmcb. If you ignore the 234 lines of new tests, the change is fairly straightforward (< 30 lines).

One important aspect that you may have a different opinion on is that I have added a new flag annotation: FlagSetByCobraAnnotation. It is used to keep track if a flag was added by Cobra (the help and version flags). This was to avoid short-circuiting completions if these flags were program flags (not added by Cobra); if it is the program that adds such flags, it is the program's responsibility to handle shell completion adequately.

What to look for when testing (any generic test program will do):

  1. see the --help and -h flags being completed for any command (e.g., prog -<TAB>)
  2. see the --version and -v flags being completed for commands that have their Version field set (e.g., prog -<TAB>)
  3. no further completion AFTER any of --help, -h, --version and -v, but only if they were created by Cobra (not created by the program itself). Note that there are also Go tests added for this case. (e.g., prog -h <TAB>)

Also the tests of the cobra-completion-testing repo all pass.
I have also done some manual testing with helm.

@marckhouzam marckhouzam added this to the 1.6.0 milestone Oct 1, 2022
jpmcb
jpmcb approved these changes Oct 2, 2022
command.go Show resolved Hide resolved
@jpmcb jpmcb merged commit 212ea40 into spf13:main Oct 4, 2022
16 checks passed
@marckhouzam marckhouzam deleted the fix/compHelpAndVersion branch October 5, 2022 01:58
@umarcor umarcor mentioned this pull request Oct 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in cobra; unintended behavior size/XL Denotes a PR that exceeds 200 lines. Caution!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature request] completion for --help/--version
2 participants