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.

Sign up for GitHub

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

Jump to bottom

zsh completion does not handle ShellCompDirectiveNoSpace properly #1211

Closed
marckhouzam opened this issue Aug 27, 2020 · 2 comments · Fixed by #1213
Closed

zsh completion does not handle ShellCompDirectiveNoSpace properly #1211

marckhouzam opened this issue Aug 27, 2020 · 2 comments · Fixed by #1213

Comments

@marckhouzam
Copy link
Collaborator

marckhouzam commented Aug 27, 2020 •
edited
Loading

While working on adding descriptions to the custom completions for helm, I ran into two bugs with zsh and ShellCompDirectiveNoSpace.

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
	},

Bug 1: NoSpace is not respected
If I run in zsh:

$ ./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:

cobra/zsh_completions.go

Line 215 in 02a0d2f

elif [ $((directive & shellCompDirectiveNoSpace)) -ne 0 ] && [ ${compCount} -eq 1 ]; then

But my ValidArgsFunction does not filter the completion but instead returns all of them, leaving zsh 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. But this is allowed.

Bug 2: Descriptions are not handled with ShellCompDirectiveNoSpace
The zsh script was using compadd directly to handle ShellCompDirectiveNoSpace:

cobra/zsh_completions.go

Line 219 in 02a0d2f

compadd -S '' "${lastComp}"

The problem is that compadd does not handle descriptions and just prints them as part of the completion. So, in my case:

$ ./testprog nospace when w<TAB>

will complete to:

./testprog nospace when why:The\ reason

I will post a PR to handle this better.
@marckhouzam marckhouzam mentioned this issue Aug 28, 2020
Closed
marckhouzam added a commit to VilledeMontreal/cobra that referenced this issue Aug 28, 2020
@marckhouzam
Fix zsh comp handling of ShellCompDirectiveNoSpace
7ed780a 
Fixes spf13#1211

When handling ShellCompDirectiveNoSpace we must still properly handle
descriptions.  To do so we cannot simply use 'compadd', but must use
zsh's '_describe' function.

Also, when handling ShellCompDirectiveNoSpace we cannot assume that
only a single completion will be given to the script.  In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix.  Therefore, we cannot use the
number of completions received in the completion script to determine
if we should activate the "no space" directive.  Instead, we can leave
it all to the '_describe' function.

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
@marckhouzam marckhouzam mentioned this issue Aug 28, 2020
Merged
marckhouzam added a commit to VilledeMontreal/cobra that referenced this issue Sep 6, 2020
@marckhouzam
Fix zsh for DirectiveNoSpace and DirectiveNoFileComp
a593b3d 
Fixes spf13#1211

When handling ShellCompDirectiveNoSpace we must still properly handle
descriptions.  To do so we cannot simply use 'compadd', but must use
zsh's '_describe' function.

Also, when handling ShellCompDirectiveNoSpace we cannot assume that
only a single completion will be given to the script.  In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix.  Therefore, we cannot use the
number of completions received in the completion script to determine
if we should activate the "no space" directive.  Instead, we can leave
it all to the '_describe' function.

Fixes spf13#1212

When handling ShellCompDirectiveNoFileComp we cannot base ourself on
the script receiving no valid completion. In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix at all.  Therefore, we cannot use
the number of completions received by the completion script to determine
if we should activate the "no file comp" directive.  Instead, we can
check if the '_describe' function has found any completions.

Finally, it is important for the script to return the return code of the
called zsh functions (_describe, _arguments).  This tells zsh if
completions were found or not, which if not, will trigger different
matching attempts, such as matching what the user typed with the the
content of possible completions (instead of just as the prefix).

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
marckhouzam added a commit to VilledeMontreal/cobra that referenced this issue Sep 6, 2020
@marckhouzam
Fix zsh for DirectiveNoSpace and DirectiveNoFileComp
c450cec 
Fixes spf13#1211

When handling ShellCompDirectiveNoSpace we must still properly handle
descriptions.  To do so we cannot simply use 'compadd', but must use
zsh's '_describe' function.

Also, when handling ShellCompDirectiveNoSpace we cannot assume that
only a single completion will be given to the script.  In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix.  Therefore, we cannot use the
number of completions received in the completion script to determine
if we should activate the "no space" directive.  Instead, we can leave
it all to the '_describe' function.

Fixes spf13#1212

When handling ShellCompDirectiveNoFileComp we cannot base ourself on
the script receiving no valid completion. In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix at all.  Therefore, we cannot use
the number of completions received by the completion script to determine
if we should activate the "no file comp" directive.  Instead, we can
check if the '_describe' function has found any completions.

Finally, it is important for the script to return the return code of the
called zsh functions (_describe, _arguments).  This tells zsh if
completions were found or not, which if not, will trigger different
matching attempts, such as matching what the user typed with the the
content of possible completions (instead of just as the prefix).

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
marckhouzam added a commit to VilledeMontreal/cobra that referenced this issue Sep 9, 2020
@marckhouzam
Fix zsh for DirectiveNoSpace and DirectiveNoFileComp
506d074 
Fixes spf13#1211

