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

Shell completions with whitespace seem to work differently on bash vs. other supported shells #1740

Open
kangasta opened this issue Jun 21, 2022 · 7 comments · May be fixed by #2126
Open

Shell completions with whitespace seem to work differently on bash vs. other supported shells #1740

kangasta opened this issue Jun 21, 2022 · 7 comments · May be fixed by #2126
Labels
area/shell-completion All shell completions kind/bug A bug in cobra; unintended behavior

Comments

@kangasta
Copy link

Other shell completions escape whitespace in completions, but bash completions seems to add completions with whitespace as multiple suggestions or multiple arguments depending on bash version. I would expect also bash completions to escape whitespace in completions from ValidArgsFunction.

bash v3:

$ ./sh-completion-demo whitespace [TAB][TAB]
no-whitespace    with whitespace
$ ./sh-completion-demo whitespace w[TAB]
$ ./sh-completion-demo whitespace with whitespace

bash v4 & v5

$ ./sh-completion-demo whitespace [TAB][TAB]
no-whitespace  whitespace     with

fish:

> ./sh-completion-demo whitespace [TAB]
no-whitespace  with whitespace
> ./sh-completion-demo whitespace w[TAB]
> ./sh-completion-demo whitespace with\ whitespace

zsh:

% ./sh-completion-demo whitespace [TAB]
no-whitespace    with whitespace
% ./sh-completion-demo whitespace w[TAB]
% ./sh-completion-demo whitespace with\ whitespace

powershell

> .\sh-completion-demo.exe whitespace [TAB]
> .\sh-completion-demo.exe whitespace with` whitespace

Minimal code to reproduce above usage with github.com/spf13/cobra v1.5.0 (Gist):

package main

import (
	"fmt"
	"os"

	"github.com/spf13/cobra"
)

var RootCmd = &cobra.Command{
	Use:  "sh-completion-demo",
	Long: "Demo shell completion with whitespace",
}

var WhitespaceCmd = &cobra.Command{
	Use:   "whitespace",
	Short: "Noun with completion that includes whitespace",
	RunE: func(cmd *cobra.Command, args []string) error {
		if args[0] != "with whitespace" && args[0] != "no-whitespace" {
			return fmt.Errorf(`argument must be "with whitespace" or "no-whitespace"`)
		}
		return nil
	},
	SilenceUsage: true,
	ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
		return []string{"with whitespace", "no-whitespace"}, cobra.ShellCompDirectiveNoFileComp
	},
}

func init() {
	RootCmd.AddCommand(WhitespaceCmd)
}

func main() {
	if err := RootCmd.Execute(); err != nil {
		os.Exit(1)
	}
}
@marckhouzam marckhouzam added kind/bug A bug in cobra; unintended behavior area/shell-completion All shell completions labels Jun 21, 2022
This was referenced Jun 23, 2022
Open
Closed
@marckhouzam
Copy link
Collaborator

@scop, you may have some expertise on this. Do you have special handling if a completion has a space in it for bash?

@scop
Copy link
Contributor

scop commented Aug 20, 2022

Hi @kangasta ;)

It's annoyingly difficult in bash.

A cheap generic way is to fake the completions to be filenames. That does take care of the escaping, but it's obviously semantically incorrect for completions that really aren't filenames. And it may have side effects.

Another one is to run the strings through printf %q ... here and there in the completion code. Some related discussion e.g. at https://stackoverflow.com/questions/26509260/bash-tab-completion-with-spaces

Anyway I believe neither of these options is currently available for cobra users. I'll have a quick peek at PR #1743.

@github-actions
Copy link

The Cobra project currently lacks enough contributors to adequately respond to all issues. This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the issue is closed.
    You can:
  • Make a comment to remove the stale label and show your support. The 60 days reset. - If an issue has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interseted in reopening

@kangasta
Copy link
Author

Rebased related PR and added comment there as well: #1743

@github-actions
Copy link

The Cobra project currently lacks enough contributors to adequately respond to all issues. This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the issue is closed.
    You can:
  • Make a comment to remove the stale label and show your support. The 60 days reset. - If an issue has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interested in reopening

@kangasta
Copy link
Author

Adding a comment to keep this from being closed.

@kangasta kangasta mentioned this issue Feb 1, 2023
Merged
kangasta added a commit to UpCloudLtd/upcloud-cli that referenced this issue Feb 6, 2023
@kangasta
feat(completion): use cobras default completion command (#204)
0f8c91d 
This requires removing whitespace from completions as cobras bash
completions do not support values with whitespace (See
spf13/cobra#1740). This is done by replacing
whitespace with non-breaking spaces during completion generation. The
argument resolvers with values that can include whitespace are also
updated to handle both modified and original arguments, i.e. user can
still input whitespaces normally when argument is quoted or escaped.
@JeffFaer
Copy link

I ran across this the other day. This isn't just limited to whitespace, but all characters that need escaping in bash. My completions were using >, which is kinda unfortunate because cobra started redirecting output to random places during completion

JeffFaer added a commit to JeffFaer/cobra that referenced this issue Mar 21, 2024
@JeffFaer
fix(bash): Fix bash completion for suggestions that contain special c…
3c78872 
…haracters.

Special characters include whitepace, so this is more general than
 spf13#1743. This should also fix spf13#1740.

I added some test cases to cobra-completion-testing. This PR makes them
pass. It also doesn't trip any of the performance regression tests. I'm
happy to submit those tests as a PR as well.
  - https://github.com/JeffFaer/cobra-completion-testing/tree/special_characters
  - JeffFaer/cobra-completion-testing@52254c1
@JeffFaer JeffFaer linked a pull request Mar 21, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/shell-completion All shell completions kind/bug A bug in cobra; unintended behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(bash): Fix bash completion for suggestions that contain special characters. JeffFaer/cobra
4 participants
@scop @marckhouzam @JeffFaer @kangasta