From 1d193c87fe2d10cbec7e0426435a56f17e673415 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 3 Nov 2026 03:45:47 +0000 Subject: [PATCH] Verify shell completion transport --- docs/roadmap/implementation.md | 21 ++- internal/cobra/completion.go | 86 +++++++++++ internal/cobra/completion_test.go | 134 ++++++++++++++++++ .../cobra/testdata/completion_terminal.py | 45 ++++++ 4 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 internal/cobra/completion.go create mode 100644 internal/cobra/completion_test.go create mode 100644 internal/cobra/testdata/completion_terminal.py diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index e8578f9..19c4e3e 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Shell completion implementation plan -Status: ready for implementation. +Status: Stage 1 complete. Stage 2 is next. Implement the [shell completion roadmap](completion.md) from baseline `2301872`. Follow the stages in order in one sprint. Intermediate stages may @@ -14,6 +14,8 @@ names, candidate ordering, and backend mechanics private. ## Stage 1: Prove the shell transport +Status: complete. + 1. Use the Cobra version pinned in the root module. In a disposable prototype, generate scripts with `GenBashCompletionV2(w, true)`, `GenZshCompletion(w)`, and `GenFishCompletion(w, true)`. Give the generator @@ -41,6 +43,23 @@ names, candidate ordering, and backend mechanics private. evidence. Carry forward only the adapter and fixtures needed for the final implementation; do not commit disposable prototype machinery. +Completion: generated scripts using Cobra v1.10.1 and exercised them through +interactive Bash 5.3.9, Zsh 5.9, and Fish 4.2.1 with a fixture that records the +query and the executed edited line. In the tested attached-value cases the +query includes the entered flag and value; a static candidate contains the value +alone, and the shell script preserves the flag prefix. Explicit empty words, +quoted prior arguments, short clusters, file and directory hints, and no-file +replies were exercised. Zsh's generated word splitting lost the attached flag +context when a path contained an escaped space. Fish split quoted prior words, +passed escapes in current words, and treated directory-only replies as ordinary +file completion. The adapter adjusts those scripts and preserves the first `=` +as the flag/value separator. The opt-in `TestCompletionShells` regression checks +actual edited lines for these cases. Run it with Bash, Zsh, Fish, Python pexpect, +and Bash completion installed using `COMMANDKIT_COMPLETION_SHELLS=1 +GOWORK=off GOTOOLCHAIN=local go test -count=1 -run '^TestCompletionShells$' +./internal/cobra`. Zsh and Fish were extracted into a disposable directory for +this run; the focused adapter suite and the full interactive matrix pass. + ## Stage 2: Capture completion hints 1. Add the following public API in the root package. Add diff --git a/internal/cobra/completion.go b/internal/cobra/completion.go new file mode 100644 index 0000000..5120cd1 --- /dev/null +++ b/internal/cobra/completion.go @@ -0,0 +1,86 @@ +package cobra + +import ( + "bytes" + "fmt" + "io" + "strings" + + spfcobra "github.com/spf13/cobra" +) + +// CompletionScript generates a shell script for the installed executable. +func CompletionScript(w io.Writer, executable, shell string) error { + root := &spfcobra.Command{Use: executable} + var generated bytes.Buffer + var err error + switch shell { + case "bash": + err = root.GenBashCompletionV2(&generated, true) + case "zsh": + err = root.GenZshCompletion(&generated) + case "fish": + err = root.GenFishCompletion(&generated, true) + default: + return fmt.Errorf("unsupported completion shell %q", shell) + } + if err != nil { + return err + } + + script := generated.String() + if shell == "zsh" { + script, err = replaceCompletionText(script, `words=("${=words[1,CURRENT]}")`, `words=("${(@)words[1,CURRENT]}")`) + if err == nil { + script, err = replaceCompletionText(script, `=~ '-.*='`, `=~ '-[^=]*='`) + } + } else if shell == "fish" { + script, err = adjustFishCompletion(script, root.Name()) + } + if err != nil { + return err + } + n, err := io.WriteString(w, script) + if err == nil && n != len(script) { + return io.ErrShortWrite + } + return err +} + +func replaceCompletionText(script, old, replacement string) (string, error) { + if strings.Count(script, old) != 1 { + return "", fmt.Errorf("unsupported generated completion script") + } + return strings.Replace(script, old, replacement, 1), nil +} + +func adjustFishCompletion(script, name string) (string, error) { + var err error + script, err = replaceCompletionText(script, + `set -l lastArg (string escape -- (commandline -ct))`, + "set -l lastArg (string unescape -- (commandline -ct))\n set -q lastArg[1]; or set lastArg \"\"") + if err != nil { + return "", err + } + script, err = replaceCompletionText(script, + `set -l results (eval $requestComp 2> /dev/null)`, + `set -lx COBRA_ACTIVE_HELP 0`+"\n "+`set -l results ($args[1] `+spfcobra.ShellCompRequestCmd+` $args[2..-1] "$lastArg" 2> /dev/null)`) + if err != nil { + return "", err + } + script, err = replaceCompletionText(script, `string match -r -- '-.*=' "$lastArg"`, `string match -r -- '-[^=]*=' "$lastArg"`) + if err != nil { + return "", err + } + variable := "__" + strings.NewReplacer("-", "_", ":", "_").Replace(name) + "_comp_results" + directories := fmt.Sprintf(`if test $dirfilter -eq 1 + set -l flagPrefix (string match -r -- '^-[^=]*=' (commandline -ct)) + set --global %[1]s + for dir in (__fish_complete_directories) + set -a %[1]s "$flagPrefix$dir" + end + return 0 + end + if test $filefilter -eq 1`, variable) + return replaceCompletionText(script, `if test $filefilter -eq 1; or test $dirfilter -eq 1`, directories) +} diff --git a/internal/cobra/completion_test.go b/internal/cobra/completion_test.go new file mode 100644 index 0000000..c029636 --- /dev/null +++ b/internal/cobra/completion_test.go @@ -0,0 +1,134 @@ +package cobra + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + spfcobra "github.com/spf13/cobra" +) + +func TestCompletionScriptGeneration(t *testing.T) { + for _, shell := range []string{"bash", "zsh", "fish"} { + var output bytes.Buffer + if err := CompletionScript(&output, "sample-app", shell); err != nil { + t.Fatalf("%s: %v", shell, err) + } + if !strings.Contains(output.String(), "__complete") { + t.Errorf("%s script lacks a completion request", shell) + } + } + if err := CompletionScript(io.Discard, "app", "other"); err == nil { + t.Fatal("unsupported shell succeeded") + } + if err := CompletionScript(shortCompletionWriter{}, "app", "bash"); !errors.Is(err, io.ErrShortWrite) { + t.Fatalf("short write: %v", err) + } +} + +type shortCompletionWriter struct{} + +func (shortCompletionWriter) Write(p []byte) (int, error) { return len(p) - 1, nil } + +// TestCompletionShells is opt-in because it requires three shells and pexpect. +func TestCompletionShells(t *testing.T) { + if os.Getenv("COMMANDKIT_COMPLETION_SHELLS") != "1" { + t.Skip("set COMMANDKIT_COMPLETION_SHELLS=1 to run shell completion checks") + } + dir := t.TempDir() + fixture := `#!/usr/bin/python3 +import json, os, sys +with open(os.environ['PROBE_LOG'], 'a') as log: + log.write(json.dumps(sys.argv[1:]) + '\n') +sys.stdout.write(os.environ['PROBE_REPLY']) +` + if err := os.WriteFile(filepath.Join(dir, "probe"), []byte(fixture), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(dir, "partial dir"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "partial file"), nil, 0o644); err != nil { + t.Fatal(err) + } + reply := func(candidates string, directive spfcobra.ShellCompDirective) string { + return fmt.Sprintf("%s:%d\n", candidates, directive) + } + tests := []struct { + name, line, reply string + query, executed []string + }{ + {"attached choice", "probe --config=./par", reply("./partial file\tdescription\n", spfcobra.ShellCompDirectiveNoFileComp), []string{"__complete", "--config=./par"}, []string{"--config=./partial file"}}, + {"choice with equals", "probe --config=foo=ba", reply("foo=bar\tdescription\n", spfcobra.ShellCompDirectiveNoFileComp), []string{"__complete", "--config=foo=ba"}, []string{"--config=foo=bar"}}, + {"separated file", `probe --config ./partial\ f`, reply("", spfcobra.ShellCompDirectiveDefault), []string{"__complete", "--config", "./partial f"}, []string{"--config", "./partial file"}}, + {"attached file", `probe --config=./partial\ f`, reply("", spfcobra.ShellCompDirectiveDefault), []string{"__complete", "--config=./partial f"}, []string{"--config=./partial file"}}, + {"directory", "probe ./partial", reply("", spfcobra.ShellCompDirectiveFilterDirs), []string{"__complete", "./partial"}, []string{"./partial dir"}}, + {"attached directory", "probe --config=./partial", reply("", spfcobra.ShellCompDirectiveFilterDirs), []string{"__complete", "--config=./partial"}, []string{"--config=./partial dir"}}, + {"empty", "probe ", reply("", spfcobra.ShellCompDirectiveNoFileComp), []string{"__complete", ""}, []string{}}, + {"no file fallback", `probe ./partial\ f`, reply("", spfcobra.ShellCompDirectiveNoFileComp), []string{"__complete", "./partial f"}, []string{"./partial f"}}, + {"cluster", "probe -ab", reply("-abc\tflag\n", spfcobra.ShellCompDirectiveNoFileComp), []string{"__complete", "-ab"}, []string{"-abc"}}, + {"quoted argument", "probe 'alpha beta' --config=./par", reply("./partial file\tdescription\n", spfcobra.ShellCompDirectiveNoFileComp), []string{"__complete", "alpha beta", "--config=./par"}, []string{"alpha beta", "--config=./partial file"}}, + } + for _, shell := range []string{"bash", "zsh", "fish"} { + script := filepath.Join(dir, shell+".completion") + var generated bytes.Buffer + if err := CompletionScript(&generated, "probe", shell); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(script, generated.Bytes(), 0o644); err != nil { + t.Fatal(err) + } + for _, tc := range tests { + t.Run(shell+"/"+tc.name, func(t *testing.T) { + log := filepath.Join(dir, "log") + if err := os.WriteFile(log, nil, 0o600); err != nil { + t.Fatal(err) + } + cmd := exec.Command("python3", "testdata/completion_terminal.py", shell, script, tc.line) + cmd.Dir = "." + cmd.Env = append(os.Environ(), "PATH="+dir+string(os.PathListSeparator)+os.Getenv("PATH"), "HOME="+dir, "TERM=xterm", "PROBE_LOG="+log, "PROBE_REPLY="+tc.reply, "PROBE_DIR="+dir) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("shell completion: %v\n%s", err, out) + } + contents, err := os.ReadFile(log) + if err != nil { + t.Fatal(err) + } + var entries [][]string + for _, line := range strings.Split(strings.TrimSpace(string(contents)), "\n") { + var args []string + if err := json.Unmarshal([]byte(line), &args); err != nil { + t.Fatalf("invalid fixture log %q: %v", contents, err) + } + entries = append(entries, args) + } + if len(entries) < 2 || !equalCompletionArgs(entries[len(entries)-2], tc.query, false) || !equalCompletionArgs(entries[len(entries)-1], tc.executed, strings.Contains(tc.name, "directory")) { + t.Fatalf("query and edited line: %q", contents) + } + }) + } + } +} + +func equalCompletionArgs(got, want []string, directory bool) bool { + if len(got) != len(want) { + return false + } + for i := range got { + value := got[i] + if directory && i == len(got)-1 { + value = strings.TrimSuffix(value, "/") + } + if value != want[i] { + return false + } + } + return true +} diff --git a/internal/cobra/testdata/completion_terminal.py b/internal/cobra/testdata/completion_terminal.py new file mode 100644 index 0000000..5deadec --- /dev/null +++ b/internal/cobra/testdata/completion_terminal.py @@ -0,0 +1,45 @@ +#!/usr/bin/env python3 +"""Exercise one generated completion through an interactive shell.""" + +import os +import shlex +import shutil +import sys + +import pexpect + +shell, script, line = sys.argv[1:] +executable = shutil.which(shell) +if executable is None: + raise SystemExit(f"{shell} is not installed") +quoted_script = shlex.quote(script) +if shell == "bash": + args = ["--noprofile", "--norc", "-i"] + setup = 'PS1="PROMPT> "; source /usr/share/bash-completion/bash_completion; source ' + quoted_script +elif shell == "zsh": + args = ["-f"] + setup = 'PROMPT="PROMPT> "; ' + if os.environ.get("COMPLETION_ZSH_MODULE_PATH"): + setup += "module_path=(" + shlex.quote(os.environ["COMPLETION_ZSH_MODULE_PATH"]) + "); " + if os.environ.get("COMPLETION_ZSH_FUNCTION_ROOT"): + setup += "fpath=(" + shlex.quote(os.environ["COMPLETION_ZSH_FUNCTION_ROOT"]) + "/**/*(N-/) $fpath); " + setup += "autoload -Uz compinit; compinit -D; source " + quoted_script +elif shell == "fish": + args = ["--no-config", "--interactive"] + setup = "function fish_prompt; printf '\\120\\122\\117\\115\\120\\124\\076\\040'; end; source " + quoted_script +else: + raise SystemExit(f"unsupported shell: {shell}") + +child = pexpect.spawn(executable, args, env=os.environ.copy(), cwd=os.environ["PROBE_DIR"], + encoding="utf-8", timeout=15, dimensions=(24, 120)) +child.delaybeforesend = 0.05 +try: + child.setecho(False) + child.sendline(setup) + child.expect_exact("PROMPT> ") + child.send(line) + child.send("\t") + child.sendline("") + child.expect_exact("PROMPT> ") +finally: + child.close(force=True)