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

Flag name completion should not be done after '--' delimiter even when DisableFlagParsing is enabled #1542

Open
marckhouzam opened this issue Nov 27, 2021 · 3 comments
Labels
area/flags-args Changes to functionality around command line flags and args area/shell-completion All shell completions lifecycle/frozen Prevents GitHub actions from labeling issues / PRs with stale and rotten triage/needs-info Needs more investigation from maintainers or more info from the issue provider

Comments

@marckhouzam
Copy link
Collaborator

While working on completion for kubectl plugins I noticed that we don't handle the -- delimiter properly when DisableFlagParsing is turned on. This is a special case that we may have overlooked in #1308.

Let's take helm plugins as an example, as helm has a fully working solution for completion. We will use the fullstatus plugin.

$ helm __complete fullstatus -- --r
--registry-config	path to the registry config file
--repository-cache	path to the file containing cached repository indexes
--repository-config	path to the file containing repository names and URLs
--revision
:4
Completion ended with directive: ShellCompDirectiveNoFileComp
# Notice that flag names are being suggested for completion even though we use the -- delimiter

# This can be compared to using a non-plugin sub-command
# which properly returns no completion as no arguments start with a --r
$ helm __complete get -- --r
:4
Completion ended with directive: ShellCompDirectiveNoFileComp

I believe the problem is that the way we figure out if the -- delimiter is being used does not work when DisableFlagParsing is enabled. I haven't dug deeper into it yet.

To reproduce the problem with helm, you can do the following, after installing helm:

$ helm plugin install https://github.com/marckhouzam/helm-fullstatus
$ helm fullstatus -- --r<TAB>

/cc @Luap99

@Luap99
Copy link
Contributor

Luap99 commented Dec 4, 2021

#1308 implemented this by parsing the flags. This is needed because of the interspersed option.

I do not fully understand why does cobra do flag completion when DisableFlagParsing is set? Why are even flags set when they are not parsed?

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

This issue is being marked as stale due to a long period of inactivity

@Luap99
Copy link
Contributor

Luap99 commented Feb 3, 2022

Maybe to give more context, currently we add -- to the arguments and then parse the flags. From the parsed flags we use the remaining argument count, now we parse the flags with the correct arguments (without --) and compare the the argument count vs the one with the --. If -- was added to the argument count we now that we should not do flag completion because everything following is counted as argument and not flag.
With DisableFlagParsing set ParseFlags() will not work and the argument count will always be the same so we will never trigger to condition to disable flag completion.

Also in your example the flags are returned from the plugin right? In this case shouldn't the plugin not send flag completion back to the main application. IMO In such scenarios cobra should just forward the completion and not try to interpret it.

@johnSchnake johnSchnake added area/shell-completion All shell completions area/flags-args Changes to functionality around command line flags and args needs investigation labels Feb 23, 2022
@jpmcb jpmcb added triage/needs-info Needs more investigation from maintainers or more info from the issue provider lifecycle/frozen Prevents GitHub actions from labeling issues / PRs with stale and rotten and removed kind/stale labels Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/flags-args Changes to functionality around command line flags and args area/shell-completion All shell completions lifecycle/frozen Prevents GitHub actions from labeling issues / PRs with stale and rotten triage/needs-info Needs more investigation from maintainers or more info from the issue provider
Projects
None yet
Development

No branches or pull requests

4 participants