When handling ShellCompDirectiveNoSpace we must still properly handle
descriptions.  To do so we cannot simply use 'compadd', but must use
zsh's '_describe' function.

Also, when handling ShellCompDirectiveNoSpace we cannot assume that
only a single completion will be given to the script.  In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix.  Therefore, we cannot use the
number of completions received in the completion script to determine
if we should activate the "no space" directive.  Instead, we can leave
it all to the '_describe' function.

Fixes spf13#1212

When handling ShellCompDirectiveNoFileComp we cannot base ourself on
the script receiving no valid completion. In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix at all.  Therefore, we cannot use
the number of completions received by the completion script to determine
if we should activate the "no file comp" directive.  Instead, we can
check if the '_describe' function has found any completions.

Finally, it is important for the script to return the return code of the
called zsh functions (_describe, _arguments).  This tells zsh if
completions were found or not, which if not, will trigger different
matching attempts, such as matching what the user typed with the the
content of possible completions (instead of just as the prefix).

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
@marckhouzam marckhouzam mentioned this issue Oct 7, 2020
Closed
marckhouzam added a commit to VilledeMontreal/cobra that referenced this issue Oct 9, 2020
@marckhouzam
Fix zsh for DirectiveNoSpace and DirectiveNoFileComp
a203253 
Fixes spf13#1211

When handling ShellCompDirectiveNoSpace we must still properly handle
descriptions.  To do so we cannot simply use 'compadd', but must use
zsh's '_describe' function.

Also, when handling ShellCompDirectiveNoSpace we cannot assume that
only a single completion will be given to the script.  In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix.  Therefore, we cannot use the
number of completions received in the completion script to determine
if we should activate the "no space" directive.  Instead, we can leave
it all to the '_describe' function.

Fixes spf13#1212

When handling ShellCompDirectiveNoFileComp we cannot base ourself on
the script receiving no valid completion. In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix at all.  Therefore, we cannot use
the number of completions received by the completion script to determine
if we should activate the "no file comp" directive.  Instead, we can
check if the '_describe' function has found any completions.

Finally, it is important for the script to return the return code of the
called zsh functions (_describe, _arguments).  This tells zsh if
completions were found or not, which if not, will trigger different
matching attempts, such as matching what the user typed with the the
content of possible completions (instead of just as the prefix).

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

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

@marckhouzam
Copy link
Collaborator Author

This one is slated for review.

marckhouzam added a commit to VilledeMontreal/cobra that referenced this issue Jan 23, 2021
@marckhouzam
Fix zsh for DirectiveNoSpace and DirectiveNoFileComp
d8628c0 
Fixes spf13#1211

When handling ShellCompDirectiveNoSpace we must still properly handle
descriptions.  To do so we cannot simply use 'compadd', but must use
zsh's '_describe' function.

Also, when handling ShellCompDirectiveNoSpace we cannot assume that
only a single completion will be given to the script.  In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix.  Therefore, we cannot use the
number of completions received in the completion script to determine
if we should activate the "no space" directive.  Instead, we can leave
it all to the '_describe' function.

Fixes spf13#1212

When handling ShellCompDirectiveNoFileComp we cannot base ourself on
the script receiving no valid completion. In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix at all.  Therefore, we cannot use
the number of completions received by the completion script to determine
if we should activate the "no file comp" directive.  Instead, we can
check if the '_describe' function has found any completions.

Finally, it is important for the script to return the return code of the
called zsh functions (_describe, _arguments).  This tells zsh if
completions were found or not, which if not, will trigger different
matching attempts, such as matching what the user typed with the the
content of possible completions (instead of just as the prefix).

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
marckhouzam added a commit to VilledeMontreal/cobra that referenced this issue Jan 31, 2021
@marckhouzam
Fix zsh for DirectiveNoSpace and DirectiveNoFileComp
a89e419 
Fixes spf13#1211

When handling ShellCompDirectiveNoSpace we must still properly handle
descriptions.  To do so we cannot simply use 'compadd', but must use
zsh's '_describe' function.

Also, when handling ShellCompDirectiveNoSpace we cannot assume that
only a single completion will be given to the script.  In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix.  Therefore, we cannot use the
number of completions received in the completion script to determine
if we should activate the "no space" directive.  Instead, we can leave
it all to the '_describe' function.

Fixes spf13#1212

When handling ShellCompDirectiveNoFileComp we cannot base ourself on
the script receiving no valid completion. In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix at all.  Therefore, we cannot use
the number of completions received by the completion script to determine
if we should activate the "no file comp" directive.  Instead, we can
check if the '_describe' function has found any completions.

Finally, it is important for the script to return the return code of the
called zsh functions (_describe, _arguments).  This tells zsh if
completions were found or not, which if not, will trigger different
matching attempts, such as matching what the user typed with the the
content of possible completions (instead of just as the prefix).

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
marckhouzam added a commit to VilledeMontreal/cobra that referenced this issue Feb 14, 2021
@marckhouzam
Fix zsh for DirectiveNoSpace and DirectiveNoFileComp
579a46e 
Fixes spf13#1211

