diff --git a/CLAUDE.md b/CLAUDE.md index cdde31cf..c14afad6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -169,6 +169,10 @@ A REF is parsed by helpers in `internal/snapshot/destination.go`: - Never print directly to stdout/stderr (e.g., `fmt.Fprintf(os.Stderr, …)`). For user-facing output, emit events through `output.Sink`. For internal diagnostics, use `log.Logger`. If neither is available (e.g., during logger setup), return errors to the caller and let them decide. - Do not call `config.Get()` from domain/business-logic packages. Instead, extract the values you need at the command boundary (`cmd/`) and pass them as explicit function arguments. This keeps domain functions testable without requiring Viper/config initialization. +# Shell Completion + +Cobra's generated bash completion script requires `_get_comp_words_by_ref` from the bash-completion package on both of its init paths, and stock macOS (bash 3.2) ships without that package — so completion failed with "command not found" on every Tab (DEVX-950). `selfContainBashCompletion` in `cmd/completion.go` wraps the autogenerated `completion bash` command to prepend a guarded pure-bash fallback (defined only when the package is absent, the git-completion.bash approach) and replaces the help text. The fallback body must stay bash 3.2 compatible (no `declare -A`, namerefs, `mapfile`, case-conversion expansions). It covers only `_get_comp_words_by_ref`; Cobra's script still calls bash-completion's `_filedir` for `ShellCompDirectiveFilterFileExt`/`ShellCompDirectiveFilterDirs` (`MarkFlagFilename`/`MarkFlagDirname`) and the ActiveHelp second-Tab path — lstk uses none of these today, so adopting one means growing the fallback. In docs/help, never recommend `source <(lstk completion bash)` — it is a silent no-op on bash 3.2; recommend `eval "$(lstk completion bash)"` instead. Zsh/fish/powershell scripts are self-contained upstream and untouched. + # CLI Help Text - Write command `Short`/`Long` as unbroken paragraphs (one line each, blank line between); never hard-wrap a sentence in source. `wrapText` in `cmd/help.go` re-wraps to the terminal width at render time and `lstk docs` reads the raw text, so manual breaks fight both. Indented lines (examples, aligned output) are left as-is. diff --git a/cmd/completion.go b/cmd/completion.go new file mode 100644 index 00000000..db5161bd --- /dev/null +++ b/cmd/completion.go @@ -0,0 +1,157 @@ +package cmd + +import ( + "fmt" + "io" + + "github.com/spf13/cobra" +) + +// bashCompletionFallback is prepended to Cobra's generated bash completion +// script. Both init paths of that script end up calling +// _get_comp_words_by_ref from the bash-completion package, but stock macOS +// ships bash 3.2 with no bash-completion at all, so every Tab press failed +// with "_get_comp_words_by_ref: command not found" (DEVX-950). Mirroring the +// approach git-completion.bash ships, this defines a minimal replacement only +// when the package is absent; with bash-completion (v1 or v2) installed the +// guard skips it. The body must stay bash 3.2 compatible. +// +// The fallback covers only _get_comp_words_by_ref — the paths lstk reaches +// today. Cobra's script still calls bash-completion's _filedir (undefined +// without the package) for ShellCompDirectiveFilterFileExt (MarkFlagFilename +// with extensions), ShellCompDirectiveFilterDirs (MarkFlagDirname), and the +// ActiveHelp second-Tab path, so adopting any of those requires growing the +// fallback (or avoiding the directive). +const bashCompletionFallback = `# lstk: self-contained fallback for the bash-completion package. +# The generated script below calls _get_comp_words_by_ref, which stock macOS +# (bash 3.2, no bash-completion package) does not provide. Define a minimal +# replacement when absent so completion works without extra packages. +if ! declare -F _get_comp_words_by_ref >/dev/null 2>&1; then + _get_comp_words_by_ref() { + local exclude="" i w issep attach="" preblank j=-1 + local line="$COMP_LINE" + local -a rebuilt=() + local rcword="$COMP_CWORD" + if [[ "$1" == "-n" ]]; then + exclude="$2" + shift 2 + fi + + # readline splits the command line at every character in + # COMP_WORDBREAKS ('=' and ':' included), so '--flag=value' arrives in + # COMP_WORDS as the three words '--flag', '=', 'value'. Rebuild the + # word list, re-joining pieces around the separators listed in + # $exclude, and track where the word under the cursor lands. Pieces + # re-join only when typed with no whitespace between them; COMP_WORDS + # is identical for '--flag=x' and '--flag = x', so adjacency has to be + # recovered from COMP_LINE. + for ((i = 0; i < ${#COMP_WORDS[@]}; i++)); do + w="${COMP_WORDS[$i]}" + preblank="" + if [[ "$line" == [[:blank:]]* ]]; then + preblank=1 + fi + line="${line#*"$w"}" + issep="" + if [[ -n "$w" && -n "$exclude" && -z "${w//[$exclude]/}" ]]; then + issep=1 + fi + # A separator (or the piece right after one) attaches to the + # previous word when adjacent on the typed line, except directly + # after the command name. + if [[ $j -ge 1 && -z "$preblank" && ( -n "$issep" || -n "$attach" ) ]]; then + rebuilt[$j]="${rebuilt[$j]}$w" + else + j=$((j + 1)) + rebuilt[$j]="$w" + fi + if [[ $i -eq $COMP_CWORD ]]; then + rcword=$j + fi + attach="$issep" + done + + while [[ $# -gt 0 ]]; do + case "$1" in + cur) + cur="${rebuilt[$rcword]}" + ;; + prev) + prev="" + if [[ $rcword -gt 0 ]]; then + prev="${rebuilt[$((rcword - 1))]}" + fi + ;; + words) + words=("${rebuilt[@]}") + ;; + cword) + cword="$rcword" + ;; + esac + shift + done + return 0 + } +fi + +` + +// selfContainBashCompletion replaces the autogenerated `completion bash` +// command's RunE so the emitted script carries bashCompletionFallback ahead +// of Cobra's output, removing the hard dependency on the bash-completion +// package (DEVX-950). Cobra's own RunE writes to a writer captured when +// InitDefaultCompletionCmd ran, so both halves are generated here against the +// writer resolved at execution time — otherwise SetOut after NewRootCmd would +// split them across two destinations. It also replaces the help text: Cobra's +// default recommends 'source <(lstk completion bash)', which is a silent +// no-op on macOS's stock bash 3.2, and states a package dependency that no +// longer holds. +func selfContainBashCompletion(completionCmd *cobra.Command) { + var bashCmd *cobra.Command + for _, sub := range completionCmd.Commands() { + if sub.Name() == "bash" { + bashCmd = sub + break + } + } + if bashCmd == nil { + return + } + + bashCmd.RunE = func(cmd *cobra.Command, args []string) error { + out := cmd.OutOrStdout() + if _, err := io.WriteString(out, bashCompletionFallback); err != nil { + return err + } + // Cobra registers --no-descriptions unless CompletionOptions turned + // it off; an absent flag means descriptions stay enabled. + noDesc := false + if v, err := cmd.Flags().GetBool("no-descriptions"); err == nil { + noDesc = v + } + return cmd.Root().GenBashCompletionV2(out, !noDesc) + } + + name := bashCmd.Root().Name() + bashCmd.Long = fmt.Sprintf(`Generate the autocompletion script for the bash shell. + +The script works with or without the 'bash-completion' package: when the package is absent (e.g. stock macOS bash), a bundled fallback is used instead. + +To load completions in your current shell session: + + eval "$(%[1]s completion bash)" + +To load completions for every new session, add the line above to ~/.bashrc (or ~/.bash_profile on macOS), or execute once: + +#### Linux: + + %[1]s completion bash > /etc/bash_completion.d/%[1]s + +#### macOS (with the bash-completion Homebrew package): + + %[1]s completion bash > $(brew --prefix)/etc/bash_completion.d/%[1]s + +You will need to start a new shell for this setup to take effect. +`, name) +} diff --git a/cmd/completion_test.go b/cmd/completion_test.go new file mode 100644 index 00000000..18e1404c --- /dev/null +++ b/cmd/completion_test.go @@ -0,0 +1,33 @@ +package cmd + +import ( + "testing" +) + +// TestCompletionBashWritesFallbackAndScriptToSameWriter guards the DEVX-950 +// wiring: the fallback prelude and Cobra's generated script must both reach +// the writer configured at execution time. Cobra captures its output writer +// when InitDefaultCompletionCmd runs (before SetOut is called here), so a +// prepend-and-delegate wrapper would send the two halves to different +// destinations. +func TestCompletionBashWritesFallbackAndScriptToSameWriter(t *testing.T) { + out, err := executeWithArgs(t, "completion", "bash") + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + assertContains(t, out, "_get_comp_words_by_ref()") + assertContains(t, out, "__start_lstk") + assertContains(t, out, "__complete") +} + +// TestCompletionBashNoDescriptionsFlagStillHonored verifies the wrapped RunE +// keeps Cobra's --no-descriptions behavior: the generated script requests +// completions via __completeNoDesc instead of __complete. +func TestCompletionBashNoDescriptionsFlagStillHonored(t *testing.T) { + out, err := executeWithArgs(t, "completion", "bash", "--no-descriptions") + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + assertContains(t, out, "_get_comp_words_by_ref()") + assertContains(t, out, "__completeNoDesc") +} diff --git a/cmd/root.go b/cmd/root.go index 0c156803..1f519422 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -156,6 +156,7 @@ func NewRootCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.C root.InitDefaultCompletionCmd() if completionCmd, _, err := root.Find([]string{"completion"}); err == nil && completionCmd.Name() == "completion" { requireSubcommand(completionCmd) + selfContainBashCompletion(completionCmd) } return root diff --git a/test/integration/completion_test.go b/test/integration/completion_test.go new file mode 100644 index 00000000..d07d7701 --- /dev/null +++ b/test/integration/completion_test.go @@ -0,0 +1,216 @@ +package integration_test + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/localstack/lstk/test/integration/env" +) + +// stockBash returns the bash to test against, preferring /bin/bash (bash 3.2 +// on macOS, the environment DEVX-950 was reported on) over whatever is first +// in PATH. +func stockBash(t *testing.T) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("bash completion not applicable on Windows") + } + if _, err := os.Stat("/bin/bash"); err == nil { + return "/bin/bash" + } + bashPath, err := exec.LookPath("bash") + if err != nil { + t.Skip("bash not available") + } + return bashPath +} + +// runBashCompletionDriver generates the bash completion script, writes it and +// the given driver script to disk, and runs the driver in a bare bash +// (--noprofile --norc, minimal env) so nothing from the developer's machine — +// in particular the bash-completion package — can leak in. That bare shell is +// exactly what DEVX-950 reproduces: stock macOS ships bash 3.2 with no +// bash-completion, so the generated script must be self-contained. The driver +// receives the completion script path as $1. +func runBashCompletionDriver(t *testing.T, driver string) (stdout, stderr string, err error) { + t.Helper() + bashPath := stockBash(t) + + tmpHome := t.TempDir() + script, genStderr, err := runLstk(t, testContext(t), t.TempDir(), testEnvWithHome(tmpHome, ""), "completion", "bash") + require.NoError(t, err, "lstk completion bash failed: %s", genStderr) + + dir := t.TempDir() + scriptPath := filepath.Join(dir, "lstk-completion.bash") + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600)) + driverPath := filepath.Join(dir, "driver.bash") + require.NoError(t, os.WriteFile(driverPath, []byte(driver), 0o600)) + + binDir, err := filepath.Abs(filepath.Dir(binaryPath())) + require.NoError(t, err) + + cmd := exec.CommandContext(testContext(t), bashPath, "--noprofile", "--norc", driverPath, scriptPath) + cmd.Dir = dir + // The driver invokes `lstk __complete ...`; force the file keyring so + // that subprocess never probes the developer's system keyring. + cmd.Env = []string{ + "HOME=" + tmpHome, + "PATH=" + binDir + ":/usr/bin:/bin", + fmt.Sprintf("%s=file", env.Keyring), + } + var outBuf, errBuf strings.Builder + cmd.Stdout = &outBuf + cmd.Stderr = &errBuf + err = cmd.Run() + return outBuf.String(), errBuf.String(), err +} + +// completeInDriver simulates pressing Tab on the given command line state and +// prints the resulting COMPREPLY, one completion per line. compWords is a +// bash array literal (readline splits at COMP_WORDBREAKS characters, '=' and +// ':' included, so a typed '--flag=value' must be given as '--flag = value'). +func completeInDriver(compWords string, cword int, line string) string { + return fmt.Sprintf(`source "$1" || exit 1 +COMP_WORDS=(%s) +COMP_CWORD=%d +COMP_LINE=%q +COMP_POINT=%d +__start_lstk +status=$? +printf '%%s\n' "${COMPREPLY[@]}" +exit $status +`, compWords, cword, line, len(line)) +} + +// TestBashCompletionWorksWithoutBashCompletionPackage guards against +// DEVX-950: on stock macOS (bash 3.2, no bash-completion package) every Tab +// press failed with "_get_comp_words_by_ref: command not found" because the +// Cobra-generated script depends on that function from the bash-completion +// package on both of its init paths. +func TestBashCompletionWorksWithoutBashCompletionPackage(t *testing.T) { + t.Parallel() + + stdout, stderr, err := runBashCompletionDriver(t, completeInDriver("lstk st", 1, "lstk st")) + require.NoError(t, err, "completion attempt failed\nstdout: %s\nstderr: %s", stdout, stderr) + assert.NotContains(t, stderr, "command not found") + + completions := strings.Fields(stdout) + assert.Contains(t, completions, "start") + assert.Contains(t, completions, "status") + assert.Contains(t, completions, "stop") +} + +// TestBashCompletionAfterWhitespaceSeparatedFlagValue covers the shape that +// separates adjacency-aware reassembly from naive gluing: in +// 'lstk --config= st' readline delivers the same COMP_WORDS as for +// 'lstk --config=st', and only COMP_LINE reveals that 'st' is a separate word +// that should be completed to a subcommand. +func TestBashCompletionAfterWhitespaceSeparatedFlagValue(t *testing.T) { + t.Parallel() + + stdout, stderr, err := runBashCompletionDriver(t, completeInDriver(`lstk --config = st`, 3, "lstk --config= st")) + require.NoError(t, err, "completion attempt failed\nstdout: %s\nstderr: %s", stdout, stderr) + + completions := strings.Fields(stdout) + assert.Contains(t, completions, "start") + assert.Contains(t, completions, "status") + assert.Contains(t, completions, "stop") +} + +// TestBashCompletionReassemblesWordbreakSplits verifies the self-contained +// _get_comp_words_by_ref fallback matches the bash-completion package's +// semantics: pieces split at COMP_WORDBREAKS characters are re-joined only +// when they were typed with no whitespace between them, which has to be +// recovered from COMP_LINE since COMP_WORDS is identical either way. +func TestBashCompletionReassemblesWordbreakSplits(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + compWords string + cword int + line string + expect []string + }{ + { + name: "adjacent pieces re-join", + compWords: `lstk --config = ./cfg`, + cword: 3, + line: "lstk --config=./cfg", + expect: []string{"cur=--config=./cfg", "prev=lstk", "cword=1", "nwords=2"}, + }, + { + name: "word after separator-then-space stays separate", + compWords: `lstk --config = st`, + cword: 3, + line: "lstk --config= st", + expect: []string{"cur=st", "prev=--config=", "cword=2", "nwords=3"}, + }, + { + name: "whitespace-surrounded separator stays separate", + compWords: `lstk --config = ./x`, + cword: 3, + line: "lstk --config = ./x", + expect: []string{"cur=./x", "prev==", "cword=3", "nwords=4"}, + }, + { + name: "empty word after separator-then-space", + compWords: `lstk --config = ""`, + cword: 3, + line: "lstk --config= ", + expect: []string{"cur=", "prev=--config=", "cword=2", "nwords=3"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + driver := fmt.Sprintf(`source "$1" || exit 1 +COMP_WORDS=(%s) +COMP_CWORD=%d +COMP_LINE=%q +COMP_POINT=%d +run_reassembly() { + local cur prev words cword + _get_comp_words_by_ref -n =: cur prev words cword || exit 1 + printf 'cur=%%s\n' "$cur" + printf 'prev=%%s\n' "$prev" + printf 'cword=%%s\n' "$cword" + printf 'nwords=%%s\n' "${#words[@]}" +} +run_reassembly +`, tc.compWords, tc.cword, tc.line, len(tc.line)) + + stdout, stderr, err := runBashCompletionDriver(t, driver) + require.NoError(t, err, "driver failed\nstdout: %s\nstderr: %s", stdout, stderr) + assert.NotContains(t, stderr, "command not found") + + lines := strings.Split(strings.TrimRight(stdout, "\n"), "\n") + assert.Equal(t, tc.expect, lines) + }) + } +} + +// TestBashCompletionFallbackYieldsToInstalledPackage verifies the fallback is +// guarded: when the bash-completion package already provides +// _get_comp_words_by_ref, sourcing the lstk script must not replace it. +func TestBashCompletionFallbackYieldsToInstalledPackage(t *testing.T) { + t.Parallel() + + driver := `_get_comp_words_by_ref() { echo "package version"; } +source "$1" || exit 1 +_get_comp_words_by_ref +` + stdout, stderr, err := runBashCompletionDriver(t, driver) + require.NoError(t, err, "driver failed\nstdout: %s\nstderr: %s", stdout, stderr) + assert.Contains(t, stdout, "package version") +}