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

Complete subcommands after local flag #1170

Closed
Luap99 opened this issue Jul 16, 2020 · 11 comments · Fixed by #1171
Closed

Complete subcommands after local flag #1170

Luap99 opened this issue Jul 16, 2020 · 11 comments · Fixed by #1171

Comments

@Luap99
Copy link
Contributor

Luap99 commented Jul 16, 2020

Currently the custom completion does not complete subcommands after a local flag was given.
I think this is wrong since a program can use subcommands even when there were local flags on the root/parent command given.

cobra/custom_completions.go

Lines 283 to 287 in b95db64

// Complete subcommand names, including the help command
if len(finalArgs) == 0 && !foundLocalNonPersistentFlag {
// We only complete sub-commands if:
// - there are no arguments on the command-line and
// - there are no local, non-peristent flag on the command-line

@Luap99
Copy link
Contributor Author

Luap99 commented Jul 16, 2020

@marckhouzam Is there a special reason for this? Otherwise i would like to change this behavior.

@marckhouzam
Copy link
Collaborator

Is there a special reason for this? Otherwise i would like to change this behavior.

Yes this is on purpose for local flags. If a command has a local flags it does not get inherited by a subcommand, so it would be wrong to suggest subcommands as they will fail with the error "unknown flag".

However for persistent flags, which are inherited by subcommands, we do complete subcommands.

Makes sense?

@Luap99
Copy link
Contributor Author

Luap99 commented Jul 17, 2020

Yes that makes sence.

However in the podman it works like this:
podman --remote ps -a remote is a local flag on the root cmd here.

I found out that this works because TraverseChildren is set to true on the root cmd so i would say the completion logic should respect TraverseChildren too.

I can open a PR if you agree?

@marckhouzam
Copy link
Collaborator

Aha. I'm not familiar with the TraverseChildren logic. You may have found a scenario that was missed. Yes please go ahead and file a PR. Thanks!

@marckhouzam
Copy link
Collaborator

@Luap99 could you check if the case you mention works with bash completion?

@Luap99
Copy link
Contributor Author

Luap99 commented Jul 17, 2020

@Luap99 could you check if the case you mention works with bash completion?

Yes it does work but regardless of whether TraverseChildren is set or not.

@marckhouzam
Copy link
Collaborator

That is strange. I mimicked the local flag behavior on what was done in the current bash completion... See #288

@Luap99
Copy link
Contributor Author

Luap99 commented Jul 17, 2020

I also noticed there is bug when you have a local boolean flag there is no subcommand completion but after you provide one argument subcommands are suggested. This doesn't happen with a Persistent Flag.

@Luap99
Copy link
Contributor Author

Luap99 commented Jul 17, 2020

I also noticed there is bug when you have a local boolean flag there is no subcommand completion but after you provide one argument subcommands are suggested.

Nevermind my test setup was wrong ;)
It works for local boolean flags, but for local flags with arguments subcommands are still getting completed. So it is inconsistent.

Luap99 pushed a commit to Luap99/cobra that referenced this issue Jul 17, 2020
subcommands when a local flag is set. This is good unless
TraverseChildren is set to true where local flags
can be set on parent commands.

This commit allows subcommands to be completed only
if TraverseChildren is true.

Closes spf13#1170

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
Luap99 pushed a commit to Luap99/cobra that referenced this issue Jul 18, 2020
subcommands when a local flag is set. This is good unless
TraverseChildren is set to true where local flags
can be set on parent commands.

This commit allows subcommands to be completed only
if TraverseChildren is true.

Closes spf13#1170

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
@marckhouzam
Copy link
Collaborator

@Luap99 could you check if the case you mention works with bash completion?

Yes it does work but regardless of whether TraverseChildren is set or not.

I've looked further into this and it seems to be a bug with the current bash completion.
When using a boolean flag or the = form for a flag (e.g. integer), sub-commands are not shown after a local flag as expected, but when using the no-= form, it does complete:

bash-5.0$ ./testprog --local 1 [tab][tab]
child1      completion  help
# Sub-commands are completed when they shouldn't

bash-5.0$ ./testprog --local=1 [tab][tab]
[file completion is done]
# Sub-commands are NOT completed as expected

bash-5.0$ ./testprog --bool [tab][tab]
[file completion is done]
# Sub-commands are NOT completed as expected

My guess is that this bug was introduced along the way as the bash completion script evolved.
I will open an issue to keep track of this.

@marckhouzam
Copy link
Collaborator

I've opened #1172 for the discrepancy with bash.

Luap99 pushed a commit to Luap99/cobra that referenced this issue Jul 19, 2020
The current custom completion logic does not complete
subcommands when a local flag is set. This is good unless
TraverseChildren is set to true where local flags
can be set on parent commands.

This commit allows subcommands to be completed
if TraverseChildren is set to true on the root cmd.

Closes spf13#1170

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
Luap99 pushed a commit to Luap99/cobra that referenced this issue Jul 19, 2020
The current custom completion logic does not complete
subcommands when a local flag is set. This is good unless
TraverseChildren is set to true where local flags
can be set on parent commands.

This commit allows subcommands to be completed
if TraverseChildren is set to true on the root cmd.

Closes spf13#1170

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
Luap99 pushed a commit to Luap99/cobra that referenced this issue Jul 20, 2020
The current custom completion logic does not complete
subcommands when a local flag is set. This is good unless
TraverseChildren is set to true where local flags
can be set on parent commands.

This commit allows subcommands to be completed
if TraverseChildren is set to true on the root cmd.

Closes spf13#1170

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
jpmcb pushed a commit that referenced this issue Sep 9, 2020
* Complete subcommands when TraverseChildren is true in custom completion

The current custom completion logic does not complete
subcommands when a local flag is set. This is good unless
TraverseChildren is set to true where local flags
can be set on parent commands.

This commit allows subcommands to be completed
if TraverseChildren is set to true on the root cmd.

Closes #1170

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>

* Complete subcommands when TraverseChildren is true in bash completion

The current bash completion logic does not complete
subcommands when a local flag is set. There is also a bug
where subcommands are sometimes still getting completed. see: #1172

If TraverseChildren is true we should allow subcommands
to be completed even if a local flag is set.

Closes #1172

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
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 a pull request may close this issue.

2 participants