When handling ShellCompDirectiveNoSpace we must still properly handle
descriptions.  To do so we cannot simply use 'compadd', but must use
zsh's '_describe' function.

Also, when handling ShellCompDirectiveNoSpace we cannot assume that
only a single completion will be given to the script.  In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix.  Therefore, we cannot use the
number of completions received in the completion script to determine
if we should activate the "no space" directive.  Instead, we can leave
it all to the '_describe' function.

Fixes spf13#1212

When handling ShellCompDirectiveNoFileComp we cannot base ourself on
the script receiving no valid completion. In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix at all.  Therefore, we cannot use
the number of completions received by the completion script to determine
if we should activate the "no file comp" directive.  Instead, we can
check if the '_describe' function has found any completions.

Finally, it is important for the script to return the return code of the
called zsh functions (_describe, _arguments).  This tells zsh if
completions were found or not, which if not, will trigger different
matching attempts, such as matching what the user typed with the the
content of possible completions (instead of just as the prefix).

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
marckhouzam added a commit to VilledeMontreal/cobra that referenced this issue Feb 15, 2021
@marckhouzam
Fix zsh for DirectiveNoSpace and DirectiveNoFileComp
45fb2c3 
Fixes spf13#1211

When handling ShellCompDirectiveNoSpace we must still properly handle
descriptions.  To do so we cannot simply use 'compadd', but must use
zsh's '_describe' function.

Also, when handling ShellCompDirectiveNoSpace we cannot assume that
only a single completion will be given to the script.  In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix.  Therefore, we cannot use the
number of completions received in the completion script to determine
if we should activate the "no space" directive.  Instead, we can leave
it all to the '_describe' function.

Fixes spf13#1212

When handling ShellCompDirectiveNoFileComp we cannot base ourself on
the script receiving no valid completion. In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix at all.  Therefore, we cannot use
the number of completions received by the completion script to determine
if we should activate the "no file comp" directive.  Instead, we can
check if the '_describe' function has found any completions.

Finally, it is important for the script to return the return code of the
called zsh functions (_describe, _arguments).  This tells zsh if
completions were found or not, which if not, will trigger different
matching attempts, such as matching what the user typed with the the
content of possible completions (instead of just as the prefix).

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
marckhouzam added a commit to VilledeMontreal/cobra that referenced this issue Feb 19, 2021
@marckhouzam
Fix zsh for DirectiveNoSpace and DirectiveNoFileComp
35d352e 
Fixes spf13#1211

When handling ShellCompDirectiveNoSpace we must still properly handle
descriptions.  To do so we cannot simply use 'compadd', but must use
zsh's '_describe' function.

Also, when handling ShellCompDirectiveNoSpace we cannot assume that
only a single completion will be given to the script.  In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix.  Therefore, we cannot use the
number of completions received in the completion script to determine
if we should activate the "no space" directive.  Instead, we can leave
it all to the '_describe' function.

Fixes spf13#1212

When handling ShellCompDirectiveNoFileComp we cannot base ourself on
the script receiving no valid completion. In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix at all.  Therefore, we cannot use
the number of completions received by the completion script to determine
if we should activate the "no file comp" directive.  Instead, we can
check if the '_describe' function has found any completions.

Finally, it is important for the script to return the return code of the
called zsh functions (_describe, _arguments).  This tells zsh if
completions were found or not, which if not, will trigger different
matching attempts, such as matching what the user typed with the the
content of possible completions (instead of just as the prefix).

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
marckhouzam added a commit to VilledeMontreal/helm that referenced this issue Feb 27, 2021
@marckhouzam
feat(comp): Add descriptions for repo completion
d91448f 
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>
jpmcb pushed a commit that referenced this issue May 3, 2021
@marckhouzam
Fix zsh for DirectiveNoSpace and DirectiveNoFileComp (#1213)
95d23d2 
Fixes #1211

When handling ShellCompDirectiveNoSpace we must still properly handle
descriptions.  To do so we cannot simply use 'compadd', but must use
zsh's '_describe' function.

Also, when handling ShellCompDirectiveNoSpace we cannot assume that
only a single completion will be given to the script.  In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix.  Therefore, we cannot use the
number of completions received in the completion script to determine
if we should activate the "no space" directive.  Instead, we can leave
it all to the '_describe' function.

Fixes #1212

When handling ShellCompDirectiveNoFileComp we cannot base ourself on
the script receiving no valid completion. In fact,
ValidArgsFunction can return multiple completions, even if they don't
match the 'toComplete' argument prefix at all.  Therefore, we cannot use
the number of completions received by the completion script to determine
if we should activate the "no file comp" directive.  Instead, we can
check if the '_describe' function has found any completions.

Finally, it is important for the script to return the return code of the
called zsh functions (_describe, _arguments).  This tells zsh if
completions were found or not, which if not, will trigger different
matching attempts, such as matching what the user typed with the the
content of possible completions (instead of just as the prefix).

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
@da-ar da-ar mentioned this issue Jun 1, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix zsh completion handling of nospace and file completion VilledeMontreal/cobra
1 participant
@marckhouzam