diff --git a/README.md b/README.md index c254a74..529cb54 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,9 @@ Discovery is bounded and configurable: These settings are recorded in managed metadata, so `update` uses the same depth, command-count, and output policy. `--timeout` remains a global runtime -override for all subprocess-based operations. +override for all subprocess-based operations. While discovery is running in an +interactive terminal, `generate` displays a spinner on stderr; redirected and +piped output remains unchanged. It is deliberately conservative: only the three exact headings above and indented `name description` rows are recognized as subcommands. In sections @@ -124,11 +126,13 @@ the current command's direct children. Aliases, multi-word command columns, missing descriptions, unrecognized headings, and unusual layouts are omitted rather than guessed. Positionals are never assumed to be files, and descriptions are rendered inert — help text can't inject code -into your shell. Reaching the depth limit leaves deeper commands listed but -does not inspect their options; exceeding the command-count, output-size, or -timeout limit fails generation without replacing the installed definition. If -the tool has a working native command, `generate` refuses and points you at -`install` unless you pass `--force`. +into your shell. Advertised subcommands whose help command fails or has no +recognizable structure remain listed but are not inspected. Reaching the depth +limit similarly leaves deeper commands listed without inspecting their options; +exceeding the command-count, output-size, or timeout limit fails generation +without replacing the installed definition. If the tool has a working native +command, `generate` refuses and points you at `install` unless you pass +`--force`. ### Provenance diff --git a/main.go b/main.go index 380bb1c..92ae583 100644 --- a/main.go +++ b/main.go @@ -14,6 +14,7 @@ import ( "regexp" "sort" "strings" + "sync" "time" "github.com/spf13/cobra" @@ -82,6 +83,60 @@ func paint(on bool, code, s string) string { return "\x1b[" + code + "m" + s + "\x1b[0m" } +type progressSpinner struct { + once sync.Once + done chan struct{} + stopped chan struct{} + w io.Writer + enabled bool +} + +func spinnerTTY(f *os.File) bool { + if os.Getenv("TERM") == "dumb" { + return false + } + st, err := f.Stat() + return err == nil && st.Mode()&os.ModeCharDevice != 0 +} + +func startSpinner(w io.Writer, label string, enabled bool) *progressSpinner { + s := &progressSpinner{w: w, enabled: enabled} + if !enabled { + return s + } + s.done = make(chan struct{}) + s.stopped = make(chan struct{}) + frames := []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} + fmt.Fprintf(w, "\r%s %s", frames[0], label) + go func() { + defer close(s.stopped) + ticker := time.NewTicker(80 * time.Millisecond) + defer ticker.Stop() + i := 1 + for { + select { + case <-ticker.C: + fmt.Fprintf(w, "\r%s %s", frames[i%len(frames)], label) + i++ + case <-s.done: + return + } + } + }() + return s +} + +func (s *progressSpinner) Stop() { + if !s.enabled { + return + } + s.once.Do(func() { + close(s.done) + <-s.stopped + fmt.Fprint(s.w, "\r\033[K") + }) +} + // suggest appends copy-pasteable commands to a message, each bare on its own // indented line directly under it, so selecting a line yields a runnable // command with no quoting or prose to strip. @@ -219,13 +274,17 @@ func captureLimit(path string, args []string, d time.Duration, limit int64) ([]b c := exec.CommandContext(ctx, path, args...) c.Stdin = nil var out, er bytes.Buffer - c.Stdout = &limited{w: &out, n: limit} - c.Stderr = &limited{w: &er, n: limit} + budget := &outputBudget{n: limit} + c.Stdout = &limited{w: &out, budget: budget} + c.Stderr = &limited{w: &er, budget: budget} e := c.Run() if ctx.Err() != nil { return nil, fmt.Errorf("command timed out") } if e != nil { + if strings.Contains(e.Error(), "output exceeds limit") { + return nil, errors.New("output exceeds limit") + } // The tool's own stderr explains the failure better than an exit code; // fall back to the exec error only when the command died silently. if msg := stripErrorPrefix(er.String()); msg != "" { @@ -250,16 +309,23 @@ func stripErrorPrefix(s string) string { } type limited struct { - w io.Writer - n int64 + w io.Writer + budget *outputBudget +} + +type outputBudget struct { + mu sync.Mutex + n int64 } func (l *limited) Write(p []byte) (int, error) { - if int64(len(p)) > l.n { + l.budget.mu.Lock() + defer l.budget.mu.Unlock() + if int64(len(p)) > l.budget.n { return 0, errors.New("output exceeds limit") } n, e := l.w.Write(p) - l.n -= int64(n) + l.budget.n -= int64(n) return n, e } func validate(tool string, b []byte) error { @@ -340,6 +406,9 @@ func (a *app) write(tool string, b []byte, m metadata) error { if _, e = f.Write(body); e == nil { e = f.Chmod(0644) } + if e == nil { + e = f.Sync() + } if e == nil { e = f.Close() } else { @@ -348,6 +417,16 @@ func (a *app) write(tool string, b []byte, m metadata) error { if e == nil { e = os.Rename(n, p) } + if e == nil { + if d, de := os.Open(a.dir); de != nil { + fmt.Fprintf(os.Stderr, "%s %s installed but completion directory was not synced: %v\n", paint(useColor.err, "1;33", "warning:"), tool, de) + } else { + if de = d.Sync(); de != nil { + fmt.Fprintf(os.Stderr, "%s %s installed but completion directory was not synced: %v\n", paint(useColor.err, "1;33", "warning:"), tool, de) + } + _ = d.Close() + } + } if e == nil { // The definition is installed at this point; a failed event write only // costs the current shell's refresh, so warn instead of failing. @@ -447,6 +526,7 @@ func (a *app) updateOne(tool string) error { func (a *app) update() *cobra.Command { return &cobra.Command{Use: "update [TOOL]", Args: cobra.MaximumNArgs(1), RunE: func(c *cobra.Command, x []string) error { tools := x + var fails []string if len(tools) == 0 { es, er := os.ReadDir(a.dir) if er != nil && !os.IsNotExist(er) { @@ -456,7 +536,7 @@ func (a *app) update() *cobra.Command { if strings.HasPrefix(e.Name(), "_") && !e.IsDir() { b, re := os.ReadFile(filepath.Join(a.dir, e.Name())) if re != nil { - tools = append(tools, strings.TrimPrefix(e.Name(), "_")) + fails = append(fails, e.Name()+": "+re.Error()) continue } if _, er := decode(b); er == nil { @@ -468,7 +548,6 @@ func (a *app) update() *cobra.Command { } sort.Strings(tools) } - var fails []string for _, t := range tools { if e := a.updateOne(t); e != nil { fails = append(fails, t+": "+e.Error()) @@ -598,6 +677,10 @@ func (a *app) generate() *cobra.Command { if e := a.shadowGuard(x[0], "completionctl generate --force "+x[0]); e != nil { return e } + } + spinner := startSpinner(c.ErrOrStderr(), "Discovering "+x[0]+" commands…", c.ErrOrStderr() == os.Stderr && spinnerTTY(os.Stderr)) + defer spinner.Stop() + if !force { for _, ar := range nativeArgs { if b, e := capture(exe, ar, a.timeout); e == nil && validate(x[0], b) == nil { return errors.New(suggest(useColor.err, @@ -615,6 +698,7 @@ func (a *app) generate() *cobra.Command { if e != nil { return e } + spinner.Stop() m := metadata{Version: 1, Tool: x[0], Source: "help", Executable: exe, Args: help, Parser: "command-tree-v1", MaxDepth: limits.MaxDepth, MaxCommands: limits.MaxCommands, MaxOutput: limits.MaxOutput} if e = a.write(x[0], z, m); e != nil { return e @@ -707,11 +791,17 @@ func (a *app) discoverHelp(exe string, helpArgs []string, limits discoveryLimits args := append(append([]string{}, path...), helpArgs...) b, err := captureLimit(exe, args, a.timeout, remaining) if err != nil { + if len(path) > 0 && !isDiscoveryLimitError(err) { + continue + } return commandTree{}, fmt.Errorf("help for %s: %w", commandLabel(path), err) } remaining -= int64(len(b)) command, err := parseHelp(b) if err != nil { + if len(path) > 0 { + continue + } return commandTree{}, fmt.Errorf("help for %s: %w", commandLabel(path), err) } tree.Nodes = append(tree.Nodes, commandNode{Path: append([]string{}, path...), Command: command}) @@ -730,6 +820,11 @@ func (a *app) discoverHelp(exe string, helpArgs []string, limits discoveryLimits return tree, nil } +func isDiscoveryLimitError(err error) bool { + s := err.Error() + return strings.Contains(s, "timed out") || strings.Contains(s, "output exceeds limit") +} + func commandLabel(path []string) string { if len(path) == 0 { return "root command" @@ -979,23 +1074,32 @@ func renderTree(tool string, tree commandTree) ([]byte, error) { } var b strings.Builder fmt.Fprintf(&b, "#compdef %s\n# help-derived: recognized command sections and options only\n_completionctl_generated() {\n", tool) - b.WriteString(" local context state line _cc_key='' _cc_i=2\n typeset -A opt_args\n") - // Consume only an exact, recognized command prefix. Once flags or unknown - // positionals begin, dispatch remains at the last confirmed node. + b.WriteString(" local context state line _cc_key='' _cc_i=2 _cc_start=2\n typeset -A opt_args\n") + // Consume a recognized command path while allowing options before commands. + // Required option arguments are skipped so a value matching a command name + // cannot accidentally change completion context. b.WriteString(" while (( _cc_i < CURRENT )); do\n case \"$_cc_key|${words[_cc_i]}\" in\n") for _, n := range tree.Nodes { parent := strings.Join(n.Path, " ") for _, sub := range n.Command.Subcommands { child := strings.TrimSpace(parent + " " + sub.Name) - fmt.Fprintf(&b, " %s) _cc_key=%s ;;\n", zshCaseWord(parent+"|"+sub.Name), zshWordUnsafe(child)) + fmt.Fprintf(&b, " %s) _cc_key=%s; _cc_start=$((_cc_i + 1)) ;;\n", zshCaseWord(parent+"|"+sub.Name), zshWordUnsafe(child)) + } + for _, option := range n.Command.Options { + if option.Cardinality != valueRequired { + continue + } + for _, alias := range option.Aliases { + fmt.Fprintf(&b, " %s) (( _cc_i++ )) ;;\n", zshCaseWord(parent+"|"+alias)) + } } } - b.WriteString(" *) break ;;\n esac\n (( _cc_i++ ))\n done\n case \"$_cc_key\" in\n") + b.WriteString(" *'|-'*) ;;\n *) break ;;\n esac\n (( _cc_i++ ))\n done\n case \"$_cc_key\" in\n") for _, n := range tree.Nodes { key := strings.Join(n.Path, " ") fmt.Fprintf(&b, " %s)\n", zshCaseWord(key)) if len(n.Path) > 0 { - fmt.Fprintf(&b, " words=(\"$words[1]\" \"${words[%d,-1]}\")\n (( CURRENT -= %d ))\n", len(n.Path)+2, len(n.Path)) + b.WriteString(" words=(\"$words[1]\" \"${words[_cc_start,-1]}\")\n (( CURRENT -= _cc_start - 2 ))\n") } specs := renderOptionSpecs(n.Command) if len(n.Command.Subcommands) > 0 { diff --git a/main_test.go b/main_test.go index 4b3f6ff..334fc09 100644 --- a/main_test.go +++ b/main_test.go @@ -243,6 +243,40 @@ esac } } +func TestDiscoverHelpSkipsUnsupportedAdvertisedCommand(t *testing.T) { + bin := t.TempDir() + tool := filepath.Join(bin, "tool") + script := `#!/bin/sh +case "$*" in + --help) printf '%s\n' 'Commands:' ' good Works' ' plugin External plugin' ;; + 'good --help') printf '%s\n' 'Options:' ' --ok works' ;; + 'plugin --help') exit 2 ;; +esac +` + if err := os.WriteFile(tool, []byte(script), 0755); err != nil { + t.Fatal(err) + } + a := app{timeout: time.Second} + tree, err := a.discoverHelp(tool, []string{"--help"}, discoveryLimits{MaxDepth: 1, MaxCommands: 3, MaxOutput: 4096}) + if err != nil { + t.Fatal(err) + } + if len(tree.Nodes) != 2 || strings.Join(tree.Nodes[1].Path, " ") != "good" { + t.Fatalf("tree=%#v", tree) + } +} + +func TestCaptureLimitIsSharedAcrossOutputStreams(t *testing.T) { + tool := filepath.Join(t.TempDir(), "tool") + script := "#!/bin/sh\nprintf '%080d' 0\nprintf '%080d' 0 >&2\n" + if err := os.WriteFile(tool, []byte(script), 0755); err != nil { + t.Fatal(err) + } + if _, err := captureLimit(tool, nil, time.Second, 100); err == nil || !strings.Contains(err.Error(), "output exceeds limit") { + t.Fatalf("shared output limit error=%v", err) + } +} + func TestRenderTreeContainsContextSpecificOptions(t *testing.T) { tree := commandTree{Nodes: []commandNode{ {Command: parsedCommand{Options: []parsedOption{{Aliases: []string{"--root"}, Description: "root"}}, Subcommands: []parsedSubcommand{{"run", "Run it"}}}}, @@ -263,6 +297,26 @@ func TestRenderTreeContainsContextSpecificOptions(t *testing.T) { } } +func TestRenderTreeAllowsOptionsBeforeSubcommands(t *testing.T) { + tree := commandTree{Nodes: []commandNode{ + {Command: parsedCommand{ + Options: []parsedOption{{Aliases: []string{"--config"}, Cardinality: valueRequired}}, + Subcommands: []parsedSubcommand{{"run", "Run it"}}, + }}, + {Path: []string{"run"}, Command: parsedCommand{Options: []parsedOption{{Aliases: []string{"--child"}}}}}, + }} + b, err := renderTree("tool", tree) + if err != nil { + t.Fatal(err) + } + s := string(b) + // Keep the assertions tied to dispatch behavior rather than the complete + // generated function. + if !strings.Contains(s, "'|--config') (( _cc_i++ ))") || !strings.Contains(s, "*'|-'*) ;;") { + t.Fatal(s) + } +} + func TestRenderRequiredAndOptional(t *testing.T) { c, _ := parseHelp([]byte("--required FILE r\n--optional [DIR] o")) b, e := renderZsh("tool", c) @@ -415,6 +469,23 @@ func TestPaintRespectsToggle(t *testing.T) { } } +func TestProgressSpinnerCanStopCleanly(t *testing.T) { + var out bytes.Buffer + spinner := startSpinner(&out, "Discovering tool commands…", true) + spinner.Stop() + spinner.Stop() + got := out.String() + if !strings.Contains(got, "Discovering tool commands…") || !strings.HasSuffix(got, "\r\033[K") { + t.Fatalf("spinner output=%q", got) + } + + out.Reset() + startSpinner(&out, "hidden", false).Stop() + if out.Len() != 0 { + t.Fatalf("disabled spinner wrote %q", out.String()) + } +} + func TestUpdateReimportsFromRecordedSource(t *testing.T) { dir, src := t.TempDir(), t.TempDir() t.Setenv(searchEnv, "") diff --git a/tests/help-completion-live.zsh b/tests/help-completion-live.zsh index 4bb0d0f..482c923 100644 --- a/tests/help-completion-live.zsh +++ b/tests/help-completion-live.zsh @@ -13,6 +13,7 @@ cat >"$tmp/bin/faketool" <<'EOF' case "$*" in --help) printf ' -o, --output Write output\n' + printf ' --verbose Enable verbose output\n' printf ' --visibility \n Set thread visibility (private, unlisted, workspace, group)\n' printf ' --mode Select mode\n' printf 'Commands:\n run Run a job\n' @@ -54,7 +55,8 @@ for want in -- --output --visibility --mode; do done # The dispatcher selects options from the confirmed nested command path. -zpty -wn comp $'faketool run deep --\t' +# Global flags before the command path must not pin dispatch at the root. +zpty -wn comp $'faketool --verbose run deep --\t' zpty -wn comp $'\n' zpty -w comp "print 'LEAF''MARKER'" zpty -r comp nested "*LEAFMARKER*"