Verify shell completion transport

2026-11-03 03:45:47 +00:00
parent 10a799a2b7
commit 1d193c87fe
4 changed files with 285 additions and 1 deletions
--- 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
--- a/internal/cobra/completion.go
+++ 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)
+}
--- a/internal/cobra/completion_test.go
+++ 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
+}
--- a/internal/cobra/testdata/completion_terminal.py
+++ 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)