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

Fish completion does not handle "nospace" or file completion properly #1248

Closed
marckhouzam opened this issue Oct 7, 2020 · 2 comments · Fixed by #1249
Closed

Fish completion does not handle "nospace" or file completion properly #1248

marckhouzam opened this issue Oct 7, 2020 · 2 comments · Fixed by #1249

Comments

@marckhouzam
Copy link
Collaborator

marckhouzam commented Oct 7, 2020

This is the same bug that is present for zsh and described in #1211 and #1212.

Bug 1: File completion is not respected

The fish completion script performs file completion only if it receives no completions:

if test $numComps -eq 0; and test $nofiles -eq 0

However, it is valid for ValidArgsFunction to return completions that don't apply to the current toComplete prefix. This means file completion should be performed, but the script does not realize it as it still believes it received valid completions.

Say I have the following ValidArgsFunction for command withfile:

ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
    // We do not consider the toComplete prefix and always return a completion
    return []string{"why"}, cobra.ShellCompDirectiveDefault
},

If I have a file called myfile and run in fish:

$ ./testprog withfile my<TAB>

I would expect to get file completion and get myfile as a completion, but I don't. This is because

$ ./testprog __complete withfile my<ENTER>
why
:0
Completion ended with directive: ShellCompDirectiveDefault

Notice that above, the script would receive a completion even though it is not technically valid. This is allowed. But then, the script does not realize it must do file completion.

Bug 2: NoSpace is not respected all the time

For the exact same reason, ShellCompDirectiveNoSpace also fails when the __complete command returns completions that don't match the toComplete prefix.

Say I have the following ValidArgsFunction for command nospace:

ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
		if len(args) == 0 {
			return []string{"when\tThe time", "who\tThe person"}, cobra.ShellCompDirectiveNoSpace
		}
		return []string{"why\tThe reason"}, cobra.ShellCompDirectiveNoSpace
	},

If I run in fish:

$ ./testprog nospace whe<TAB>

a space is added after ./testprog nospace when even though the ShellCompDirectiveNoSpace was given.

This is because the script only handles ShellCompDirectiveNoSpace if a single completion is returned:

if test $numComps -eq 1; and test $nospace -ne 0

But my ValidArgsFunction does not filter the completions but instead returns all of them, leaving fish to filter them. In this case:

$ ./testprog __complete nospace whe<ENTER>
when	The time
who	The person
:2
Completion ended with directive: ShellCompDirectiveNoSpace

Notice that with the result above the completion script would receive two completions even though the who completion is not technically valid. This is allowed.

Bug 3: NoSpace not respected with descriptions

Finally, the nospace logic for fish does not handle the case when there is a description. To achieve "nospace" the script adds a second completion with an extra "." to prevent the shell from adding a space. However if there is a description, the extra "." is added to the description instead of the completion, and the nospace directive does not work.

I will post a PR to handle this better.

@github-actions
Copy link

github-actions bot commented Dec 7, 2020

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

@marckhouzam
Copy link
Collaborator Author

PR #1249 awaiting review.

marckhouzam added a commit to VilledeMontreal/helm that referenced this issue Feb 27, 2021
Ref: HIP 0008

When completing a repo name, extra information will be shown for
shells that support completions (fish, zsh).  For example:

$ helm repo remove <TAB>
bitnami  -- https://charts.bitnami.com/bitnami
center   -- https://repo.chartcenter.io
stable   -- https://kubernetes-charts.storage.googleapis.com

This commit does not add the description to repo names for the commands:
intall, template, upgrade, show and pull.  This is because we first need
a couple of Cobra fixes:
spf13/cobra#1211
spf13/cobra#1248

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
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.

1 participant