From e1e772cb6c958c255d5ca7261ba2450ed77e8437 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Tue, 4 Aug 2026 23:27:29 +0900 Subject: [PATCH 01/38] fix: name the shell completion request so "--" cannot hide it A completion request appended to the end of the command line cannot be told apart from a positional argument that looks like one, and after "--" that is exactly what it is. The two readings are both required: $ aqua exec -- foo --generate-shell-completion # pass it on to foo (#1932) $ aqua exec -- foo # complete, run nothing (#1993) The shell sends the same argv either way, so no reading of it can serve both, and #1932 was fixed at the cost of #1993: a command line holding "--" runs instead of completing. Pressing tab on 'app exec -- git push origin main' runs 'git push origin' today. The request is now named in the first argument, where "--" cannot reach it: __complete ... Everything the shell has typed follows, and the word under the cursor comes last, empty or not. That word was guesswork until now, since the scripts sent it only when it started with "-", which is why "cmd --" and "cmd -- " arrived as the same request. The word being completed after a "--" gets no suggestion, because it is a positional argument of whatever the command runs, but the request is still a completion, so nothing is run. The deprecated form keeps working for scripts generated before this change, with the ambiguity it cannot escape: a command line holding "--" is answered as an ordinary run. It no longer swallows the flag while doing so, which is what #1932 asked for and what #2316 regressed by stripping it. Regenerating the script and sourcing it again is what closes #1993 for good. The request form shadows nothing: an app with a command of that name is left to answer for itself. Fixes #1993 --- autocomplete/bash_autocomplete | 26 +- autocomplete/fish_autocomplete | 13 +- autocomplete/powershell_autocomplete.ps1 | 28 +- autocomplete/zsh_autocomplete | 16 +- command.go | 7 + command_run.go | 13 +- completion.go | 11 + completion_test.go | 245 +++++++++++++++++- .../v3/examples/completions/customizations.md | 7 +- .../examples/completions/shell-completions.md | 15 +- help.go | 109 ++++++-- help_test.go | 103 +++++++- 12 files changed, 527 insertions(+), 66 deletions(-) diff --git a/autocomplete/bash_autocomplete b/autocomplete/bash_autocomplete index 42eb17b8b2..da7aa08ccc 100755 --- a/autocomplete/bash_autocomplete +++ b/autocomplete/bash_autocomplete @@ -11,15 +11,22 @@ __%[1]s_init_completion() { fi } +# The request names the completion in its first argument, where a "--" typed on the +# command line cannot turn it into a positional argument of whatever the command runs. +# The word under the cursor is sent as the last argument, empty or not, so that +# "cmd --" and "cmd -- " can be told apart. +# +# It is built as an array rather than as a string to eval, so that a word holding a +# space or a quote reaches the command as the single word it is. __%[1]s_build_completion_request() { - local -a words_before_cursor=("${COMP_WORDS[@]:0:${COMP_CWORD}}") - local current_word="${COMP_WORDS[COMP_CWORD]}" + __cli_completion_request=("${COMP_WORDS[0]}" "__complete") - if [[ "${current_word}" == "-"* ]]; then - printf '%%s %%s --generate-shell-completion' "${words_before_cursor[*]}" "${current_word}" - else - printf '%%s --generate-shell-completion' "${words_before_cursor[*]}" - fi + local i + for (( i = 1; i < COMP_CWORD; i++ )); do + __cli_completion_request+=("${COMP_WORDS[i]}") + done + + __cli_completion_request+=("${COMP_WORDS[COMP_CWORD]-}") } # Keep Bash 3 compatibility: associative arrays require Bash 4+, so @@ -44,15 +51,14 @@ __%[1]s_bash_autocomplete() { if [[ "${words[0]}" != "source" ]]; then local cur opts local cword="${COMP_CWORD}" - local request_comp COMPREPLY=() cur="${words[$cword]}" __%[1]s_init_completion -n "=:" || return - request_comp="$(__%[1]s_build_completion_request)" - opts=$(eval "${request_comp}" 2>/dev/null) + __%[1]s_build_completion_request + opts=$("${__cli_completion_request[@]}" 2>/dev/null) # Completion output lines use "token:description" format. # Keep token/description in parallel arrays for Bash 3 compatibility. diff --git a/autocomplete/fish_autocomplete b/autocomplete/fish_autocomplete index 5f2fcd7f6b..0363d39883 100644 --- a/autocomplete/fish_autocomplete +++ b/autocomplete/fish_autocomplete @@ -5,12 +5,13 @@ function __%[1]s_perform_completion set -l args (commandline -opc) # Extract the last arg (partial input) set -l lastArg (commandline -ct) - - if string match -q -- "-*" $lastArg - set results ($args[1] $args[2..-1] $lastArg --generate-shell-completion 2> /dev/null) - else - set results ($args[1] $args[2..-1] --generate-shell-completion 2> /dev/null) - end + + # The request names the completion in its first argument, where a "--" typed on + # the command line cannot turn it into a positional argument of whatever the + # command runs. The word under the cursor is sent as the last argument, quoted so + # that an empty one is still an argument, which tells "cmd --" from + # "cmd -- ". + set results ($args[1] __complete $args[2..-1] "$lastArg" 2> /dev/null) # Remove trailing empty lines for line in $results[-1..1] diff --git a/autocomplete/powershell_autocomplete.ps1 b/autocomplete/powershell_autocomplete.ps1 index fee6d0c7d2..a54abecc36 100644 --- a/autocomplete/powershell_autocomplete.ps1 +++ b/autocomplete/powershell_autocomplete.ps1 @@ -1,9 +1,29 @@ $fn = $($MyInvocation.MyCommand.Name) $name = $fn -replace "(.*)\.ps1$", '$1' Register-ArgumentCompleter -Native -CommandName $name -ScriptBlock { - param($commandName, $wordToComplete, $cursorPosition) - $other = "$wordToComplete --generate-shell-completion" - Invoke-Expression $other | ForEach-Object { + param($wordToComplete, $commandAst, $cursorPosition) + + # The request names the completion in its first argument, where a "--" typed on + # the command line cannot turn it into a positional argument of whatever the + # command runs. The word under the cursor is sent as the last argument, empty or + # not, so that "cmd --" and "cmd -- " can be told apart. + $elements = @($commandAst.CommandElements | ForEach-Object { $_.ToString() }) + $command = $elements[0] + $words = @() + if ($elements.Count -gt 1) { + $words = $elements[1..($elements.Count - 1)] + } + # Once the word under the cursor has any character it is an element of its own, + # so it is dropped here and sent as the last argument instead. + if ($wordToComplete -and $words.Count -gt 0 -and $words[-1] -eq $wordToComplete) { + if ($words.Count -eq 1) { + $words = @() + } else { + $words = $words[0..($words.Count - 2)] + } + } + + & $command __complete @words $wordToComplete 2>$null | ForEach-Object { $parts = $_.Split(':', 2) if ($parts.Count -eq 2) { $completion = $parts[0].Trim() @@ -13,4 +33,4 @@ Register-ArgumentCompleter -Native -CommandName $name -ScriptBlock { [System.Management.Automation.CompletionResult]::new($_, $_, 'ParameterValue', $_) } } -} \ No newline at end of file +} diff --git a/autocomplete/zsh_autocomplete b/autocomplete/zsh_autocomplete index d24049a72f..873da695aa 100644 --- a/autocomplete/zsh_autocomplete +++ b/autocomplete/zsh_autocomplete @@ -5,15 +5,13 @@ compdef _%[1]s %[1]s _%[1]s() { local -a opts # Declare a local array - local current - current=${words[-1]} # -1 means "the last element" - if [[ "$current" == "-"* ]]; then - # Current word starts with a hyphen, so complete flags/options - opts=("${(@f)$(${words[@]:0:#words[@]-1} ${current} --generate-shell-completion)}") - else - # Current word does not start with a hyphen, so complete subcommands - opts=("${(@f)$(${words[@]:0:#words[@]-1} --generate-shell-completion)}") - fi + local -a request + # The request names the completion in its first argument, where a "--" typed on + # the command line cannot turn it into a positional argument of whatever the + # command runs. The word under the cursor is sent as the last argument, empty or + # not, so that "cmd --" and "cmd -- " can be told apart. + request=("${words[1]}" "__complete" "${(@)words[2,CURRENT-1]}" "${words[CURRENT]}") + opts=("${(@f)$("${request[@]}")}") if [[ "${opts[1]}" != "" ]]; then _describe 'values' opts diff --git a/command.go b/command.go index 4cd907a558..d84e525fa0 100644 --- a/command.go +++ b/command.go @@ -157,6 +157,13 @@ type Command struct { didSetupDefaults bool // whether in shell completion mode shellCompletion bool + // the word the shell is completing, or nil when the request did not carry it, + // which is every request in the deprecated form. Only the root command holds it. + completionWord *string + // whether a "--" precedes the word being completed, which makes that word a + // positional argument of whatever the command runs. Only the root command holds + // it. + completionTerminated bool // whether global help flag was added globaHelpFlagAdded bool // whether global version flag was added diff --git a/command_run.go b/command_run.go index 8d5907151e..abff2533f8 100644 --- a/command_run.go +++ b/command_run.go @@ -118,20 +118,21 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context osArgs = append(osArgs, args...) } } - // handle the completion flag separately from the flagset since + // handle the completion request separately from the flagset since // completion could be attempted after a flag, but before its value was put // on the command line. this causes the flagset to interpret the completion - // flag name as the value of the flag before it which is undesirable + // request as the value of the flag before it which is undesirable // note that we can only do this because the shell autocomplete function - // always appends the completion flag at the end of the command + // sends the request in a place the flagset never reaches: the first argument, + // or, for a script generated before that change, the last one tracef("checking osArgs %v (cmd=%[2]q)", osArgs, cmd.Name) - cmd.shellCompletion, osArgs = checkShellCompleteFlag(cmd, osArgs) + cmd.shellCompletion, osArgs = parseShellCompleteRequest(cmd, osArgs) - tracef("setting cmd.shellCompletion=%[1]v from checkShellCompleteFlag (cmd=%[2]q)", cmd.shellCompletion && cmd.EnableShellCompletion, cmd.Name) + tracef("setting cmd.shellCompletion=%[1]v from parseShellCompleteRequest (cmd=%[2]q)", cmd.shellCompletion && cmd.EnableShellCompletion, cmd.Name) cmd.shellCompletion = cmd.EnableShellCompletion && cmd.shellCompletion } - tracef("using post-checkShellCompleteFlag arguments %[1]q (cmd=%[2]q)", osArgs, cmd.Name) + tracef("using post-parseShellCompleteRequest arguments %[1]q (cmd=%[2]q)", osArgs, cmd.Name) tracef("setting self as cmd in context (cmd=%[1]q)", cmd.Name) ctx = context.WithValue(ctx, commandContextKey, cmd) diff --git a/completion.go b/completion.go index b78dc385fd..8ca4d4831b 100644 --- a/completion.go +++ b/completion.go @@ -11,7 +11,18 @@ const ( completionCommandName = "completion" // This flag is supposed to only be used by the completion script itself to generate completions on the fly. + // + // Deprecated: completion scripts name the request with completionCommandRequest + // instead. A request appended to the end of the command line is indistinguishable + // from a positional argument after "--", which is why it is no longer generated. + // It is still understood so that scripts generated before that change keep + // working. completionFlag = "--generate-shell-completion" + + // This argument is supposed to only be used by the completion script itself to + // generate completions on the fly. It is the first argument of the request, where + // "--" cannot turn it into a positional argument. + completionCommandRequest = "__complete" ) type renderCompletion func(cmd *Command, appName string) (string, error) diff --git a/completion_test.go b/completion_test.go index 8550f6b41a..381d23dee2 100644 --- a/completion_test.go +++ b/completion_test.go @@ -242,7 +242,10 @@ func TestCompletionFishFormat(t *testing.T) { r.Contains(output, "(__myapp_perform_completion)", "completion function should be registered") } -func TestCompletionFishOmitsPositionalTokenFromDynamicCompletion(t *testing.T) { +func TestCompletionFishSendsTokenBeingCompleted(t *testing.T) { + // The word under the cursor is part of the request, quoted so that an empty one + // is still an argument: without it, "cmd --" and "cmd -- " would reach + // the command as the same request. cmd := &Command{ Name: "myapp", EnableShellCompletion: true, @@ -256,12 +259,15 @@ func TestCompletionFishOmitsPositionalTokenFromDynamicCompletion(t *testing.T) { output, err := fishRender(cmd, "myapp") r.NoError(err) - r.Contains(output, `if string match -q -- "-*" $lastArg`) - r.Contains(output, "set results ($args[1] $args[2..-1] $lastArg --generate-shell-completion 2> /dev/null)") - r.Contains(output, "set results ($args[1] $args[2..-1] --generate-shell-completion 2> /dev/null)") + r.Contains(output, `set results ($args[1] __complete $args[2..-1] "$lastArg" 2> /dev/null)`) + r.NotContains(output, completionFlag, "the deprecated request form must not be generated") } -func TestCompletionBashOmitsPositionalTokenFromDynamicCompletion(t *testing.T) { +func TestCompletionBashSendsTokenBeingCompleted(t *testing.T) { + // The word under the cursor is part of the request, empty or not: without it, + // "cmd --" and "cmd -- " would reach the command as the same request. + // The request is an array rather than a string to eval, so a word holding a space + // or a quote reaches the command as the single word it is. cmd := &Command{ Name: "myapp", EnableShellCompletion: true, @@ -275,9 +281,47 @@ func TestCompletionBashOmitsPositionalTokenFromDynamicCompletion(t *testing.T) { output, err := bashRender(cmd, "myapp") r.NoError(err) - r.Contains(output, `if [[ "${current_word}" == "-"* ]]; then`) - r.Contains(output, `printf '%s %s --generate-shell-completion' "${words_before_cursor[*]}" "${current_word}"`) - r.Contains(output, `printf '%s --generate-shell-completion' "${words_before_cursor[*]}"`) + r.Contains(output, `__cli_completion_request=("${COMP_WORDS[0]}" "__complete")`) + r.Contains(output, `__cli_completion_request+=("${COMP_WORDS[COMP_CWORD]-}")`) + r.Contains(output, `opts=$("${__cli_completion_request[@]}" 2>/dev/null)`) + r.NotContains(output, `eval "`, "the request must not go through eval") + r.NotContains(output, completionFlag, "the deprecated request form must not be generated") +} + +func TestCompletionZshSendsTokenBeingCompleted(t *testing.T) { + cmd := &Command{ + Name: "myapp", + EnableShellCompletion: true, + } + + r := require.New(t) + + zshRender := shellCompletions["zsh"] + r.NotNil(zshRender, "zsh completion renderer should exist") + + output, err := zshRender(cmd, "myapp") + r.NoError(err) + + r.Contains(output, `request=("${words[1]}" "__complete" "${(@)words[2,CURRENT-1]}" "${words[CURRENT]}")`) + r.NotContains(output, completionFlag, "the deprecated request form must not be generated") +} + +func TestCompletionPowershellSendsTokenBeingCompleted(t *testing.T) { + cmd := &Command{ + Name: "myapp", + EnableShellCompletion: true, + } + + r := require.New(t) + + pwshRender := shellCompletions["pwsh"] + r.NotNil(pwshRender, "pwsh completion renderer should exist") + + output, err := pwshRender(cmd, "myapp") + r.NoError(err) + + r.Contains(output, `& $command __complete @words $wordToComplete 2>$null`) + r.NotContains(output, completionFlag, "the deprecated request form must not be generated") } func TestCompletionSubcommand(t *testing.T) { @@ -569,3 +613,188 @@ func TestCompletionShellWriteError(t *testing.T) { err := cmd.Run(buildTestContext(t), []string{"foo", completionCommandName, shellName}) assert.ErrorContains(t, err, "writer error") } + +// TestCompletionRequestNeverRunsAction is the regression test for +// https://github.com/urfave/cli/issues/1993: a shell asking for completions must +// never run the command, whatever the command line holds. A request appended to the +// end of the command line cannot promise that, because "--" turns it into a +// positional argument that a wrapper command is entitled to pass on, which is what +// https://github.com/urfave/cli/issues/1932 asked for. +func TestCompletionRequestNeverRunsAction(t *testing.T) { + for _, tc := range []struct { + name string + args []string + }{ + { + name: "plain", + args: []string{"foo", completionCommandRequest, "exec", ""}, + }, + { + name: "after a double dash", + args: []string{"foo", completionCommandRequest, "exec", "--", "rm", "-rf"}, + }, + { + name: "completing the double dash", + args: []string{"foo", completionCommandRequest, "exec", "--"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ran := false + out := &bytes.Buffer{} + cmd := &Command{ + EnableShellCompletion: true, + Writer: out, + Commands: []*Command{ + { + Name: "exec", + SkipFlagParsing: true, + Action: func(context.Context, *Command) error { + ran = true + return nil + }, + }, + }, + } + + r := require.New(t) + r.NoError(cmd.Run(buildTestContext(t), tc.args)) + r.False(ran, "the action must not run for a completion request") + }) + } +} + +// TestCompletionRequestAfterDoubleDash checks that the words after a "--" get no +// suggestion: they are positional arguments of whatever the command runs, so this +// command's flags and subcommands are no answer to them. The "--" being completed is +// not one of them. +func TestCompletionRequestAfterDoubleDash(t *testing.T) { + for _, tc := range []struct { + name string + args []string + expected string + }{ + { + // The completion is for the word after "exec", which has no subcommand of + // its own to offer beyond the built-in help. + name: "before the double dash", + args: []string{"foo", completionCommandRequest, "exec", ""}, + expected: "help:Shows a list of commands or help for one command\n", + }, + { + name: "the double dash itself", + args: []string{"foo", completionCommandRequest, "exec", "--"}, + expected: "--excitement\n--help:show help\n", + }, + { + name: "after the double dash", + args: []string{"foo", completionCommandRequest, "exec", "--", "git", "pu"}, + expected: "", + }, + { + name: "a flag after the double dash", + args: []string{"foo", completionCommandRequest, "exec", "--", "git", "--ver"}, + expected: "", + }, + } { + t.Run(tc.name, func(t *testing.T) { + out := &bytes.Buffer{} + cmd := &Command{ + EnableShellCompletion: true, + Writer: out, + Commands: []*Command{ + { + Name: "exec", + Flags: []Flag{&BoolFlag{Name: "excitement"}}, + Action: func(context.Context, *Command) error { return nil }, + }, + }, + } + + r := require.New(t) + r.NoError(cmd.Run(buildTestContext(t), tc.args)) + r.Equal(tc.expected, out.String()) + }) + } +} + +// TestCompletionDeprecatedRequestPassedOnAfterDoubleDash is the regression test for +// https://github.com/urfave/cli/issues/1932: after a "--", the deprecated request +// form is a positional argument, so a wrapper command passes it on to whatever it +// runs instead of answering it. That command, run by the wrapper, is the one the +// shell was asking about. +func TestCompletionDeprecatedRequestPassedOnAfterDoubleDash(t *testing.T) { + var got []string + out := &bytes.Buffer{} + cmd := &Command{ + EnableShellCompletion: true, + Writer: out, + Commands: []*Command{ + { + Name: "exec", + SkipFlagParsing: true, + Action: func(_ context.Context, cmd *Command) error { + got = cmd.Args().Slice() + return nil + }, + }, + }, + } + + r := require.New(t) + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "exec", "--", "child", completionFlag})) + r.Equal([]string{"--", "child", completionFlag}, got) + r.Empty(out.String(), "the wrapper must not answer a request meant for what it runs") +} + +// TestCompletionRequestKeepsArgsShape checks that a ShellComplete function sees the +// same cmd.Args() under both request forms: the word being completed is part of them +// when it starts with "-", and left out otherwise. +func TestCompletionRequestKeepsArgsShape(t *testing.T) { + for _, tc := range []struct { + name string + args []string + expected string + }{ + { + name: "deprecated form completing a flag", + args: []string{"foo", "sub", "arg", "-", completionFlag}, + expected: "[arg -]\n", + }, + { + name: "request completing a flag", + args: []string{"foo", completionCommandRequest, "sub", "arg", "-"}, + expected: "[arg -]\n", + }, + { + name: "deprecated form completing a word", + args: []string{"foo", "sub", "arg", completionFlag}, + expected: "[arg]\n", + }, + { + name: "request completing a word", + args: []string{"foo", completionCommandRequest, "sub", "arg", "wor"}, + expected: "[arg]\n", + }, + } { + t.Run(tc.name, func(t *testing.T) { + out := &bytes.Buffer{} + cmd := &Command{ + EnableShellCompletion: true, + Writer: out, + Commands: []*Command{ + { + Name: "sub", + ShellComplete: func(_ context.Context, cmd *Command) { + fmt.Fprintf(cmd.Root().Writer, "%v\n", cmd.Args().Slice()) + }, + Action: func(context.Context, *Command) error { return nil }, + }, + }, + } + + r := require.New(t) + r.NoError(cmd.Run(buildTestContext(t), tc.args)) + r.Equal(tc.expected, out.String()) + }) + } +} diff --git a/docs/v3/examples/completions/customizations.md b/docs/v3/examples/completions/customizations.md index 96f3565359..c080748360 100644 --- a/docs/v3/examples/completions/customizations.md +++ b/docs/v3/examples/completions/customizations.md @@ -107,11 +107,12 @@ func main() { #### Customization -The default shell completion flag (`--generate-shell-completion`) is defined as -`cli.EnableShellCompletion`, and may be redefined if desired, e.g.: +Setting `cli.EnableShellCompletion` makes the app answer a completion request, which the +generated scripts send as a `__complete` first argument followed by the words typed so far and +the word being completed: ```go diff --git a/docs/v3/examples/completions/shell-completions.md b/docs/v3/examples/completions/shell-completions.md index 8c4137e598..bcb31d7a63 100644 --- a/docs/v3/examples/completions/shell-completions.md +++ b/docs/v3/examples/completions/shell-completions.md @@ -7,7 +7,9 @@ search: The urfave/cli v3 library supports programmable completion for apps utilizing its framework. This means that the completion is generated dynamically at runtime by invoking the app itself with a special hidden -flag. The urfave/cli searches for this flag and activates a different flow for command paths than regular flow +first argument, `__complete`, followed by the words typed so far and, as the last argument, the word being +completed. The urfave/cli searches for that argument and activates a different flow for command paths than +regular flow. The following shells are supported - bash @@ -115,6 +117,17 @@ The procedure for other shells is similar to bash though the specific paths for shells may vary. Some of the sections below detail the setup need for other shells as well as examples in those shells. +#### Regenerate the script after upgrading + +Completion scripts generated before urfave/cli asked for completions with `__complete` end their request +with a `--generate-shell-completion` flag instead. Those scripts keep working, but a command line holding +a `--` cannot be answered through them: after `--` only positional arguments are accepted, so the flag +belongs to whatever the app runs rather than to the app itself, and the app runs instead of completing +(see [#1932](https://github.com/urfave/cli/issues/1932) and +[#1993](https://github.com/urfave/cli/issues/1993)). Regenerating the script and sourcing it again is what +resolves that: `__complete` is the first argument, where a `--` typed later on the command line can no +longer turn it into a positional argument. + #### Default auto-completion ```go diff --git a/help.go b/help.go index 4bedf87d5d..09938521f7 100644 --- a/help.go +++ b/help.go @@ -255,11 +255,28 @@ func DefaultCompleteWithFlags(ctx context.Context, cmd *Command) { } else { tracef("running default complete with os.Args flags[%v]", args) } - argsLen := len(args) + + if cmd == nil { + return + } + + // Everything after "--" is a positional argument of whatever the command runs, so + // this command's flags and subcommands are no answer to it. + // https://unix.stackexchange.com/a/11382 + if cmd.Root().completionTerminated { + tracef("not suggesting past a \"--\" on command %[1]q", cmd.Name) + return + } + lastArg := "" - // parent command will have --generate-shell-completion so we need - // to account for that - if argsLen > 1 { + if word := cmd.Root().completionWord; word != nil { + // The request says which word is being completed, so there is nothing to work + // out from the position of the arguments. + lastArg = *word + } else if argsLen := len(args); argsLen > 1 { + // A request in the deprecated form leaves the word out unless it starts with + // "-", and the parent command still has completionFlag on it, so the word is + // looked for one before the end. lastArg = args[argsLen-2] } else if argsLen > 0 { lastArg = args[argsLen-1] @@ -275,11 +292,8 @@ func DefaultCompleteWithFlags(ctx context.Context, cmd *Command) { return } - if cmd != nil { - tracef("printing command suggestions on command %[1]q", cmd.Name) - printCommandSuggestions(cmd.Commands, cmd.Root().Writer) - return - } + tracef("printing command suggestions on command %[1]q", cmd.Name) + printCommandSuggestions(cmd.Commands, cmd.Root().Writer) } // ShowCommandHelpAndExit exits with code after showing help via ShowCommandHelp. @@ -471,11 +485,42 @@ func checkVersion(cmd *Command) bool { return cmd.versionFlag != nil && cmd.versionFlag.IsSet() } -func checkShellCompleteFlag(c *Command, arguments []string) (bool, []string) { +// parseShellCompleteRequest reports whether arguments are a shell completion request +// and returns the arguments to parse. What the request says about the word being +// completed is recorded on c, which is the root command. +// +// Two request forms are understood. The current one names the request up front: +// +// __complete ... +// +// The completion scripts send every word before the cursor, then the word under the +// cursor, which is the empty string when the cursor sits on a fresh word. Naming the +// request in the first argument is what keeps it out of reach of "--": everything +// after that terminator is a positional argument, so a request appended at the end of +// the command line cannot be told apart from a positional argument that happens to +// look like one. See the deprecated form below for what that ambiguity costs. +// +// The deprecated form appends completionFlag to the command line. Scripts generated +// before this change still use it, so it keeps working, with one caveat it cannot +// escape: a command line holding "--" is answered as an ordinary run rather than as a +// completion, because after "--" the flag is a positional argument that belongs to +// whatever the command runs. That is what a wrapper command needs (see +// https://github.com/urfave/cli/issues/1932), and it is why a shell that appends the +// flag after a "--" runs the command instead of completing it (see +// https://github.com/urfave/cli/issues/1993). Regenerating the completion script and +// sourcing it again resolves that in favor of completing, since the request is then +// no longer something a command line can imitate. +func parseShellCompleteRequest(c *Command, arguments []string) (bool, []string) { if (c.parent == nil && !c.EnableShellCompletion) || (c.parent != nil && !c.Root().shellCompletion) { return false, arguments } + // A command of that name, if the app happens to have one, is what was asked for: + // the request form is understood only where it shadows nothing. + if len(arguments) > 1 && arguments[1] == completionCommandRequest && c.Command(completionCommandRequest) == nil { + return true, c.parseCompletionRequest(arguments) + } + pos := len(arguments) - 1 lastArg := arguments[pos] @@ -483,18 +528,52 @@ func checkShellCompleteFlag(c *Command, arguments []string) (bool, []string) { return false, arguments } - // If arguments include "--" before the token being completed, shell completion - // is disabled because after "--" only positional arguments are accepted. + // The word being completed is at position pos-1, immediately before + // completionFlag, so only the arguments before that position are checked and + // completing "--" itself still works. // https://unix.stackexchange.com/a/11382 - // Note: The token being completed is at position pos-1 (immediately before completionFlag). - // We only check arguments before that position, so completing "--" itself still works. if pos >= 1 && slices.Contains(arguments[:pos-1], "--") { - return false, arguments[:pos] + // The flag is a positional argument here, so it is left in place for the + // command to pass on, and the command runs. + return false, arguments } + // This request form does not say which word is being completed, so nothing is + // recorded and DefaultCompleteWithFlags works it out from the arguments. return true, arguments[:pos] } +// parseCompletionRequest records what a request naming completionCommandRequest says +// about the word being completed, and returns the arguments to parse. +// +// The word is kept in those arguments when it starts with "-", and dropped from them +// otherwise, which is the shape the deprecated request form produced. A ShellComplete +// function reading cmd.Args() therefore sees the same thing under both forms. +func (cmd *Command) parseCompletionRequest(arguments []string) []string { + // arguments[0] is the program, arguments[1] is completionCommandRequest, and the + // word being completed is last. A request holding neither, which no script sends, + // is read as an empty word on an empty command line. + var words []string + word := "" + if len(arguments) > 2 { + words = arguments[2 : len(arguments)-1] + word = arguments[len(arguments)-1] + } + cmd.completionWord = &word + // Everything after a "--" is a positional argument of whatever the command runs, + // so this command has no suggestion for it. A "--" being completed is not one: + // it is the word itself, and flags still answer it. + cmd.completionTerminated = slices.Contains(words, "--") + + args := make([]string, 0, len(arguments)-1) + args = append(args, arguments[0]) + args = append(args, words...) + if strings.HasPrefix(word, "-") { + args = append(args, word) + } + return args +} + func shouldRunCompletion(cmd *Command) bool { tracef("checking completions on command %[1]q", cmd.Name) diff --git a/help_test.go b/help_test.go index d3c831371d..0828057575 100644 --- a/help_test.go +++ b/help_test.go @@ -1884,7 +1884,7 @@ GLOBAL OPTIONS: `, output.String()) } -func Test_checkShellCompleteFlag(t *testing.T) { +func Test_parseShellCompleteRequest(t *testing.T) { t.Parallel() tests := []struct { name string @@ -1892,6 +1892,9 @@ func Test_checkShellCompleteFlag(t *testing.T) { arguments []string wantShellCompletion bool wantArgs []string + wantWord string + wantWordSet bool + wantTerminated bool }{ { name: "disable-shell-completion", @@ -1919,13 +1922,15 @@ func Test_checkShellCompleteFlag(t *testing.T) { wantArgs: []string{"foo"}, }, { + // The flag is a positional argument of whatever the command runs, so it + // stays in place and the command runs. name: "arguments include double dash", arguments: []string{"--", "foo", completionFlag}, cmd: &Command{ EnableShellCompletion: true, }, wantShellCompletion: false, - wantArgs: []string{"--", "foo"}, + wantArgs: []string{"--", "foo", completionFlag}, }, { name: "shell completion", @@ -1945,15 +1950,105 @@ func Test_checkShellCompleteFlag(t *testing.T) { wantShellCompletion: true, wantArgs: []string{"foo", "--"}, }, + { + // The deprecated request form says nothing about the word being completed, + // which DefaultCompleteWithFlags then works out from the arguments. + name: "deprecated form records no word", + arguments: []string{"prog", "sub", "-", completionFlag}, + cmd: &Command{ + EnableShellCompletion: true, + }, + wantShellCompletion: true, + wantArgs: []string{"prog", "sub", "-"}, + }, + { + name: "request names the completion", + arguments: []string{"prog", completionCommandRequest, "sub", ""}, + cmd: &Command{ + EnableShellCompletion: true, + }, + wantShellCompletion: true, + wantArgs: []string{"prog", "sub"}, + wantWordSet: true, + }, + { + // A word starting with "-" stays in the arguments, which is the shape the + // deprecated form produced, so a ShellComplete function sees no difference. + name: "request names the completion of a flag", + arguments: []string{"prog", completionCommandRequest, "sub", "--fl"}, + cmd: &Command{ + EnableShellCompletion: true, + }, + wantShellCompletion: true, + wantArgs: []string{"prog", "sub", "--fl"}, + wantWord: "--fl", + wantWordSet: true, + }, + { + // The word being completed is a positional argument of whatever the + // command runs, which this command has no suggestion for. It must still be + // a completion, or the command would run. + name: "request names the completion after a double dash", + arguments: []string{"prog", completionCommandRequest, "exec", "--", "git", "pu"}, + cmd: &Command{ + EnableShellCompletion: true, + }, + wantShellCompletion: true, + wantArgs: []string{"prog", "exec", "--", "git"}, + wantWord: "pu", + wantWordSet: true, + wantTerminated: true, + }, + { + // The "--" is the word being completed here, not a terminator, so flags + // still answer it. + name: "request names the completion of a double dash", + arguments: []string{"prog", completionCommandRequest, "exec", "--"}, + cmd: &Command{ + EnableShellCompletion: true, + }, + wantShellCompletion: true, + wantArgs: []string{"prog", "exec", "--"}, + wantWord: "--", + wantWordSet: true, + }, + { + name: "request without a word being completed", + arguments: []string{"prog", completionCommandRequest}, + cmd: &Command{ + EnableShellCompletion: true, + }, + wantShellCompletion: true, + wantArgs: []string{"prog"}, + wantWordSet: true, + }, + { + // The request form shadows nothing: a command of that name is what was + // asked for. + name: "a command of the same name wins", + arguments: []string{"prog", completionCommandRequest, "sub", ""}, + cmd: &Command{ + EnableShellCompletion: true, + Commands: []*Command{{Name: completionCommandRequest}}, + }, + wantShellCompletion: false, + wantArgs: []string{"prog", completionCommandRequest, "sub", ""}, + }, } for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - shellCompletion, args := checkShellCompleteFlag(tt.cmd, tt.arguments) + shellCompletion, args := parseShellCompleteRequest(tt.cmd, tt.arguments) assert.Equal(t, tt.wantShellCompletion, shellCompletion) assert.Equal(t, tt.wantArgs, args) + gotWord := "" + if tt.cmd.completionWord != nil { + gotWord = *tt.cmd.completionWord + } + assert.Equal(t, tt.wantWordSet, tt.cmd.completionWord != nil) + assert.Equal(t, tt.wantWord, gotWord) + assert.Equal(t, tt.wantTerminated, tt.cmd.completionTerminated) }) } } From 746f1c9041b798f82c6148148b2f556b39d31aaf Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 22:31:17 +0900 Subject: [PATCH 02/38] fix: answer each completion request on its own terms The word being completed and the "--" before it were recorded on the root command by the request that carried them, and by nothing else, so a Command answering a second request still held what the first one said. A shell runs one request per process and never sees it; a test, a REPL or an embedded use answers several through one Command and does: a request past a "--" made every later request answer as though it were past one too. The two fields become one, which every run replaces, so there is one place that has to be right rather than two that have to agree. --- command.go | 12 +++++------- completion_test.go | 38 ++++++++++++++++++++++++++++++++++++++ help.go | 42 ++++++++++++++++++++++++++++++++---------- help_test.go | 10 +++++----- 4 files changed, 80 insertions(+), 22 deletions(-) diff --git a/command.go b/command.go index d84e525fa0..b17eadbaf2 100644 --- a/command.go +++ b/command.go @@ -157,13 +157,11 @@ type Command struct { didSetupDefaults bool // whether in shell completion mode shellCompletion bool - // the word the shell is completing, or nil when the request did not carry it, - // which is every request in the deprecated form. Only the root command holds it. - completionWord *string - // whether a "--" precedes the word being completed, which makes that word a - // positional argument of whatever the command runs. Only the root command holds - // it. - completionTerminated bool + // what the shell completion request being answered says about the word being + // completed, or nil when this run is answering none. Only the root command holds + // it, and every run replaces it, so a Command answering several requests never + // carries one request's state into the next. + completion *completionRequest // whether global help flag was added globaHelpFlagAdded bool // whether global version flag was added diff --git a/completion_test.go b/completion_test.go index 381d23dee2..74d91d8386 100644 --- a/completion_test.go +++ b/completion_test.go @@ -798,3 +798,41 @@ func TestCompletionRequestKeepsArgsShape(t *testing.T) { }) } } + +// TestCompletionRequestStateIsPerRun checks that a Command answering several requests +// carries no state from one into the next. A shell runs one request per process, but +// a test, a REPL or an embedded use answers several through the same Command. +func TestCompletionRequestStateIsPerRun(t *testing.T) { + out := &bytes.Buffer{} + cmd := &Command{ + EnableShellCompletion: true, + Writer: out, + Commands: []*Command{ + { + Name: "exec", + Flags: []Flag{&BoolFlag{Name: "excitement"}}, + Action: func(context.Context, *Command) error { return nil }, + }, + }, + } + + r := require.New(t) + + // A request past a "--" gets no suggestion, and records that. + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "exec", "--", "git", "pu"})) + r.Empty(out.String()) + + // The next request is a different one, and is answered on its own terms. + out.Reset() + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "exec", "-"})) + r.Equal("--excitement\n--help:show help\n", out.String()) + + // The same holds for a request in the deprecated form, which says nothing about + // the word being completed and so must not read what an earlier one said. + out.Reset() + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "exec", "--", "git", "pu"})) + r.Empty(out.String()) + out.Reset() + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "exec", "-", completionFlag})) + r.Equal("--excitement\n--help:show help\n", out.String()) +} diff --git a/help.go b/help.go index 09938521f7..55b05f122d 100644 --- a/help.go +++ b/help.go @@ -260,19 +260,21 @@ func DefaultCompleteWithFlags(ctx context.Context, cmd *Command) { return } + req := cmd.Root().completion + // Everything after "--" is a positional argument of whatever the command runs, so // this command's flags and subcommands are no answer to it. // https://unix.stackexchange.com/a/11382 - if cmd.Root().completionTerminated { + if req != nil && req.terminated { tracef("not suggesting past a \"--\" on command %[1]q", cmd.Name) return } lastArg := "" - if word := cmd.Root().completionWord; word != nil { + if req != nil && req.wordKnown { // The request says which word is being completed, so there is nothing to work // out from the position of the arguments. - lastArg = *word + lastArg = req.word } else if argsLen := len(args); argsLen > 1 { // A request in the deprecated form leaves the word out unless it starts with // "-", and the parent command still has completionFlag on it, so the word is @@ -485,6 +487,19 @@ func checkVersion(cmd *Command) bool { return cmd.versionFlag != nil && cmd.versionFlag.IsSet() } +// completionRequest is what a shell completion request says about the word being +// completed. +type completionRequest struct { + // word is the word the shell is completing. wordKnown says whether the request + // carried it: the deprecated request form does not. + word string + wordKnown bool + // terminated says whether a "--" precedes the word, which makes that word a + // positional argument of whatever the command runs rather than one this command + // has any suggestion for. + terminated bool +} + // parseShellCompleteRequest reports whether arguments are a shell completion request // and returns the arguments to parse. What the request says about the word being // completed is recorded on c, which is the root command. @@ -511,6 +526,9 @@ func checkVersion(cmd *Command) bool { // sourcing it again resolves that in favor of completing, since the request is then // no longer something a command line can imitate. func parseShellCompleteRequest(c *Command, arguments []string) (bool, []string) { + // Whatever the previous run of this Command recorded says nothing about this one. + c.completion = nil + if (c.parent == nil && !c.EnableShellCompletion) || (c.parent != nil && !c.Root().shellCompletion) { return false, arguments } @@ -538,8 +556,9 @@ func parseShellCompleteRequest(c *Command, arguments []string) (bool, []string) return false, arguments } - // This request form does not say which word is being completed, so nothing is - // recorded and DefaultCompleteWithFlags works it out from the arguments. + // This request form does not say which word is being completed, so + // DefaultCompleteWithFlags works it out from the arguments. + c.completion = &completionRequest{} return true, arguments[:pos] } @@ -559,11 +578,14 @@ func (cmd *Command) parseCompletionRequest(arguments []string) []string { words = arguments[2 : len(arguments)-1] word = arguments[len(arguments)-1] } - cmd.completionWord = &word - // Everything after a "--" is a positional argument of whatever the command runs, - // so this command has no suggestion for it. A "--" being completed is not one: - // it is the word itself, and flags still answer it. - cmd.completionTerminated = slices.Contains(words, "--") + cmd.completion = &completionRequest{ + word: word, + wordKnown: true, + // Everything after a "--" is a positional argument of whatever the command + // runs. A "--" being completed is not one: it is the word itself, and flags + // still answer it. + terminated: slices.Contains(words, "--"), + } args := make([]string, 0, len(arguments)-1) args = append(args, arguments[0]) diff --git a/help_test.go b/help_test.go index 0828057575..ed06c582f7 100644 --- a/help_test.go +++ b/help_test.go @@ -2042,13 +2042,13 @@ func Test_parseShellCompleteRequest(t *testing.T) { shellCompletion, args := parseShellCompleteRequest(tt.cmd, tt.arguments) assert.Equal(t, tt.wantShellCompletion, shellCompletion) assert.Equal(t, tt.wantArgs, args) - gotWord := "" - if tt.cmd.completionWord != nil { - gotWord = *tt.cmd.completionWord + gotWord, gotWordSet, gotTerminated := "", false, false + if req := tt.cmd.completion; req != nil { + gotWord, gotWordSet, gotTerminated = req.word, req.wordKnown, req.terminated } - assert.Equal(t, tt.wantWordSet, tt.cmd.completionWord != nil) + assert.Equal(t, tt.wantWordSet, gotWordSet) assert.Equal(t, tt.wantWord, gotWord) - assert.Equal(t, tt.wantTerminated, tt.cmd.completionTerminated) + assert.Equal(t, tt.wantTerminated, gotTerminated) }) } } From f0021673f52a7527958c96d1657825622eb19a2b Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 22:31:59 +0900 Subject: [PATCH 03/38] fix: suggest nothing past a "--" whatever the completion func The rule that the words after a "--" belong to whatever the command runs was kept by DefaultCompleteWithFlags alone, so a command carrying a ShellComplete of its own went on suggesting past the terminator. That left the rule as something every app had to reimplement, from a "--" it cannot see: the parsed arguments no longer say where the terminator was. Answering with nothing before the completion func runs applies it to every command instead. The action still does not run, which is what a shell asking for completions needs either way. --- completion_test.go | 33 +++++++++++++++++++++++++++++++++ help.go | 23 ++++++++++++----------- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/completion_test.go b/completion_test.go index 74d91d8386..50b8a8ddd6 100644 --- a/completion_test.go +++ b/completion_test.go @@ -836,3 +836,36 @@ func TestCompletionRequestStateIsPerRun(t *testing.T) { r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "exec", "-", completionFlag})) r.Equal("--excitement\n--help:show help\n", out.String()) } + +// TestCompletionCustomShellCompleteNotRunPastDoubleDash checks that a command +// carrying a ShellComplete of its own suggests nothing past a "--" without having to +// know about "--": the words there are positional arguments of whatever it runs. +func TestCompletionCustomShellCompleteNotRunPastDoubleDash(t *testing.T) { + ran := false + out := &bytes.Buffer{} + cmd := &Command{ + EnableShellCompletion: true, + Writer: out, + Commands: []*Command{ + { + Name: "exec", + ShellComplete: func(_ context.Context, cmd *Command) { + ran = true + fmt.Fprintln(cmd.Root().Writer, "custom") + }, + Action: func(context.Context, *Command) error { return nil }, + }, + }, + } + + r := require.New(t) + + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "exec", "--", "git", "pu"})) + r.False(ran, "the completion func must not run past a double dash") + r.Empty(out.String()) + + // It is the "--" that stops it, not the command. + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "exec", "pu"})) + r.True(ran) + r.Equal("custom\n", out.String()) +} diff --git a/help.go b/help.go index 55b05f122d..140ef3ef33 100644 --- a/help.go +++ b/help.go @@ -260,18 +260,8 @@ func DefaultCompleteWithFlags(ctx context.Context, cmd *Command) { return } - req := cmd.Root().completion - - // Everything after "--" is a positional argument of whatever the command runs, so - // this command's flags and subcommands are no answer to it. - // https://unix.stackexchange.com/a/11382 - if req != nil && req.terminated { - tracef("not suggesting past a \"--\" on command %[1]q", cmd.Name) - return - } - lastArg := "" - if req != nil && req.wordKnown { + if req := cmd.Root().completion; req != nil && req.wordKnown { // The request says which word is being completed, so there is nothing to work // out from the position of the arguments. lastArg = req.word @@ -617,6 +607,17 @@ func shouldRunCompletion(cmd *Command) bool { } func runCompletion(ctx context.Context, cmd *Command) { + // Everything after "--" is a positional argument of whatever the command runs, so + // this command has no suggestion for it. Answering with nothing here rather than + // in the completion func applies that to every command, including one carrying a + // ShellComplete of its own, which would otherwise have to know about "--" itself + // to keep the promise the terminator makes. + // https://unix.stackexchange.com/a/11382 + if req := cmd.Root().completion; req != nil && req.terminated { + tracef("not suggesting past a \"--\" on command %[1]q", cmd.Name) + return + } + if cmd.ShellComplete != nil { tracef("running shell completion func for command %[1]q", cmd.Name) cmd.ShellComplete(ctx, cmd) From 6646190de3fa78771a3b0a802598232079ac41dc Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 22:49:27 +0900 Subject: [PATCH 04/38] fix: build the bash request from the words bash-completion reassembled Bash splits the command line on COMP_WORDBREAKS, which holds "=" and ":", so COMP_WORDS turns "--opt=value" into three words. __init_completion is called with -n "=:" precisely to put those back together, and the candidates are filtered against the cur it returns, but the request was built from the raw COMP_WORDS: pressing tab on "app --opt=va" asked the app about "--opt", "=" and "va" as three arguments while filtering the answer against "--opt=va". zsh, which does not split there, sent one word for the same line. The words also arrive quoted, as typed: "app run \"hello world\" " asked the app about a word holding the two quote characters. eval used to take those off as a side effect of re-parsing the line, and dropping eval dropped that too. They are now taken off by walking the word, so a line holding $(...) is still not executed by pressing the tab key. --- autocomplete/bash_autocomplete | 64 ++++++++++++++++++++++++++++++---- completion_test.go | 8 +++-- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/autocomplete/bash_autocomplete b/autocomplete/bash_autocomplete index da7aa08ccc..13c2e8a458 100755 --- a/autocomplete/bash_autocomplete +++ b/autocomplete/bash_autocomplete @@ -11,22 +11,72 @@ __%[1]s_init_completion() { fi } +# Remove one level of shell quoting from a word, the way the shell does before it +# hands a word to a command, and leave the result in __%[1]s_dequoted. +# +# Doing it here rather than with eval is what keeps a command line holding $(...) or +# `...` from being executed by pressing the tab key. +__%[1]s_dequote() { + local s="$1" out="" c n quote="" + local i=0 len=${#1} + + while (( i < len )); do + c="${s:i:1}" + if [[ "${quote}" == "'" ]]; then + if [[ "${c}" == "'" ]]; then quote=""; else out="${out}${c}"; fi + elif [[ "${quote}" == '"' ]]; then + if [[ "${c}" == '"' ]]; then + quote="" + elif [[ "${c}" == "\\" ]]; then + i=$(( i + 1 )) + n="${s:i:1}" + # Inside double quotes a backslash only escapes these. + case "${n}" in + '"' | "\\" | '$' | '`') out="${out}${n}" ;; + *) out="${out}\\${n}" ;; + esac + else + out="${out}${c}" + fi + else + case "${c}" in + "'" | '"') quote="${c}" ;; + "\\") i=$(( i + 1 )); out="${out}${s:i:1}" ;; + *) out="${out}${c}" ;; + esac + fi + i=$(( i + 1 )) + done + + __%[1]s_dequoted="${out}" +} + # The request names the completion in its first argument, where a "--" typed on the # command line cannot turn it into a positional argument of whatever the command runs. # The word under the cursor is sent as the last argument, empty or not, so that # "cmd --" and "cmd -- " can be told apart. # # It is built as an array rather than as a string to eval, so that a word holding a -# space or a quote reaches the command as the single word it is. +# space reaches the command as the single word it is. +# +# The words come from words/cword rather than from COMP_WORDS/COMP_CWORD: bash splits +# the line on COMP_WORDBREAKS, so "--opt=value" is three words in COMP_WORDS, while +# __%[1]s_init_completion puts it back together. The candidates are filtered against +# cur, which comes from there too, so a request built from anything else would ask the +# command about a different word than the one being completed. __%[1]s_build_completion_request() { - __cli_completion_request=("${COMP_WORDS[0]}" "__complete") - local i - for (( i = 1; i < COMP_CWORD; i++ )); do - __cli_completion_request+=("${COMP_WORDS[i]}") + + __%[1]s_dequote "${words[0]}" + __%[1]s_completion_request=("${__%[1]s_dequoted}" "__complete") + + for (( i = 1; i < cword; i++ )); do + __%[1]s_dequote "${words[i]}" + __%[1]s_completion_request+=("${__%[1]s_dequoted}") done - __cli_completion_request+=("${COMP_WORDS[COMP_CWORD]-}") + __%[1]s_dequote "${words[cword]-}" + __%[1]s_completion_request+=("${__%[1]s_dequoted}") } # Keep Bash 3 compatibility: associative arrays require Bash 4+, so @@ -58,7 +108,7 @@ __%[1]s_bash_autocomplete() { __%[1]s_init_completion -n "=:" || return __%[1]s_build_completion_request - opts=$("${__cli_completion_request[@]}" 2>/dev/null) + opts=$("${__%[1]s_completion_request[@]}" 2>/dev/null) # Completion output lines use "token:description" format. # Keep token/description in parallel arrays for Bash 3 compatibility. diff --git a/completion_test.go b/completion_test.go index 50b8a8ddd6..f8a388105f 100644 --- a/completion_test.go +++ b/completion_test.go @@ -281,9 +281,11 @@ func TestCompletionBashSendsTokenBeingCompleted(t *testing.T) { output, err := bashRender(cmd, "myapp") r.NoError(err) - r.Contains(output, `__cli_completion_request=("${COMP_WORDS[0]}" "__complete")`) - r.Contains(output, `__cli_completion_request+=("${COMP_WORDS[COMP_CWORD]-}")`) - r.Contains(output, `opts=$("${__cli_completion_request[@]}" 2>/dev/null)`) + r.Contains(output, `__myapp_completion_request=("${__myapp_dequoted}" "__complete")`) + r.Contains(output, `__myapp_dequote "${words[cword]-}"`) + r.Contains(output, `opts=$("${__myapp_completion_request[@]}" 2>/dev/null)`) + r.Contains(output, `for (( i = 1; i < cword; i++ )); do`, + "the request must come from the words __myapp_init_completion reassembled, not from COMP_WORDS") r.NotContains(output, `eval "`, "the request must not go through eval") r.NotContains(output, completionFlag, "the deprecated request form must not be generated") } From 489d2f1579e170355acdba792141cff664e7f92b Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 22:56:04 +0900 Subject: [PATCH 05/38] fix: take the quotes off the zsh request and drop the command's stderr The words arrive as typed, so "app run \"hello world\" " asked the app about a word holding the two quote characters. (Q) takes one level of quoting off each of them, which is what the shell would do before handing a word to a command. The command's stderr went to the terminal, where it lands in the middle of the prompt: the other three scripts redirect it, and this one now does too. --- autocomplete/zsh_autocomplete | 7 +++++-- completion_test.go | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/autocomplete/zsh_autocomplete b/autocomplete/zsh_autocomplete index 873da695aa..506e82b7c3 100644 --- a/autocomplete/zsh_autocomplete +++ b/autocomplete/zsh_autocomplete @@ -10,8 +10,11 @@ _%[1]s() { # the command line cannot turn it into a positional argument of whatever the # command runs. The word under the cursor is sent as the last argument, empty or # not, so that "cmd --" and "cmd -- " can be told apart. - request=("${words[1]}" "__complete" "${(@)words[2,CURRENT-1]}" "${words[CURRENT]}") - opts=("${(@f)$("${request[@]}")}") + # (Q) takes one level of quoting off each word, the way the shell would before + # handing it to a command, so that a quoted word reaches the command as the word + # it is rather than with its quotes. + request=("${(@Q)words[1]}" "__complete" "${(@Q)words[2,CURRENT-1]}" "${(@Q)words[CURRENT]}") + opts=("${(@f)$("${request[@]}" 2>/dev/null)}") if [[ "${opts[1]}" != "" ]]; then _describe 'values' opts diff --git a/completion_test.go b/completion_test.go index f8a388105f..1a881a1894 100644 --- a/completion_test.go +++ b/completion_test.go @@ -304,7 +304,9 @@ func TestCompletionZshSendsTokenBeingCompleted(t *testing.T) { output, err := zshRender(cmd, "myapp") r.NoError(err) - r.Contains(output, `request=("${words[1]}" "__complete" "${(@)words[2,CURRENT-1]}" "${words[CURRENT]}")`) + r.Contains(output, `request=("${(@Q)words[1]}" "__complete" "${(@Q)words[2,CURRENT-1]}" "${(@Q)words[CURRENT]}")`) + r.Contains(output, `opts=("${(@f)$("${request[@]}" 2>/dev/null)}")`, + "a command writing to stderr must not break the prompt") r.NotContains(output, completionFlag, "the deprecated request form must not be generated") } From 3974dfaacb8466f01efdf1d7c8fd303d2bcf6fab Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 22:58:30 +0900 Subject: [PATCH 06/38] fix: read the pwsh word being completed from the cursor The word under the cursor was found by comparing each element of the command line with $wordToComplete, which PowerShell hands over normalized: an unfinished "hello arrives as "hello", matches no element as written, and was therefore sent twice, once as a word of its own and once as the word being completed. The command was asked about a command line one argument longer than the one on screen, which is the opposite of what naming the word is for. The cursor says which element it is, and reading it also leaves out whatever follows: completing in the middle of a line asked about the words after the cursor too. The words come from the parsed element rather than from its source text, so one level of quoting comes off, the way the shell would take it off before handing a word to a command. Nothing is expanded while doing so: the value of a $(...) or a $env:VAR is no longer worked out to answer a completion, which Invoke-Expression used to do by re-parsing the whole line. --- autocomplete/powershell_autocomplete.ps1 | 51 ++++++++++++++++++------ completion_test.go | 4 +- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/autocomplete/powershell_autocomplete.ps1 b/autocomplete/powershell_autocomplete.ps1 index a54abecc36..14d7a82e68 100644 --- a/autocomplete/powershell_autocomplete.ps1 +++ b/autocomplete/powershell_autocomplete.ps1 @@ -3,27 +3,52 @@ $name = $fn -replace "(.*)\.ps1$", '$1' Register-ArgumentCompleter -Native -CommandName $name -ScriptBlock { param($wordToComplete, $commandAst, $cursorPosition) + # One level of quoting is taken off each word, the way the shell would before + # handing it to a command. The value is not expanded: nothing on the command line + # is evaluated to answer a completion, so a "$(...)" reaches the command as the + # text it is rather than being run by pressing the tab key. + function __cliCompletionText($element) { + if ($element -is [System.Management.Automation.Language.StringConstantExpressionAst] -or + $element -is [System.Management.Automation.Language.ExpandableStringExpressionAst]) { + return $element.Value + } + return $element.Extent.Text + } + + $elements = $commandAst.CommandElements + if ($elements.Count -eq 0) { + return + } + + # The command name itself is the shell's to complete, not the command's. + if ($cursorPosition -le $elements[0].Extent.EndOffset) { + return + } + # The request names the completion in its first argument, where a "--" typed on # the command line cannot turn it into a positional argument of whatever the # command runs. The word under the cursor is sent as the last argument, empty or # not, so that "cmd --" and "cmd -- " can be told apart. - $elements = @($commandAst.CommandElements | ForEach-Object { $_.ToString() }) - $command = $elements[0] + # + # Which word that is comes from the cursor rather than from a comparison with + # $wordToComplete, which PowerShell hands over normalized: an unfinished "hello + # arrives here as "hello", matches no element as written, and would be sent both + # as a word of its own and as the word being completed. Reading the cursor also + # leaves out what follows it, so completing in the middle of a line asks about + # the line up to that point. + $command = $elements[0].Extent.Text $words = @() - if ($elements.Count -gt 1) { - $words = $elements[1..($elements.Count - 1)] - } - # Once the word under the cursor has any character it is an element of its own, - # so it is dropped here and sent as the last argument instead. - if ($wordToComplete -and $words.Count -gt 0 -and $words[-1] -eq $wordToComplete) { - if ($words.Count -eq 1) { - $words = @() - } else { - $words = $words[0..($words.Count - 2)] + $word = '' + for ($i = 1; $i -lt $elements.Count; $i++) { + $extent = $elements[$i].Extent + if ($cursorPosition -gt $extent.StartOffset -and $cursorPosition -le $extent.EndOffset) { + $word = __cliCompletionText $elements[$i] + } elseif ($extent.EndOffset -lt $cursorPosition) { + $words += __cliCompletionText $elements[$i] } } - & $command __complete @words $wordToComplete 2>$null | ForEach-Object { + & $command __complete @words $word 2>$null | ForEach-Object { $parts = $_.Split(':', 2) if ($parts.Count -eq 2) { $completion = $parts[0].Trim() diff --git a/completion_test.go b/completion_test.go index 1a881a1894..f564aa8203 100644 --- a/completion_test.go +++ b/completion_test.go @@ -324,7 +324,9 @@ func TestCompletionPowershellSendsTokenBeingCompleted(t *testing.T) { output, err := pwshRender(cmd, "myapp") r.NoError(err) - r.Contains(output, `& $command __complete @words $wordToComplete 2>$null`) + r.Contains(output, `& $command __complete @words $word 2>$null`) + r.Contains(output, `if ($cursorPosition -gt $extent.StartOffset -and $cursorPosition -le $extent.EndOffset) {`, + "the word being completed must come from the cursor, not from a comparison with $wordToComplete") r.NotContains(output, completionFlag, "the deprecated request form must not be generated") } From b32595ebd997c1f85144920efc9265eeda8f418d Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 22:59:35 +0900 Subject: [PATCH 07/38] fix: send the same word from zsh as from bash and PowerShell A word whose quote is still open, as in 'app run "hello wo', has no closing quote for (Q) to take off with it, so zsh sent the opening quote along with the word. bash and PowerShell send "hello wo" without it for the same line, and a command filtering candidates by what has been typed finds nothing for a word that begins with a quote character. --- autocomplete/zsh_autocomplete | 11 ++++++++++- completion_test.go | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/autocomplete/zsh_autocomplete b/autocomplete/zsh_autocomplete index 506e82b7c3..80a8d1eb1a 100644 --- a/autocomplete/zsh_autocomplete +++ b/autocomplete/zsh_autocomplete @@ -13,7 +13,16 @@ _%[1]s() { # (Q) takes one level of quoting off each word, the way the shell would before # handing it to a command, so that a quoted word reaches the command as the word # it is rather than with its quotes. - request=("${(@Q)words[1]}" "__complete" "${(@Q)words[2,CURRENT-1]}" "${(@Q)words[CURRENT]}") + local raw="${words[CURRENT]}" + local current="${(Q)words[CURRENT]}" + # A word whose quote is still open has no closing quote to take off with it, so + # (Q) leaves it alone. Dropping the opening quote asks the command about the word + # being typed rather than about one starting with a quote character, which is what + # bash and PowerShell send for the same line. + if [[ "$current" == "$raw" && "$raw" == [\"\']* ]]; then + current="${current#[\"\']}" + fi + request=("${(@Q)words[1]}" "__complete" "${(@Q)words[2,CURRENT-1]}" "$current") opts=("${(@f)$("${request[@]}" 2>/dev/null)}") if [[ "${opts[1]}" != "" ]]; then diff --git a/completion_test.go b/completion_test.go index f564aa8203..1f1c48c532 100644 --- a/completion_test.go +++ b/completion_test.go @@ -304,7 +304,7 @@ func TestCompletionZshSendsTokenBeingCompleted(t *testing.T) { output, err := zshRender(cmd, "myapp") r.NoError(err) - r.Contains(output, `request=("${(@Q)words[1]}" "__complete" "${(@Q)words[2,CURRENT-1]}" "${(@Q)words[CURRENT]}")`) + r.Contains(output, `request=("${(@Q)words[1]}" "__complete" "${(@Q)words[2,CURRENT-1]}" "$current")`) r.Contains(output, `opts=("${(@f)$("${request[@]}" 2>/dev/null)}")`, "a command writing to stderr must not break the prompt") r.NotContains(output, completionFlag, "the deprecated request form must not be generated") From 582e040fab6b317230f208a9170bac6b4c7b03d8 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 23:03:46 +0900 Subject: [PATCH 08/38] test: run the completion scripts in the shells they are written for The script tests asserted on the text of the generated script, which says that a line was written, not what the shell does with it. Every difference found while reviewing this branch lived in that gap: a word bash splits on COMP_WORDBREAKS, a word arriving with its quotes, a word PowerShell hands over normalized and so sent twice. Each script is now sourced in its shell and asked to complete a command line, with the command it asks recording the arguments it receives. A shell that is not installed skips, so this costs nothing on a machine without it, and -short skips all of them. It found one more difference while being written: fish sent the word being completed with the quote the user had opened, where the other three take it off. --- autocomplete/fish_autocomplete | 6 +- completion_shell_test.go | 266 +++++++++++++++++++++++++++++++++ completion_test.go | 1 + 3 files changed, 271 insertions(+), 2 deletions(-) create mode 100644 completion_shell_test.go diff --git a/autocomplete/fish_autocomplete b/autocomplete/fish_autocomplete index 0363d39883..ccb6ca4ce6 100644 --- a/autocomplete/fish_autocomplete +++ b/autocomplete/fish_autocomplete @@ -3,8 +3,10 @@ function __%[1]s_perform_completion # Extract all args except the last one set -l args (commandline -opc) - # Extract the last arg (partial input) - set -l lastArg (commandline -ct) + # Extract the last arg (partial input), with one level of quoting taken off the + # way the shell would take it off before handing a word to a command. The words + # before it come tokenized, which does that already. + set -l lastArg (string unescape -- (commandline -ct)) # The request names the completion in its first argument, where a "--" typed on # the command line cannot turn it into a positional argument of whatever the diff --git a/completion_shell_test.go b/completion_shell_test.go new file mode 100644 index 0000000000..909d7484c8 --- /dev/null +++ b/completion_shell_test.go @@ -0,0 +1,266 @@ +//go:build !windows + +package cli + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// This file checks what the generated completion scripts actually send, by running +// them in the shells they are written for and recording the arguments the command +// receives. Asserting on the text of a script only says that it was generated; a word +// the shell splits, quotes or normalizes differently is a difference the text cannot +// show. +// +// Every shell is skipped when it is not installed, so this adds nothing to a machine +// that has none of them. + +// completionCase is a command line, with the cursor at its end unless the line ends +// in a space, and the arguments the command is expected to receive for it. +type completionCase struct { + name string + line string + want []string +} + +func completionCases() []completionCase { + return []completionCase{ + { + name: "a word being typed", + line: "app su", + want: []string{"__complete", "su"}, + }, + { + name: "a fresh word", + line: "app sub ", + want: []string{"__complete", "sub", ""}, + }, + { + name: "a flag being typed", + line: "app sub --fl", + want: []string{"__complete", "sub", "--fl"}, + }, + { + // COMP_WORDBREAKS holds "=", so bash splits this into three words and has + // to put them back together before asking. + name: "a flag holding its value", + line: "app --opt=va", + want: []string{"__complete", "--opt=va"}, + }, + { + // One level of quoting comes off, the way the shell takes it off before + // handing a word to a command. + name: "a quoted word", + line: `app sub "hello world" `, + want: []string{"__complete", "sub", "hello world", ""}, + }, + { + // The quote is still open, so there is no closing quote to take off with + // it. The word is what is being typed, not one starting with a quote. + name: "a word whose quote is still open", + line: `app sub "hello wo`, + want: []string{"__complete", "sub", "hello wo"}, + }, + { + // Everything after "--" is a positional argument of whatever the command + // runs, and the command is asked about it rather than running it. + name: "past a double dash", + line: "app exec -- git push ", + want: []string{"__complete", "exec", "--", "git", "push", ""}, + }, + { + // Answering a completion must not evaluate the command line. The old + // scripts re-parsed it, so a command substitution ran on the tab key. + name: "a command substitution", + line: "app sub $(touch NOPE) ", + want: []string{"__complete", "sub", "$(touch NOPE)", ""}, + }, + } +} + +// TestCompletionScriptsRequest runs the generated scripts in the shells they are +// written for and checks the request each one builds. +func TestCompletionScriptsRequest(t *testing.T) { + if testing.Short() { + t.Skip("driving four shells takes seconds, not milliseconds") + } + + t.Parallel() + + for _, shell := range []string{"bash", "zsh", "fish", "pwsh"} { + t.Run(shell, func(t *testing.T) { + t.Parallel() + + driver := shellDrivers[shell] + interpreter, err := exec.LookPath(driver.interpreter) + if err != nil { + t.Skipf("%s is not installed", driver.interpreter) + } + + render := shellCompletions[shell] + require.NotNil(t, render) + script, err := render(&Command{Name: "app", EnableShellCompletion: true}, "app") + require.NoError(t, err) + + for _, tc := range completionCases() { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + scriptPath := filepath.Join(dir, "completion."+shell) + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o644)) + + argvPath := filepath.Join(dir, "argv") + writeCompletionTestApp(t, dir) + + prelude := driver.prelude(t, interpreter) + program := driver.program(scriptPath, tc.line) + + cmd := exec.Command(interpreter, driver.args(prelude+program)...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "PATH="+dir+string(os.PathListSeparator)+os.Getenv("PATH"), + "ARGV_LOG="+argvPath, + ) + out, err := cmd.CombinedOutput() + require.NoError(t, err, "driving %s: %s", shell, out) + + got, err := os.ReadFile(argvPath) + require.NoError(t, err, "the completion did not run the command: %s", out) + assert.Equal(t, tc.want, strings.Split(strings.TrimSuffix(string(got), "\n"), "\n")) + + assert.NoFileExists(t, filepath.Join(dir, "NOPE"), + "the command line must not be evaluated to answer a completion") + }) + } + }) + } +} + +// writeCompletionTestApp writes the command the completion scripts ask, which records +// the arguments it receives and answers with one candidate. +func writeCompletionTestApp(t *testing.T, dir string) { + t.Helper() + app := "#!/bin/sh\n: > \"$ARGV_LOG\"\nfor a in \"$@\"; do printf '%s\\n' \"$a\" >> \"$ARGV_LOG\"; done\necho candidate\n" + require.NoError(t, os.WriteFile(filepath.Join(dir, "app"), []byte(app), 0o755)) +} + +// shellDriver runs a completion the way its shell would, without a terminal. Each +// shell offers its own way in: what they have in common is that the script under test +// is sourced and the completion for a command line is asked for. +type shellDriver struct { + interpreter string + args func(program string) []string + prelude func(t *testing.T, interpreter string) string + program func(scriptPath, line string) string +} + +var shellDrivers = map[string]shellDriver{ + "bash": { + interpreter: "bash", + args: func(p string) []string { return []string{"-c", p} }, + // The script calls the word-splitting helpers of bash-completion, so without + // it there is nothing to drive. + prelude: func(t *testing.T, _ string) string { + t.Helper() + for _, p := range []string{ + "/usr/share/bash-completion/bash_completion", + "/etc/bash_completion", + "/opt/homebrew/share/bash-completion/bash_completion", + "/usr/local/share/bash-completion/bash_completion", + } { + if _, err := os.Stat(p); err == nil { + return ". " + p + "\n" + } + } + t.Skip("bash-completion is not installed") + return "" + }, + program: func(scriptPath, line string) string { + // COMP_WORDS and COMP_CWORD are what bash sets before it calls the + // completion function, which is what the script reads. + return fmt.Sprintf(` +. %s +line=%s +eval "COMP_WORDS=($line)" +[ "${line: -1}" = " " ] && COMP_WORDS+=("") +COMP_CWORD=$(( ${#COMP_WORDS[@]} - 1 )) +COMP_LINE="$line" +COMP_POINT=${#line} +__app_bash_autocomplete +`, shQuote(scriptPath), shQuote(line)) + }, + }, + "zsh": { + interpreter: "zsh", + args: func(p string) []string { return []string{"-f", "-c", p} }, + prelude: func(*testing.T, string) string { return "" }, + // The completion system is not started, so the parts of it the script uses + // stand in for it: what is under test is the request the script builds from + // words and CURRENT, which zsh fills the same way here. + program: func(scriptPath, line string) string { + return fmt.Sprintf(` +compdef() { : } +_describe() { : } +_files() { : } +. %s +line=%s +words=("${(z)line}") +[[ "$line" == *" " ]] && words+=("") +CURRENT=$#words +_app +`, shQuote(scriptPath), shQuote(line)) + }, + }, + "fish": { + interpreter: "fish", + args: func(p string) []string { return []string{"-c", p} }, + prelude: func(*testing.T, string) string { return "" }, + // complete -C asks for the completions of a command line, which is the entry + // point fish itself uses. + program: func(scriptPath, line string) string { + return fmt.Sprintf("source %s\ncomplete -C %s\n", fishQuote(scriptPath), fishQuote(line)) + }, + }, + "pwsh": { + interpreter: "pwsh", + args: func(p string) []string { return []string{"-NoProfile", "-Command", p} }, + prelude: func(*testing.T, string) string { return "" }, + // The script registers its completer under the name of the file it is in, so + // its body is registered directly here. TabExpansion2 is what PowerShell calls + // on the tab key. + program: func(scriptPath, line string) string { + return fmt.Sprintf(` +$script = Get-Content %s -Raw +$start = $script.IndexOf('-ScriptBlock {') + '-ScriptBlock {'.Length +$body = $script.Substring($start, $script.LastIndexOf('}') - $start) +Register-ArgumentCompleter -Native -CommandName app -ScriptBlock ([scriptblock]::Create($body)) +$line = %s +$null = TabExpansion2 -inputScript $line -cursorColumn $line.Length +`, pwshQuote(scriptPath), pwshQuote(line)) + }, + }, +} + +// shQuote quotes s for bash and zsh. +func shQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" +} + +// fishQuote quotes s for fish. +func fishQuote(s string) string { + return "'" + strings.NewReplacer(`\`, `\\`, "'", `\'`).Replace(s) + "'" +} + +// pwshQuote quotes s for PowerShell. +func pwshQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", "''") + "'" +} diff --git a/completion_test.go b/completion_test.go index 1f1c48c532..495cea8f46 100644 --- a/completion_test.go +++ b/completion_test.go @@ -259,6 +259,7 @@ func TestCompletionFishSendsTokenBeingCompleted(t *testing.T) { output, err := fishRender(cmd, "myapp") r.NoError(err) + r.Contains(output, `set -l lastArg (string unescape -- (commandline -ct))`) r.Contains(output, `set results ($args[1] __complete $args[2..-1] "$lastArg" 2> /dev/null)`) r.NotContains(output, completionFlag, "the deprecated request form must not be generated") } From 2bc0af43f9e1ccd4d40a392d0c6dc6b5791e8771 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 23:04:45 +0900 Subject: [PATCH 09/38] test: drop the -short guard that panicked in the full run Tests in this package add flags of their own to the standard flag set, so testing.Short finds it unparsed and panics. It passed under -run because nothing had touched the flag set by then. --- completion_shell_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/completion_shell_test.go b/completion_shell_test.go index 909d7484c8..83f8c839cd 100644 --- a/completion_shell_test.go +++ b/completion_shell_test.go @@ -89,10 +89,11 @@ func completionCases() []completionCase { // TestCompletionScriptsRequest runs the generated scripts in the shells they are // written for and checks the request each one builds. func TestCompletionScriptsRequest(t *testing.T) { - if testing.Short() { - t.Skip("driving four shells takes seconds, not milliseconds") - } - + // testing.Short is not read here: tests in this package add flags of their own to + // the standard flag set, which leaves testing.Short panicking on a flag set that + // has not been parsed. The shells run in parallel and each one skips when it is + // not installed, so the cost of leaving it in is a few seconds on a machine that + // has all four. t.Parallel() for _, shell := range []string{"bash", "zsh", "fish", "pwsh"} { From 288c900e8ef7eebfd218d39064cc5b2a9afb7cad Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 23:05:16 +0900 Subject: [PATCH 10/38] docs: say what "__complete" reserves and what an old script now does Setting EnableShellCompletion takes the first argument for a completion request, which an app taking free-form positional arguments no longer receives. Only an app declaring a command of that name was said to be affected, which is the guard, not the reservation. A script generated before this change also behaves differently on a command line holding a "--": the request stays on the line as a positional argument, so the run it starts usually ends in an Incorrect Usage message. --- docs/v3/examples/completions/shell-completions.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/v3/examples/completions/shell-completions.md b/docs/v3/examples/completions/shell-completions.md index bcb31d7a63..f573476b9b 100644 --- a/docs/v3/examples/completions/shell-completions.md +++ b/docs/v3/examples/completions/shell-completions.md @@ -117,6 +117,14 @@ The procedure for other shells is similar to bash though the specific paths for shells may vary. Some of the sections below detail the setup need for other shells as well as examples in those shells. +#### `__complete` is reserved + +Setting `EnableShellCompletion` reserves `__complete` as the first argument of your app: a run +starting with it is answered as a completion request rather than passed on, and the words after it +are read as the command line being completed. An app that takes free-form positional arguments +therefore cannot receive `__complete` as its first one. An app that declares a command of that name +keeps it, and stops being completable in exchange. + #### Regenerate the script after upgrading Completion scripts generated before urfave/cli asked for completions with `__complete` end their request @@ -128,6 +136,10 @@ belongs to whatever the app runs rather than to the app itself, and the app runs resolves that: `__complete` is the first argument, where a `--` typed later on the command line can no longer turn it into a positional argument. +Until the script is regenerated, pressing tab on such a line runs the app with the request left on it +as a positional argument, which usually ends in an `Incorrect Usage` message rather than in the run +completing quietly. + #### Default auto-completion ```go From a8df3be2cffd4892b1d68a2571a415cf361150b4 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 23:06:12 +0900 Subject: [PATCH 11/38] test: cover a disabled app and a nested command Nothing said that an app which has not enabled shell completion receives "__complete" as the positional argument it is, which is the other half of reserving it. Nothing reached past one level of subcommands either. The last case there is the one the arguments alone cannot answer: a flag being completed after a positional argument, where the deprecated request form looks one word too far back and suggests commands instead. --- completion_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/completion_test.go b/completion_test.go index 495cea8f46..a92703d345 100644 --- a/completion_test.go +++ b/completion_test.go @@ -876,3 +876,60 @@ func TestCompletionCustomShellCompleteNotRunPastDoubleDash(t *testing.T) { r.True(ran) r.Equal("custom\n", out.String()) } + +// TestCompletionRequestIgnoredWhenDisabled checks that the request form means nothing +// to an app that has not enabled shell completion: the first argument reaches it as +// the positional argument it wrote. +func TestCompletionRequestIgnoredWhenDisabled(t *testing.T) { + var got []string + out := &bytes.Buffer{} + cmd := &Command{ + Writer: out, + Action: func(_ context.Context, cmd *Command) error { + got = cmd.Args().Slice() + return nil + }, + } + + r := require.New(t) + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "bar"})) + r.Equal([]string{completionCommandRequest, "bar"}, got) + r.Empty(out.String()) +} + +// TestCompletionRequestNestedSubcommand checks that a request is answered by the +// command it names however deep that is, rather than by the one above it. +func TestCompletionRequestNestedSubcommand(t *testing.T) { + out := &bytes.Buffer{} + cmd := &Command{ + EnableShellCompletion: true, + Writer: out, + Commands: []*Command{ + { + Name: "one", + Commands: []*Command{ + { + Name: "two", + Flags: []Flag{&BoolFlag{Name: "deep"}}, + Action: func(context.Context, *Command) error { return nil }, + }, + }, + }, + }, + } + + r := require.New(t) + + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "one", ""})) + r.Equal("two\nhelp:Shows a list of commands or help for one command\n", out.String()) + + out.Reset() + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "one", "two", "-"})) + r.Equal("--deep\n--help:show help\n", out.String()) + + // The word being completed is a flag of the command it follows, even with a + // positional argument in between, which the arguments alone could not say. + out.Reset() + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "one", "two", "arg", "--de"})) + r.Equal("--deep\n", out.String()) +} From f506b1834da189c2e3b245da5a29faa511fb1687 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 23:23:22 +0900 Subject: [PATCH 12/38] test: give the bash driver the words bash actually produces The driver built COMP_WORDS with eval, which is not what bash does with a command line and got all three of the cases that matter wrong. eval took the quotes off before the script saw them, so the case named "a quoted word" never reached the script's dequoting and would pass with it deleted. eval joined "--opt=va" into one word, so the reassembly the case is named after was never exercised. And eval ran the command substitution the case is there to prove is not run, creating the file the assertion looks for: on a machine with bash-completion, that case failed. The words are now written out as bash produces them, measured with a completion function that dumps COMP_WORDS, so what the driver feeds the script is what bash would. --- completion_shell_test.go | 104 +++++++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 42 deletions(-) diff --git a/completion_shell_test.go b/completion_shell_test.go index 83f8c839cd..5478841d76 100644 --- a/completion_shell_test.go +++ b/completion_shell_test.go @@ -28,60 +28,74 @@ import ( type completionCase struct { name string line string - want []string + // bashWords is what bash puts in COMP_WORDS for line, with the cursor at its end. + // Bash splits on COMP_WORDBREAKS and keeps the quoting as typed, and a driver that + // works the words out for itself tests its own idea of that rather than the + // script's: these were measured in bash 5.3 with a completion function that dumps + // COMP_WORDS. + bashWords []string + want []string } func completionCases() []completionCase { return []completionCase{ { - name: "a word being typed", - line: "app su", - want: []string{"__complete", "su"}, + name: "a word being typed", + line: "app su", + bashWords: []string{"app", "su"}, + want: []string{"__complete", "su"}, }, { - name: "a fresh word", - line: "app sub ", - want: []string{"__complete", "sub", ""}, + name: "a fresh word", + line: "app sub ", + bashWords: []string{"app", "sub", ""}, + want: []string{"__complete", "sub", ""}, }, { - name: "a flag being typed", - line: "app sub --fl", - want: []string{"__complete", "sub", "--fl"}, + name: "a flag being typed", + line: "app sub --fl", + bashWords: []string{"app", "sub", "--fl"}, + want: []string{"__complete", "sub", "--fl"}, }, { // COMP_WORDBREAKS holds "=", so bash splits this into three words and has // to put them back together before asking. - name: "a flag holding its value", - line: "app --opt=va", - want: []string{"__complete", "--opt=va"}, + name: "a flag holding its value", + line: "app --opt=va", + bashWords: []string{"app", "--opt", "=", "va"}, + want: []string{"__complete", "--opt=va"}, }, { // One level of quoting comes off, the way the shell takes it off before // handing a word to a command. - name: "a quoted word", - line: `app sub "hello world" `, - want: []string{"__complete", "sub", "hello world", ""}, + name: "a quoted word", + line: `app sub "hello world" `, + bashWords: []string{"app", "sub", `"hello world"`, ""}, + want: []string{"__complete", "sub", "hello world", ""}, }, { // The quote is still open, so there is no closing quote to take off with // it. The word is what is being typed, not one starting with a quote. - name: "a word whose quote is still open", - line: `app sub "hello wo`, - want: []string{"__complete", "sub", "hello wo"}, + name: "a word whose quote is still open", + line: `app sub "hello wo`, + bashWords: []string{"app", "sub", `"hello wo`}, + want: []string{"__complete", "sub", "hello wo"}, }, { // Everything after "--" is a positional argument of whatever the command // runs, and the command is asked about it rather than running it. - name: "past a double dash", - line: "app exec -- git push ", - want: []string{"__complete", "exec", "--", "git", "push", ""}, + name: "past a double dash", + line: "app exec -- git push ", + bashWords: []string{"app", "exec", "--", "git", "push", ""}, + want: []string{"__complete", "exec", "--", "git", "push", ""}, }, { // Answering a completion must not evaluate the command line. The old // scripts re-parsed it, so a command substitution ran on the tab key. - name: "a command substitution", - line: "app sub $(touch NOPE) ", - want: []string{"__complete", "sub", "$(touch NOPE)", ""}, + name: "a command substitution", + line: "app sub $(touch NOPE) ", + bashWords: []string{"app", "sub", "$(touch NOPE)", ""}, + want: []string{"__complete", "sub", "$(touch NOPE)", ""}, }, } } @@ -123,7 +137,7 @@ func TestCompletionScriptsRequest(t *testing.T) { writeCompletionTestApp(t, dir) prelude := driver.prelude(t, interpreter) - program := driver.program(scriptPath, tc.line) + program := driver.program(scriptPath, tc) cmd := exec.Command(interpreter, driver.args(prelude+program)...) cmd.Dir = dir @@ -161,7 +175,7 @@ type shellDriver struct { interpreter string args func(program string) []string prelude func(t *testing.T, interpreter string) string - program func(scriptPath, line string) string + program func(scriptPath string, tc completionCase) string } var shellDrivers = map[string]shellDriver{ @@ -185,19 +199,25 @@ var shellDrivers = map[string]shellDriver{ t.Skip("bash-completion is not installed") return "" }, - program: func(scriptPath, line string) string { + program: func(scriptPath string, tc completionCase) string { // COMP_WORDS and COMP_CWORD are what bash sets before it calls the - // completion function, which is what the script reads. + // completion function, which is what the script reads. They are written + // out as measured rather than worked out here: quoting them apart or + // letting eval build them would test this driver's idea of what bash does + // with a command line instead of the script's handling of what bash + // actually produces, and eval would run a command substitution on the way. + words := make([]string, 0, len(tc.bashWords)) + for _, w := range tc.bashWords { + words = append(words, shQuote(w)) + } return fmt.Sprintf(` . %s -line=%s -eval "COMP_WORDS=($line)" -[ "${line: -1}" = " " ] && COMP_WORDS+=("") -COMP_CWORD=$(( ${#COMP_WORDS[@]} - 1 )) -COMP_LINE="$line" -COMP_POINT=${#line} +COMP_WORDS=(%s) +COMP_CWORD=%d +COMP_LINE=%s +COMP_POINT=%d __app_bash_autocomplete -`, shQuote(scriptPath), shQuote(line)) +`, shQuote(scriptPath), strings.Join(words, " "), len(tc.bashWords)-1, shQuote(tc.line), len(tc.line)) }, }, "zsh": { @@ -207,7 +227,7 @@ __app_bash_autocomplete // The completion system is not started, so the parts of it the script uses // stand in for it: what is under test is the request the script builds from // words and CURRENT, which zsh fills the same way here. - program: func(scriptPath, line string) string { + program: func(scriptPath string, tc completionCase) string { return fmt.Sprintf(` compdef() { : } _describe() { : } @@ -218,7 +238,7 @@ words=("${(z)line}") [[ "$line" == *" " ]] && words+=("") CURRENT=$#words _app -`, shQuote(scriptPath), shQuote(line)) +`, shQuote(scriptPath), shQuote(tc.line)) }, }, "fish": { @@ -227,8 +247,8 @@ _app prelude: func(*testing.T, string) string { return "" }, // complete -C asks for the completions of a command line, which is the entry // point fish itself uses. - program: func(scriptPath, line string) string { - return fmt.Sprintf("source %s\ncomplete -C %s\n", fishQuote(scriptPath), fishQuote(line)) + program: func(scriptPath string, tc completionCase) string { + return fmt.Sprintf("source %s\ncomplete -C %s\n", fishQuote(scriptPath), fishQuote(tc.line)) }, }, "pwsh": { @@ -238,7 +258,7 @@ _app // The script registers its completer under the name of the file it is in, so // its body is registered directly here. TabExpansion2 is what PowerShell calls // on the tab key. - program: func(scriptPath, line string) string { + program: func(scriptPath string, tc completionCase) string { return fmt.Sprintf(` $script = Get-Content %s -Raw $start = $script.IndexOf('-ScriptBlock {') + '-ScriptBlock {'.Length @@ -246,7 +266,7 @@ $body = $script.Substring($start, $script.LastIndexOf('}') - $start) Register-ArgumentCompleter -Native -CommandName app -ScriptBlock ([scriptblock]::Create($body)) $line = %s $null = TabExpansion2 -inputScript $line -cursorColumn $line.Length -`, pwshQuote(scriptPath), pwshQuote(line)) +`, pwshQuote(scriptPath), pwshQuote(tc.line)) }, }, } From 6df5887c3f20157d181a02fc78fbaae471eafec0 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 23:24:40 +0900 Subject: [PATCH 13/38] ci: install the shells the completion scripts are run in Nothing installed bash-completion, zsh or fish, so the tests that run the generated scripts skipped, and a skip reads the same as a pass. The ubuntu job installs them and sets CLI_SHELL_TESTS_REQUIRED, which turns "this shell is not here" into a failure, so the coverage cannot go away without anyone noticing. Elsewhere the variable is unset and a missing shell still skips. --- .github/workflows/test.yml | 8 ++++++++ completion_shell_test.go | 15 +++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0349d7c984..befa8324f8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -34,6 +34,12 @@ jobs: - name: Set PATH run: echo "${GITHUB_WORKSPACE}/.local/bin" >>"${GITHUB_PATH}" + # The completion scripts are run in these to check what they send. Without + # them those tests skip, which is silent: CLI_SHELL_TESTS_REQUIRED below turns + # a skip into a failure so that the coverage cannot go away unnoticed. + - if: matrix.os == 'ubuntu-24.04' + run: sudo apt-get update && sudo apt-get install -y bash-completion zsh fish + - if: matrix.go == 'stable' && matrix.os == 'ubuntu-24.04' run: make ensure-goimports @@ -42,6 +48,8 @@ jobs: - run: make vet - run: make test + env: + CLI_SHELL_TESTS_REQUIRED: ${{ matrix.os == 'ubuntu-24.04' && '1' || '' }} - run: make check-binary-size - if: matrix.go == 'stable' && matrix.os == 'ubuntu-24.04' diff --git a/completion_shell_test.go b/completion_shell_test.go index 5478841d76..a61d0ac0d3 100644 --- a/completion_shell_test.go +++ b/completion_shell_test.go @@ -117,7 +117,7 @@ func TestCompletionScriptsRequest(t *testing.T) { driver := shellDrivers[shell] interpreter, err := exec.LookPath(driver.interpreter) if err != nil { - t.Skipf("%s is not installed", driver.interpreter) + skipMissingShell(t, driver.interpreter+" is not installed") } render := shellCompletions[shell] @@ -160,6 +160,17 @@ func TestCompletionScriptsRequest(t *testing.T) { } } +// skipMissingShell skips a shell that is not installed, unless the environment says +// these tests are expected to run. A skip is silent, and a machine that has none of +// the four reports the same green as one where every request is right. +func skipMissingShell(t *testing.T, reason string) { + t.Helper() + if os.Getenv("CLI_SHELL_TESTS_REQUIRED") != "" { + t.Fatalf("CLI_SHELL_TESTS_REQUIRED is set: %s", reason) + } + t.Skip(reason) +} + // writeCompletionTestApp writes the command the completion scripts ask, which records // the arguments it receives and answers with one candidate. func writeCompletionTestApp(t *testing.T, dir string) { @@ -196,7 +207,7 @@ var shellDrivers = map[string]shellDriver{ return ". " + p + "\n" } } - t.Skip("bash-completion is not installed") + skipMissingShell(t, "bash-completion is not installed") return "" }, program: func(scriptPath string, tc completionCase) string { From 4383d6ef8505b76cf46d78a201d499a76c1803e0 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 23:26:29 +0900 Subject: [PATCH 14/38] fix: take the quotes off the pwsh command name too Every word but the first went through the dequoting, so a command whose path holds a space, and is therefore typed quoted, was run under a name with the quote characters in it: nothing ran and the completion returned empty. bash takes them off words[0] the same way. --- autocomplete/powershell_autocomplete.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autocomplete/powershell_autocomplete.ps1 b/autocomplete/powershell_autocomplete.ps1 index 14d7a82e68..969117d8da 100644 --- a/autocomplete/powershell_autocomplete.ps1 +++ b/autocomplete/powershell_autocomplete.ps1 @@ -36,7 +36,7 @@ Register-ArgumentCompleter -Native -CommandName $name -ScriptBlock { # as a word of its own and as the word being completed. Reading the cursor also # leaves out what follows it, so completing in the middle of a line asks about # the line up to that point. - $command = $elements[0].Extent.Text + $command = __cliCompletionText $elements[0] $words = @() $word = '' for ($i = 1; $i -lt $elements.Count; $i++) { From e27be3e2a07a6d2d66a116c036c1d595d709f648 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 23:27:17 +0900 Subject: [PATCH 15/38] fix: do not run Before for a request past a "--" The completion func stopped being run past the terminator, but the Before chain in front of it did not: a Before with a side effect fired on every tab key there, to produce an answer that is always empty. There is nothing to prepare for a completion that is not going to happen. --- command_run.go | 14 ++++++++++---- completion_test.go | 32 ++++++++++++++++++++++++++++++++ help.go | 23 ++++++++++++----------- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/command_run.go b/command_run.go index abff2533f8..2c706b5ec7 100644 --- a/command_run.go +++ b/command_run.go @@ -165,11 +165,17 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context tracef("using post-parse arguments %[1]q (cmd=%[2]q)", args, cmd.Name) if shouldRunCompletion(cmd) { - var beforeErr error - if ctx, beforeErr = runBefore(ctx, commandChain(cmd)); beforeErr != nil { - return ctx, beforeErr + // Everything after "--" is a positional argument of whatever the command runs, + // so there is no completion to run and nothing to prepare for one: a Before + // with a side effect would otherwise fire on every tab key past the + // terminator, for an answer that is always empty. + if !cmd.Root().completionTerminated() { + var beforeErr error + if ctx, beforeErr = runBefore(ctx, commandChain(cmd)); beforeErr != nil { + return ctx, beforeErr + } + runCompletion(ctx, cmd) } - runCompletion(ctx, cmd) return ctx, nil } diff --git a/completion_test.go b/completion_test.go index a92703d345..9d30180bcb 100644 --- a/completion_test.go +++ b/completion_test.go @@ -933,3 +933,35 @@ func TestCompletionRequestNestedSubcommand(t *testing.T) { r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "one", "two", "arg", "--de"})) r.Equal("--deep\n", out.String()) } + +// TestCompletionBeforeNotRunPastDoubleDash checks that a Before is not run for a +// request past a "--": there is no completion to prepare for, since the words there +// belong to whatever the command runs, and a Before with a side effect would fire on +// every tab key for an answer that is always empty. +func TestCompletionBeforeNotRunPastDoubleDash(t *testing.T) { + ran := 0 + out := &bytes.Buffer{} + cmd := &Command{ + EnableShellCompletion: true, + Writer: out, + Before: func(ctx context.Context, _ *Command) (context.Context, error) { + ran++ + return ctx, nil + }, + Commands: []*Command{ + { + Name: "exec", + Action: func(context.Context, *Command) error { return nil }, + }, + }, + } + + r := require.New(t) + + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "exec", "--", "git", "pu"})) + r.Zero(ran, "Before must not run for a request past a double dash") + r.Empty(out.String()) + + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", completionCommandRequest, "exec", "pu"})) + r.Equal(1, ran, "Before still runs for a request the command answers") +} diff --git a/help.go b/help.go index 140ef3ef33..12fa8e0b6a 100644 --- a/help.go +++ b/help.go @@ -606,18 +606,19 @@ func shouldRunCompletion(cmd *Command) bool { return true } -func runCompletion(ctx context.Context, cmd *Command) { - // Everything after "--" is a positional argument of whatever the command runs, so - // this command has no suggestion for it. Answering with nothing here rather than - // in the completion func applies that to every command, including one carrying a - // ShellComplete of its own, which would otherwise have to know about "--" itself - // to keep the promise the terminator makes. - // https://unix.stackexchange.com/a/11382 - if req := cmd.Root().completion; req != nil && req.terminated { - tracef("not suggesting past a \"--\" on command %[1]q", cmd.Name) - return - } +// completionTerminated reports whether the completion request being answered has a +// "--" before the word being completed. The words there are positional arguments of +// whatever the command runs, so this command has no suggestion for them, and its +// completion func is not run at all: keeping that here rather than in the func +// applies it to every command, including one carrying a ShellComplete of its own, +// which would otherwise have to know about a "--" the parsed arguments no longer show. +// https://unix.stackexchange.com/a/11382 +func (cmd *Command) completionTerminated() bool { + req := cmd.Root().completion + return req != nil && req.terminated +} +func runCompletion(ctx context.Context, cmd *Command) { if cmd.ShellComplete != nil { tracef("running shell completion func for command %[1]q", cmd.Name) cmd.ShellComplete(ctx, cmd) From da7275bd9ba492e5a8c8fbcd610738026f32baa7 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 23:27:44 +0900 Subject: [PATCH 16/38] docs: say that nothing is completed after a "--" Not running the completion func past the terminator makes "no suggestion there" a guarantee of the library rather than what the default func happened to do, and it means a command wrapping another cannot hand its completions on. Neither was written down. --- docs/v3/examples/completions/shell-completions.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/v3/examples/completions/shell-completions.md b/docs/v3/examples/completions/shell-completions.md index f573476b9b..ddab1dd9a8 100644 --- a/docs/v3/examples/completions/shell-completions.md +++ b/docs/v3/examples/completions/shell-completions.md @@ -125,6 +125,14 @@ are read as the command line being completed. An app that takes free-form positi therefore cannot receive `__complete` as its first one. An app that declares a command of that name keeps it, and stops being completable in exchange. +#### Nothing is completed after a `--` + +The words after a `--` are positional arguments of whatever your app runs with them, so urfave/cli +answers a request for one with no candidates and does not run your `ShellComplete` at all. A command +that wraps another one therefore cannot hand its completions on: `myapp exec -- git pu` offers +nothing rather than what `git` would offer. What it does do is leave your app's action alone, which +is what a shell asking for completions needs. + #### Regenerate the script after upgrading Completion scripts generated before urfave/cli asked for completions with `__complete` end their request From 77c2a0cdaa585e9a5876d2a5a73a3fece3c883f8 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Thu, 6 Aug 2026 23:45:21 +0900 Subject: [PATCH 17/38] ci: require the shells the job installs, not every shell CLI_SHELL_TESTS_REQUIRED turned a missing shell into a failure, whatever it was, and the ubuntu job installs three of the four: pwsh comes with the runner image, so a change of image would have failed the job over a shell nobody had asked it to cover. It now names the shells it requires. --- .github/workflows/test.yml | 9 ++++++--- completion_shell_test.go | 22 ++++++++++++++-------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index befa8324f8..b5492da6f2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -35,8 +35,8 @@ jobs: run: echo "${GITHUB_WORKSPACE}/.local/bin" >>"${GITHUB_PATH}" # The completion scripts are run in these to check what they send. Without - # them those tests skip, which is silent: CLI_SHELL_TESTS_REQUIRED below turns - # a skip into a failure so that the coverage cannot go away unnoticed. + # them those tests skip, which is silent: CLI_SHELL_TESTS_REQUIRED below names + # them so that a skip is a failure and the coverage cannot go away unnoticed. - if: matrix.os == 'ubuntu-24.04' run: sudo apt-get update && sudo apt-get install -y bash-completion zsh fish @@ -49,7 +49,10 @@ jobs: - run: make vet - run: make test env: - CLI_SHELL_TESTS_REQUIRED: ${{ matrix.os == 'ubuntu-24.04' && '1' || '' }} + # The shells installed above, and only those: pwsh comes with the + # runner image, so requiring it would turn a change of image into a + # failure here. + CLI_SHELL_TESTS_REQUIRED: ${{ matrix.os == 'ubuntu-24.04' && 'bash,zsh,fish' || '' }} - run: make check-binary-size - if: matrix.go == 'stable' && matrix.os == 'ubuntu-24.04' diff --git a/completion_shell_test.go b/completion_shell_test.go index a61d0ac0d3..9797437d6d 100644 --- a/completion_shell_test.go +++ b/completion_shell_test.go @@ -117,7 +117,7 @@ func TestCompletionScriptsRequest(t *testing.T) { driver := shellDrivers[shell] interpreter, err := exec.LookPath(driver.interpreter) if err != nil { - skipMissingShell(t, driver.interpreter+" is not installed") + skipMissingShell(t, shell, driver.interpreter+" is not installed") } render := shellCompletions[shell] @@ -160,13 +160,19 @@ func TestCompletionScriptsRequest(t *testing.T) { } } -// skipMissingShell skips a shell that is not installed, unless the environment says -// these tests are expected to run. A skip is silent, and a machine that has none of -// the four reports the same green as one where every request is right. -func skipMissingShell(t *testing.T, reason string) { +// skipMissingShell skips a shell that is not installed, unless it is one the +// environment names as required. A skip is silent, and a machine that has none of the +// four reports the same green as one where every request is right, so a run that is +// meant to cover a shell says which ones and fails when it cannot. +// +// The shells are named rather than required as a group, so that a job requiring what +// it installs is not broken by a shell disappearing from a runner image. +func skipMissingShell(t *testing.T, shell, reason string) { t.Helper() - if os.Getenv("CLI_SHELL_TESTS_REQUIRED") != "" { - t.Fatalf("CLI_SHELL_TESTS_REQUIRED is set: %s", reason) + for _, required := range strings.Split(os.Getenv("CLI_SHELL_TESTS_REQUIRED"), ",") { + if strings.TrimSpace(required) == shell { + t.Fatalf("%s is required by CLI_SHELL_TESTS_REQUIRED: %s", shell, reason) + } } t.Skip(reason) } @@ -207,7 +213,7 @@ var shellDrivers = map[string]shellDriver{ return ". " + p + "\n" } } - skipMissingShell(t, "bash-completion is not installed") + skipMissingShell(t, "bash", "bash-completion is not installed") return "" }, program: func(scriptPath string, tc completionCase) string { From 893749c5ae42c6c1896d16e70ee7f455a82ab706 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Fri, 7 Aug 2026 00:08:00 +0900 Subject: [PATCH 18/38] test: skip bash when its bash-completion cannot be used, and check the required names Two holes in the harness, both of which report something other than what is true. The bash driver looked for a bash-completion file and assumed that finding one meant it could be used. bash-completion 2.12 and later need bash 4.2, so on macOS, where /bin/bash is 3.2 and the file usually comes from Homebrew, sourcing it fails, the word-splitting helpers are never defined, and the script returns without asking anything. What the contributor saw was a syntax error from inside bash-completion and a failing assertion, for a change that has nothing to do with either. It now asks that bash whether the helpers end up defined, and skips saying so when they do not. CLI_SHELL_TESTS_REQUIRED matched names against the shells driven here and ignored anything else, so a typo in it required nothing at all and the CI guarantee it exists to provide would have gone away quietly. Names that match no driver now fail. --- completion_shell_test.go | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/completion_shell_test.go b/completion_shell_test.go index 9797437d6d..9b6f64ba2c 100644 --- a/completion_shell_test.go +++ b/completion_shell_test.go @@ -110,6 +110,8 @@ func TestCompletionScriptsRequest(t *testing.T) { // has all four. t.Parallel() + checkRequiredShells(t) + for _, shell := range []string{"bash", "zsh", "fish", "pwsh"} { t.Run(shell, func(t *testing.T) { t.Parallel() @@ -160,6 +162,22 @@ func TestCompletionScriptsRequest(t *testing.T) { } } +// checkRequiredShells fails when CLI_SHELL_TESTS_REQUIRED names a shell this file does +// not know. The variable is there so that coverage cannot go away quietly, which a +// typo in it would undo: a name matching nothing requires nothing. +func checkRequiredShells(t *testing.T) { + t.Helper() + for _, name := range strings.Split(os.Getenv("CLI_SHELL_TESTS_REQUIRED"), ",") { + name = strings.TrimSpace(name) + if name == "" { + continue + } + if _, ok := shellDrivers[name]; !ok { + t.Fatalf("CLI_SHELL_TESTS_REQUIRED names %q, which is not a shell driven here", name) + } + } +} + // skipMissingShell skips a shell that is not installed, unless it is one the // environment names as required. A skip is silent, and a machine that has none of the // four reports the same green as one where every request is right, so a run that is @@ -201,7 +219,7 @@ var shellDrivers = map[string]shellDriver{ args: func(p string) []string { return []string{"-c", p} }, // The script calls the word-splitting helpers of bash-completion, so without // it there is nothing to drive. - prelude: func(t *testing.T, _ string) string { + prelude: func(t *testing.T, interpreter string) string { t.Helper() for _, p := range []string{ "/usr/share/bash-completion/bash_completion", @@ -209,9 +227,20 @@ var shellDrivers = map[string]shellDriver{ "/opt/homebrew/share/bash-completion/bash_completion", "/usr/local/share/bash-completion/bash_completion", } { - if _, err := os.Stat(p); err == nil { - return ". " + p + "\n" + if _, err := os.Stat(p); err != nil { + continue + } + // Finding the file is not the same as being able to use it: + // bash-completion 2.12 and later need bash 4.2, so sourcing it in the + // bash macOS ships leaves the helpers undefined and the script with + // nothing to call. Ask this bash what it ends up with rather than + // assuming that the file is enough. + usable := exec.Command(interpreter, "-c", ". "+shQuote(p)+ + " >/dev/null 2>&1; declare -F _comp_initialize >/dev/null 2>&1 || declare -F _get_comp_words_by_ref >/dev/null 2>&1") + if err := usable.Run(); err != nil { + skipMissingShell(t, "bash", interpreter+" cannot use the bash-completion in "+p) } + return ". " + p + "\n" } skipMissingShell(t, "bash", "bash-completion is not installed") return "" From dfdb97a0a437b72198663ef171b6cbc1670aecb6 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Fri, 7 Aug 2026 00:15:30 +0900 Subject: [PATCH 19/38] test: keep looking when a bash-completion cannot be used Skipping inside the loop ended the search at the first file that was there but unusable, so a machine holding both a bash-completion its bash cannot source and an older one it can would have skipped over the one that works. The unusable ones are collected instead, and named in the skip when none of them turned out to be usable. --- completion_shell_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/completion_shell_test.go b/completion_shell_test.go index 9b6f64ba2c..cb31fe737f 100644 --- a/completion_shell_test.go +++ b/completion_shell_test.go @@ -221,6 +221,7 @@ var shellDrivers = map[string]shellDriver{ // it there is nothing to drive. prelude: func(t *testing.T, interpreter string) string { t.Helper() + var unusable []string for _, p := range []string{ "/usr/share/bash-completion/bash_completion", "/etc/bash_completion", @@ -234,14 +235,19 @@ var shellDrivers = map[string]shellDriver{ // bash-completion 2.12 and later need bash 4.2, so sourcing it in the // bash macOS ships leaves the helpers undefined and the script with // nothing to call. Ask this bash what it ends up with rather than - // assuming that the file is enough. + // assuming that the file is enough, and go on looking when the answer + // is no: an older one further down the list may still work. usable := exec.Command(interpreter, "-c", ". "+shQuote(p)+ " >/dev/null 2>&1; declare -F _comp_initialize >/dev/null 2>&1 || declare -F _get_comp_words_by_ref >/dev/null 2>&1") if err := usable.Run(); err != nil { - skipMissingShell(t, "bash", interpreter+" cannot use the bash-completion in "+p) + unusable = append(unusable, p) + continue } return ". " + p + "\n" } + if len(unusable) > 0 { + skipMissingShell(t, "bash", interpreter+" cannot use the bash-completion in "+strings.Join(unusable, ", ")) + } skipMissingShell(t, "bash", "bash-completion is not installed") return "" }, From 4af9af43a7a005105cc2637aac3e737d0fe41671 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Fri, 7 Aug 2026 00:51:51 +0900 Subject: [PATCH 20/38] fix: run a bash command typed as "~/bin/app" as the path it stands for eval expanded the tilde as a side effect of re-parsing the command line, and dropping eval dropped that too: the completion looked for a command whose name begins with a tilde, found none, and offered nothing. bash still calls the function, since it falls back to the compspec for the last path component. The tilde is put back on its own rather than by evaluating anything, so a "$(...)" on the line is still not run: it is the one expansion worth having, and the raw word decides, because a quoted "~" is not a home directory to the shell either. --- autocomplete/bash_autocomplete | 12 +++++++-- completion_shell_test.go | 49 ++++++++++++++++++++++++++++++++++ completion_test.go | 2 +- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/autocomplete/bash_autocomplete b/autocomplete/bash_autocomplete index 13c2e8a458..703292592a 100755 --- a/autocomplete/bash_autocomplete +++ b/autocomplete/bash_autocomplete @@ -65,10 +65,18 @@ __%[1]s_dequote() { # cur, which comes from there too, so a request built from anything else would ask the # command about a different word than the one being completed. __%[1]s_build_completion_request() { - local i + local i cmd __%[1]s_dequote "${words[0]}" - __%[1]s_completion_request=("${__%[1]s_dequoted}" "__complete") + cmd="${__%[1]s_dequoted}" + # A command typed as "~/bin/app" has to be run as the path it stands for. eval used + # to do that as a side effect of re-parsing the line, along with everything else on + # it; this is the one expansion worth keeping, and it needs nothing evaluated. The + # raw word decides, because a quoted "~" is not a home directory to the shell either. + if [[ "${words[0]}" == "~" || "${words[0]}" == "~/"* ]]; then + cmd="${HOME}${cmd:1}" + fi + __%[1]s_completion_request=("${cmd}" "__complete") for (( i = 1; i < cword; i++ )); do __%[1]s_dequote "${words[i]}" diff --git a/completion_shell_test.go b/completion_shell_test.go index cb31fe737f..0ca514e446 100644 --- a/completion_shell_test.go +++ b/completion_shell_test.go @@ -337,3 +337,52 @@ func fishQuote(s string) string { func pwshQuote(s string) string { return "'" + strings.ReplaceAll(s, "'", "''") + "'" } + +// TestCompletionBashScriptTildeCommand checks that a command typed as "~/bin/app" is +// run as the path it stands for. eval expanded it as a side effect of re-parsing the +// command line, and dropping eval dropped that with it, leaving the completion looking +// for a command whose name starts with a tilde and finding nothing. +func TestCompletionBashScriptTildeCommand(t *testing.T) { + t.Parallel() + + driver := shellDrivers["bash"] + interpreter, err := exec.LookPath(driver.interpreter) + if err != nil { + skipMissingShell(t, "bash", driver.interpreter+" is not installed") + } + + render := shellCompletions["bash"] + require.NotNil(t, render) + script, err := render(&Command{Name: "app", EnableShellCompletion: true}, "app") + require.NoError(t, err) + + home := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(home, "bin"), 0o755)) + writeCompletionTestApp(t, filepath.Join(home, "bin")) + + dir := t.TempDir() + scriptPath := filepath.Join(dir, "completion.bash") + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o644)) + argvPath := filepath.Join(dir, "argv") + + // The word is written with a quoted tilde so that this driver does not expand it: + // what the script receives has to be the tilde bash puts in COMP_WORDS. + program := fmt.Sprintf(` +. %s +COMP_WORDS=('~/bin/app' 'su') +COMP_CWORD=1 +COMP_LINE='~/bin/app su' +COMP_POINT=12 +__app_bash_autocomplete +`, shQuote(scriptPath)) + + cmd := exec.Command(interpreter, driver.args(driver.prelude(t, interpreter)+program)...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), "HOME="+home, "ARGV_LOG="+argvPath) + out, err := cmd.CombinedOutput() + require.NoError(t, err, "driving bash: %s", out) + + got, err := os.ReadFile(argvPath) + require.NoError(t, err, "the completion did not run the command: %s", out) + assert.Equal(t, []string{"__complete", "su"}, strings.Split(strings.TrimSuffix(string(got), "\n"), "\n")) +} diff --git a/completion_test.go b/completion_test.go index 9d30180bcb..57ace11100 100644 --- a/completion_test.go +++ b/completion_test.go @@ -282,7 +282,7 @@ func TestCompletionBashSendsTokenBeingCompleted(t *testing.T) { output, err := bashRender(cmd, "myapp") r.NoError(err) - r.Contains(output, `__myapp_completion_request=("${__myapp_dequoted}" "__complete")`) + r.Contains(output, `__myapp_completion_request=("${cmd}" "__complete")`) r.Contains(output, `__myapp_dequote "${words[cword]-}"`) r.Contains(output, `opts=$("${__myapp_completion_request[@]}" 2>/dev/null)`) r.Contains(output, `for (( i = 1; i < cword; i++ )); do`, From 24813db8c4809c08ba15085136be79bfc8b1cdb6 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Fri, 7 Aug 2026 00:52:40 +0900 Subject: [PATCH 21/38] docs: say what a script generated before this change actually does The note said that pressing tab on a command line holding a "--" usually ends in an Incorrect Usage message. It does not: after a "--" the request is a positional argument, so it reaches the action along with the rest of the line and the run completes as though enter had been pressed. Saying that it usually errors reads as though the danger were mostly theoretical, when it is the one #1993 is about. The Customization heading no longer had any customization under it, since what it introduced is now the request form, which an app cannot rename: the script and the app have to agree on the name. Two comments were claiming more than they check: what "--" means when a flag takes it as its value, which is not knowable before the flags are parsed, and what the zsh driver fills words and CURRENT with, which is zsh's tokenizer standing in for a completion system that needs a terminal to drive. --- completion_shell_test.go | 9 ++++++--- docs/v3/examples/completions/customizations.md | 5 +++-- docs/v3/examples/completions/shell-completions.md | 8 +++++--- help.go | 6 ++++++ 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/completion_shell_test.go b/completion_shell_test.go index 0ca514e446..1813634a1c 100644 --- a/completion_shell_test.go +++ b/completion_shell_test.go @@ -276,9 +276,12 @@ __app_bash_autocomplete interpreter: "zsh", args: func(p string) []string { return []string{"-f", "-c", p} }, prelude: func(*testing.T, string) string { return "" }, - // The completion system is not started, so the parts of it the script uses - // stand in for it: what is under test is the request the script builds from - // words and CURRENT, which zsh fills the same way here. + // The completion system is not started, since driving it needs a pseudo + // terminal, so the parts of it the script uses stand in for it and words and + // CURRENT are filled with zsh's own tokenizer. That last part is an assumption + // rather than something checked: unlike the bash words, which are written out + // as measured, these are what (z) makes of the line, which is close to what + // the completion system would pass but not known to be identical. program: func(scriptPath string, tc completionCase) string { return fmt.Sprintf(` compdef() { : } diff --git a/docs/v3/examples/completions/customizations.md b/docs/v3/examples/completions/customizations.md index c080748360..5cf9026603 100644 --- a/docs/v3/examples/completions/customizations.md +++ b/docs/v3/examples/completions/customizations.md @@ -105,11 +105,12 @@ func main() { } ``` -#### Customization +#### The completion request Setting `cli.EnableShellCompletion` makes the app answer a completion request, which the generated scripts send as a `__complete` first argument followed by the words typed so far and -the word being completed: +the word being completed. That name is fixed: unlike the flag it replaces, an app cannot rename it, +since the completion script and the app have to agree on it.