From 2dd4e9947084ec72cf6ba1b3092cbe3abd7c312b Mon Sep 17 00:00:00 2001 From: Taran Pelkey Date: Fri, 28 Aug 2026 19:21:42 -0500 Subject: [PATCH 1/3] Replace bash-completion machinery with posener/complete The built-in completion support worked by having the shell script append a hidden --generate-bash-completion flag, which App.Run stripped off before flag parsing and answered by calling a user-supplied BashCompleteFunc that printed candidate names. Applications had to write those callbacks by hand, and completion of flag values was not supported at all. Replace it with github.com/posener/complete. App.Run now detects a completion request via COMP_LINE and answers it from a complete.Command tree built by walking the app's commands, aliases, flags and global flags. - Flag gains GetPredictor(); each generated flag type gains a CustomFlagPredictor field for completing its value. - Command.BashComplete is replaced by Command.CustomCompletePredictor for completing positional arguments. - New SetupShellCompletion (install.go) installs and uninstalls the shell hook, replacing the hand-maintained autocomplete/ scripts. - BashCompleteFunc, BashCompletionFlag, DefaultAppComplete, ShowCompletions, ShowCommandCompletions and Context.shellComplete are removed. The code generator is fixed to run under python3 (NamedTemporaryFile needs an explicit text mode) and renamed to generate-flag-types.py; the doc comment in cli.go is reflowed to gofmt's current style. --- app.go | 44 ++-- app_test.go | 101 +------- autocomplete/bash_autocomplete | 16 -- autocomplete/zsh_autocomplete | 5 - cli.go | 28 +-- command.go | 16 +- complete.go | 81 +++++++ complete_test.go | 208 +++++++++++++++++ context.go | 9 +- flag.go | 9 +- flag_generated.go | 215 +++++++++++++----- funcs.go | 3 - generate-flag-types => generate-flag-types.py | 28 ++- go.mod | 7 + go.sum | 16 ++ help.go | 69 ------ install.go | 121 ++++++++++ install_test.go | 90 ++++++++ 18 files changed, 749 insertions(+), 317 deletions(-) delete mode 100755 autocomplete/bash_autocomplete delete mode 100644 autocomplete/zsh_autocomplete create mode 100644 complete.go create mode 100644 complete_test.go rename generate-flag-types => generate-flag-types.py (88%) create mode 100644 go.sum create mode 100644 install.go create mode 100644 install_test.go diff --git a/app.go b/app.go index 1341281c2c..fbb49292cb 100644 --- a/app.go +++ b/app.go @@ -50,8 +50,6 @@ type App struct { // Set when the app is run via RunAsSubcommand; the version flag belongs to // the top-level app only runningAsSubcommand bool - // An action to execute when the bash-completion flag is set - BashComplete BashCompleteFunc // An action to execute before any subcommands are run, but after the context is ready // If a non-nil error is returned, no subcommands are run Before BeforeFunc @@ -128,16 +126,15 @@ func compileTime() time.Time { // Usage, Version and Action. func NewApp() *App { return &App{ - Name: filepath.Base(os.Args[0]), - HelpName: filepath.Base(os.Args[0]), - Usage: "A new cli application", - UsageText: "", - Version: "0.0.0", - BashComplete: DefaultAppComplete, - Action: helpCommand.Action, - Compiled: compileTime(), - Writer: os.Stdout, - HelpWriter: os.Stdout, + Name: filepath.Base(os.Args[0]), + HelpName: filepath.Base(os.Args[0]), + Usage: "A new cli application", + UsageText: "", + Version: "0.0.0", + Action: helpCommand.Action, + Compiled: compileTime(), + Writer: os.Stdout, + HelpWriter: os.Stdout, } } @@ -190,13 +187,13 @@ func (a *App) Setup() { func (a *App) Run(arguments []string) (err error) { a.Setup() - // handle the completion flag 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 - // note that we can only do this because the shell autocomplete function - // always appends the completion flag at the end of the command - shellComplete, arguments := checkShellCompleteFlag(a, arguments) + // Answer shell-completion requests: when completion is enabled and the + // process was spawned by the shell for completion (COMP_LINE is set), + // emit predictions and return without running the command. + if a.EnableBashCompletion && os.Getenv("COMP_LINE") != "" { + a.runShellCompletion() + return nil + } // parse flags flags := a.resolveFlags() @@ -213,11 +210,6 @@ func (a *App) Run(arguments []string) (err error) { fmt.Fprintln(a.Writer, nerr) return nerr } - context.shellComplete = shellComplete - - if checkCompletions(context) { - return nil - } if err != nil { if onUsageError := a.resolveOnUsageError(); onUsageError != nil { @@ -330,10 +322,6 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { return nerr } - if checkCompletions(context) { - return nil - } - if err != nil { if onUsageError := a.resolveOnUsageError(); onUsageError != nil { err = onUsageError(context, err, true) diff --git a/app_test.go b/app_test.go index 0da939c1a0..74d2cdd44e 100644 --- a/app_test.go +++ b/app_test.go @@ -11,6 +11,8 @@ import ( "reflect" "strings" "testing" + + "github.com/posener/complete" ) var ( @@ -27,7 +29,7 @@ func init() { } type opCounts struct { - Total, BashComplete, OnUsageError, Before, CommandNotFound, Action, After, SubCommand int + Total, OnUsageError, Before, CommandNotFound, Action, After, SubCommand int } func ExampleApp_Run() { @@ -228,43 +230,6 @@ func ExampleApp_Run_subcommandNoAction() { // --help, -h show help } -func ExampleApp_Run_bashComplete() { - // set args for examples sake - os.Args = []string{"greet", "--generate-bash-completion"} - - app := NewApp() - app.Name = "greet" - app.EnableBashCompletion = true - app.Commands = []Command{ - { - Name: "describeit", - Aliases: []string{"d"}, - Usage: "use it to see a description", - Description: "This is how we describe describeit the function", - Action: func(c *Context) error { - fmt.Printf("i like to describe things") - return nil - }, - }, { - Name: "next", - Usage: "next example", - Description: "more stuff to see when generating bash completion", - Action: func(c *Context) error { - fmt.Printf("the next example") - return nil - }, - }, - } - - app.Run(os.Args) - // Output: - // describeit - // d - // next - // help - // h -} - func TestApp_Run(t *testing.T) { s := "" @@ -884,12 +849,6 @@ func TestApp_OrderOfOperations(t *testing.T) { resetCounts := func() { counts = &opCounts{} } app := NewApp() - app.EnableBashCompletion = true - app.BashComplete = func(c *Context) { - counts.Total++ - counts.BashComplete = counts.Total - } - app.OnUsageError = func(c *Context, err error, isSubcommand bool) error { counts.Total++ counts.OnUsageError = counts.Total @@ -950,12 +909,6 @@ func TestApp_OrderOfOperations(t *testing.T) { resetCounts() - _ = app.Run([]string{"command", "--generate-bash-completion"}) - expect(t, counts.BashComplete, 1) - expect(t, counts.Total, 1) - - resetCounts() - oldOnUsageError := app.OnUsageError app.OnUsageError = nil _ = app.Run([]string{"command", "--nope"}) @@ -1596,6 +1549,10 @@ func (c *customBoolFlag) Apply(set *flag.FlagSet) { set.String(c.Nombre, c.Nombre, "") } +func (c *customBoolFlag) GetPredictor() complete.Predictor { + return complete.PredictNothing +} + func TestCustomFlagsUnused(t *testing.T) { app := NewApp() app.Flags = []Flag{&customBoolFlag{"custom"}} @@ -1650,47 +1607,3 @@ func TestHandleAction_WithUnknownPanic(t *testing.T) { } app.Action(NewContext(app, fs, nil)) } - -func TestShellCompletionForIncompleteFlags(t *testing.T) { - app := NewApp() - app.Flags = []Flag{ - IntFlag{ - Name: "test-completion", - }, - } - app.EnableBashCompletion = true - app.BashComplete = func(ctx *Context) { - for _, command := range ctx.App.Commands { - if command.Hidden { - continue - } - - for _, name := range command.Names() { - fmt.Fprintln(ctx.App.Writer, name) - } - } - - for _, flag := range ctx.App.Flags { - for _, name := range strings.Split(flag.GetName(), ",") { - if name == BashCompletionFlag.GetName() { - continue - } - - switch name = strings.TrimSpace(name); len(name) { - case 0: - case 1: - fmt.Fprintln(ctx.App.Writer, "-"+name) - default: - fmt.Fprintln(ctx.App.Writer, "--"+name) - } - } - } - } - app.Action = func(ctx *Context) error { - return fmt.Errorf("should not get here") - } - err := app.Run([]string{"", "--test-completion", "--" + BashCompletionFlag.GetName()}) - if err != nil { - t.Errorf("app should not return an error: %s", err) - } -} diff --git a/autocomplete/bash_autocomplete b/autocomplete/bash_autocomplete deleted file mode 100755 index 37d9c14513..0000000000 --- a/autocomplete/bash_autocomplete +++ /dev/null @@ -1,16 +0,0 @@ -#! /bin/bash - -: ${PROG:=$(basename ${BASH_SOURCE})} - -_cli_bash_autocomplete() { - local cur opts base - COMPREPLY=() - cur="${COMP_WORDS[COMP_CWORD]}" - opts=$( ${COMP_WORDS[@]:0:$COMP_CWORD} --generate-bash-completion ) - COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) ) - return 0 -} - -complete -F _cli_bash_autocomplete $PROG - -unset PROG diff --git a/autocomplete/zsh_autocomplete b/autocomplete/zsh_autocomplete deleted file mode 100644 index 5430a18f95..0000000000 --- a/autocomplete/zsh_autocomplete +++ /dev/null @@ -1,5 +0,0 @@ -autoload -U compinit && compinit -autoload -U bashcompinit && bashcompinit - -script_dir=$(dirname $0) -source ${script_dir}/bash_autocomplete diff --git a/cli.go b/cli.go index 74fd101f45..c5c6f4cf01 100644 --- a/cli.go +++ b/cli.go @@ -1,21 +1,23 @@ // Package cli provides a minimal framework for creating and organizing command line // Go applications. cli is designed to be easy to understand and write, the most simple // cli application can be written as follows: -// func main() { -// cli.NewApp().Run(os.Args) -// } +// +// func main() { +// cli.NewApp().Run(os.Args) +// } // // Of course this application does not do much, so let's make this an actual application: -// func main() { -// app := cli.NewApp() -// app.Name = "greet" -// app.Usage = "say a greeting" -// app.Action = func(c *cli.Context) error { -// println("Greetings") -// } // -// app.Run(os.Args) -// } +// func main() { +// app := cli.NewApp() +// app.Name = "greet" +// app.Usage = "say a greeting" +// app.Action = func(c *cli.Context) error { +// println("Greetings") +// } +// +// app.Run(os.Args) +// } package cli -//go:generate python ./generate-flag-types cli -i flag-types.json -o flag_generated.go +//go:generate python3 ./generate-flag-types.py cli -i flag-types.json -o flag_generated.go diff --git a/command.go b/command.go index 74a27fc056..9af3829cf0 100644 --- a/command.go +++ b/command.go @@ -6,6 +6,8 @@ import ( "slices" "sort" "strings" + + "github.com/posener/complete" ) // Command is a subcommand for a cli.App. @@ -26,8 +28,9 @@ type Command struct { ArgsUsage string // The category the command is part of Category string - // The function to call when checking for bash command completions - BashComplete BashCompleteFunc + // CustomCompletePredictor predicts positional-argument completions for + // this command during shell completion. + CustomCompletePredictor complete.Predictor // An action to execute before any sub-subcommands are run, but after the context is ready // If a non-nil error is returned, no sub-subcommands are run Before BeforeFunc @@ -187,9 +190,6 @@ func (c Command) Run(ctx *Context) (err error) { context := NewContext(ctx.App, set, ctx) context.Command = c - if checkCommandCompletions(context, c.Name) { - return nil - } if err != nil { if onUsageError := c.resolveOnUsageError(context); onUsageError != nil { @@ -325,12 +325,6 @@ func (c Command) startApp(ctx *Context) error { sort.Sort(app.categories) - // bash completion - app.EnableBashCompletion = ctx.App.EnableBashCompletion - if c.BashComplete != nil { - app.BashComplete = c.BashComplete - } - // set the actions app.Before = c.resolveBefore(ctx) app.After = c.resolveAfter(ctx) diff --git a/complete.go b/complete.go new file mode 100644 index 0000000000..ded5d94494 --- /dev/null +++ b/complete.go @@ -0,0 +1,81 @@ +package cli + +import ( + "os" + "path/filepath" + "strings" + + "github.com/posener/complete" +) + +// flagsToCompleteFlags transforms a cli.Flag to complete.Flags +// understood by posener/complete library. +func flagsToCompleteFlags(flags []Flag) complete.Flags { + complFlags := make(complete.Flags) + flags = visibleFlags(flags) + for _, f := range flags { + for _, s := range strings.Split(f.GetName(), ",") { + var flagName string + s = strings.TrimSpace(s) + if len(s) == 1 { + flagName = "-" + s + } else { + flagName = "--" + s + } + complFlags[flagName] = f.GetPredictor() + } + } + return complFlags +} + +// cmdToCompleteCmd recursively transforms a Command (and its Subcommands) into +// a complete.Command understood by the posener/complete library. Hidden +// commands are skipped; aliases are registered alongside the primary name. The +// argument and flag-value predictors come from the command's own +// CustomCompletePredictor / CustomFlagPredictor fields. +func cmdToCompleteCmd(cmd Command, parentSubcommandMap complete.Commands) { + if cmd.Hidden { + return + } + + sub := make(complete.Commands) + for _, subCmd := range cmd.Subcommands { + cmdToCompleteCmd(subCmd, sub) + } + + compCmd := complete.Command{ + Sub: sub, + Args: cmd.CustomCompletePredictor, + Flags: flagsToCompleteFlags(cmd.Flags), + } + parentSubcommandMap[cmd.Name] = compCmd + if cmd.HiddenAliases { + return + } + for _, alias := range cmd.Aliases { + parentSubcommandMap[alias] = compCmd + } +} + +// shellCompleteCommand builds the root complete.Command for the application by +// walking its visible commands and global flags. +func (a *App) shellCompleteCommand() complete.Command { + sub := make(complete.Commands) + for _, cmd := range a.Commands { + cmdToCompleteCmd(cmd, sub) + } + return complete.Command{ + Sub: sub, + Flags: flagsToCompleteFlags(a.Flags), + GlobalFlags: flagsToCompleteFlags(a.GlobalFlags), + } +} + +// runShellCompletion answers a single shell-completion request described by the +// COMP_LINE / COMP_POINT environment variables and prints the predicted +// options. It is invoked from App.Run when EnableBashCompletion is set and the +// process was spawned by the shell for completion. The name passed to posener +// must match how the shell invoked the binary, hence filepath.Base(os.Args[0]). +func (a *App) runShellCompletion() { + complete.New(filepath.Base(os.Args[0]), a.shellCompleteCommand()).Complete() +} diff --git a/complete_test.go b/complete_test.go new file mode 100644 index 0000000000..3465cdc976 --- /dev/null +++ b/complete_test.go @@ -0,0 +1,208 @@ +package cli + +import ( + "bufio" + "os" + "sort" + "strconv" + "strings" + "testing" + + "github.com/posener/complete" +) + +// newCompletionTestApp builds an app exercising every completion path: +// commands with aliases (including a hidden top-level alias and a hidden +// nested alias), a hidden command, a subcommand tree (with a hidden child), +// a leaf command carrying an arg predictor, a command with both a visible +// and a hidden flag-value predictor, and app-level (root-only) and global +// (inherited) flags, each with a visible and a hidden one. Help/version are +// hidden to keep predictions clean. +func newCompletionTestApp() *App { + app := NewApp() + app.Name = "prog" + app.HideHelp = true + app.HideHelpCommand = true + app.HideVersion = true + app.EnableBashCompletion = true + app.Flags = []Flag{ + StringFlag{Name: "verbosity", CustomFlagPredictor: complete.PredictSet("debug", "info")}, + StringFlag{Name: "secret-app-flag", Hidden: true}, + } + app.GlobalFlags = []Flag{ + StringFlag{Name: "profile", CustomFlagPredictor: complete.PredictSet("dev", "prod")}, + StringFlag{Name: "secret-global-flag", Hidden: true}, + } + app.Commands = []Command{ + { + Name: "widget", + Aliases: []string{"w"}, + Subcommands: Commands{ + // mk is a hidden alias of make: exercises the nested + // HiddenAliases path alongside the top-level one on pick. + {Name: "make", Aliases: []string{"mk"}, HiddenAliases: true}, + {Name: "list"}, + {Name: "internal", Hidden: true}, + }, + }, + { + Name: "pick", + Aliases: []string{"pk"}, + HiddenAliases: true, + CustomCompletePredictor: complete.PredictSet("alpha", "beta"), + }, + { + Name: "paint", + Flags: []Flag{ + StringFlag{Name: "color", CustomFlagPredictor: complete.PredictSet("green", "red")}, + StringFlag{Name: "secret-color-flag", Hidden: true}, + }, + }, + { + Name: "secret", + Hidden: true, + }, + } + return app +} + +func TestShellCompletion(t *testing.T) { + app := newCompletionTestApp() + + cases := []struct { + name string + line string + want []string + }{ + { + name: "top-level command names, hidden excluded, aliases included, hidden alias excluded", + line: "prog ", + want: []string{"paint", "pick", "w", "widget"}, + }, + { + name: "subcommand recursion, hidden child excluded, hidden nested alias excluded", + line: "prog widget ", + want: []string{"list", "make"}, + }, + { + name: "alias resolves to same subcommands", + line: "prog w ", + want: []string{"list", "make"}, + }, + { + name: "arg predictor on leaf command", + line: "prog pick ", + want: []string{"alpha", "beta"}, + }, + { + name: "arg predictor honors prefix", + line: "prog pick a", + want: []string{"alpha"}, + }, + { + name: "app-level flag name completion at root, hidden app flag excluded", + line: "prog -", + want: []string{"--profile", "--verbosity"}, + }, + { + name: "app-level flag value prediction at root", + line: "prog --verbosity ", + want: []string{"debug", "info"}, + }, + { + name: "app-level flags are not inherited by commands, unlike global flags", + line: "prog paint -", + want: []string{"--color", "--profile"}, + }, + { + name: "flag value prediction", + line: "prog paint --color ", + want: []string{"green", "red"}, + }, + { + name: "global flag value predicted inside a child command", + line: "prog widget --profile ", + want: []string{"dev", "prod"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := completeLine(t, app, tc.line) + if !eqStrings(got, tc.want) { + t.Errorf("line %q: got %v, want %v", tc.line, got, tc.want) + } + }) + } +} + +// TestShellCompletionDisabled verifies that without EnableBashCompletion the +// COMP_LINE path is not taken. +func TestShellCompletionDisabled(t *testing.T) { + app := newCompletionTestApp() + app.EnableBashCompletion = false + // No-op action so the normal (non-completion) path produces no output. + app.Action = func(*Context) error { return nil } + + got := completeLine(t, app, "prog ") + if len(got) != 0 { + t.Errorf("expected no completion output when disabled, got %v", got) + } +} + +// completeLine drives a single COMP_LINE completion request through app.Run and +// returns the predicted options, sorted, capturing what posener writes to +// stdout. +func completeLine(t *testing.T, app *App, line string) []string { + t.Helper() + + if err := os.Setenv("COMP_LINE", line); err != nil { + t.Fatal(err) + } + if err := os.Setenv("COMP_POINT", strconv.Itoa(len(line))); err != nil { + t.Fatal(err) + } + defer os.Unsetenv("COMP_LINE") + defer os.Unsetenv("COMP_POINT") + + old := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stdout = w + + runErr := app.Run([]string{"prog"}) + + w.Close() + os.Stdout = old + + if runErr != nil { + t.Fatalf("Run returned error during completion: %v", runErr) + } + + var out []string + sc := bufio.NewScanner(r) + for sc.Scan() { + if s := strings.TrimSpace(sc.Text()); s != "" { + out = append(out, s) + } + } + if err := sc.Err(); err != nil { + t.Fatal(err) + } + sort.Strings(out) + return out +} + +func eqStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/context.go b/context.go index cb89e92a08..19de0d8482 100644 --- a/context.go +++ b/context.go @@ -15,7 +15,6 @@ import ( type Context struct { App *App Command Command - shellComplete bool flagSet *flag.FlagSet setFlags map[string]bool parentContext *Context @@ -23,13 +22,7 @@ type Context struct { // NewContext creates a new context. For use in when invoking an App or Command action. func NewContext(app *App, set *flag.FlagSet, parentCtx *Context) *Context { - c := &Context{App: app, flagSet: set, parentContext: parentCtx} - - if parentCtx != nil { - c.shellComplete = parentCtx.shellComplete - } - - return c + return &Context{App: app, flagSet: set, parentContext: parentCtx} } // NumFlags returns the number of flags set diff --git a/flag.go b/flag.go index 8547f5385d..8b8fbfc483 100644 --- a/flag.go +++ b/flag.go @@ -9,16 +9,12 @@ import ( "strings" "syscall" "time" + + "github.com/posener/complete" ) const defaultPlaceholder = "value" -// BashCompletionFlag enables bash-completion for all commands and subcommands -var BashCompletionFlag Flag = BoolFlag{ - Name: "generate-bash-completion", - Hidden: true, -} - // VersionFlag prints the version for the application var VersionFlag Flag = BoolFlag{ Name: "version, v", @@ -60,6 +56,7 @@ type Flag interface { // Apply Flag settings to the given flag set Apply(*flag.FlagSet) GetName() string + GetPredictor() complete.Predictor } // errorableFlag is an interface that allows us to return errors during apply diff --git a/flag_generated.go b/flag_generated.go index 491b61956c..8890556259 100644 --- a/flag_generated.go +++ b/flag_generated.go @@ -4,6 +4,8 @@ import ( "flag" "strconv" "time" + + "github.com/posener/complete" ) // WARNING: This file is generated! @@ -28,6 +30,12 @@ func (f BoolFlag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f BoolFlag) GetPredictor() complete.Predictor { + return complete.PredictNothing +} + // Bool looks up the value of a local BoolFlag, returns // false if not found func (c *Context) Bool(name string) bool { @@ -75,6 +83,12 @@ func (f BoolTFlag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f BoolTFlag) GetPredictor() complete.Predictor { + return complete.PredictNothing +} + // BoolT looks up the value of a local BoolTFlag, returns // false if not found func (c *Context) BoolT(name string) bool { @@ -104,12 +118,13 @@ func lookupBoolT(name string, set *flag.FlagSet) bool { // DurationFlag is a flag with type time.Duration (see https://golang.org/pkg/time/#ParseDuration) type DurationFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value time.Duration - Destination *time.Duration + Name string + Usage string + EnvVar string + Hidden bool + Value time.Duration + Destination *time.Duration + CustomFlagPredictor complete.Predictor } // String returns a readable representation of this value @@ -123,6 +138,12 @@ func (f DurationFlag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f DurationFlag) GetPredictor() complete.Predictor { + return f.CustomFlagPredictor +} + // Duration looks up the value of a local DurationFlag, returns // 0 if not found func (c *Context) Duration(name string) time.Duration { @@ -152,12 +173,13 @@ func lookupDuration(name string, set *flag.FlagSet) time.Duration { // Float64Flag is a flag with type float64 type Float64Flag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value float64 - Destination *float64 + Name string + Usage string + EnvVar string + Hidden bool + Value float64 + Destination *float64 + CustomFlagPredictor complete.Predictor } // String returns a readable representation of this value @@ -171,6 +193,12 @@ func (f Float64Flag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f Float64Flag) GetPredictor() complete.Predictor { + return f.CustomFlagPredictor +} + // Float64 looks up the value of a local Float64Flag, returns // 0 if not found func (c *Context) Float64(name string) float64 { @@ -200,11 +228,12 @@ func lookupFloat64(name string, set *flag.FlagSet) float64 { // GenericFlag is a flag with type Generic type GenericFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value Generic + Name string + Usage string + EnvVar string + Hidden bool + Value Generic + CustomFlagPredictor complete.Predictor } // String returns a readable representation of this value @@ -218,6 +247,12 @@ func (f GenericFlag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f GenericFlag) GetPredictor() complete.Predictor { + return f.CustomFlagPredictor +} + // Generic looks up the value of a local GenericFlag, returns // nil if not found func (c *Context) Generic(name string) interface{} { @@ -247,12 +282,13 @@ func lookupGeneric(name string, set *flag.FlagSet) interface{} { // Int64Flag is a flag with type int64 type Int64Flag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value int64 - Destination *int64 + Name string + Usage string + EnvVar string + Hidden bool + Value int64 + Destination *int64 + CustomFlagPredictor complete.Predictor } // String returns a readable representation of this value @@ -266,6 +302,12 @@ func (f Int64Flag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f Int64Flag) GetPredictor() complete.Predictor { + return f.CustomFlagPredictor +} + // Int64 looks up the value of a local Int64Flag, returns // 0 if not found func (c *Context) Int64(name string) int64 { @@ -295,12 +337,13 @@ func lookupInt64(name string, set *flag.FlagSet) int64 { // IntFlag is a flag with type int type IntFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value int - Destination *int + Name string + Usage string + EnvVar string + Hidden bool + Value int + Destination *int + CustomFlagPredictor complete.Predictor } // String returns a readable representation of this value @@ -314,6 +357,12 @@ func (f IntFlag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f IntFlag) GetPredictor() complete.Predictor { + return f.CustomFlagPredictor +} + // Int looks up the value of a local IntFlag, returns // 0 if not found func (c *Context) Int(name string) int { @@ -343,11 +392,12 @@ func lookupInt(name string, set *flag.FlagSet) int { // IntSliceFlag is a flag with type *IntSlice type IntSliceFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value *IntSlice + Name string + Usage string + EnvVar string + Hidden bool + Value *IntSlice + CustomFlagPredictor complete.Predictor } // String returns a readable representation of this value @@ -361,6 +411,12 @@ func (f IntSliceFlag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f IntSliceFlag) GetPredictor() complete.Predictor { + return f.CustomFlagPredictor +} + // IntSlice looks up the value of a local IntSliceFlag, returns // nil if not found func (c *Context) IntSlice(name string) []int { @@ -390,11 +446,12 @@ func lookupIntSlice(name string, set *flag.FlagSet) []int { // Int64SliceFlag is a flag with type *Int64Slice type Int64SliceFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value *Int64Slice + Name string + Usage string + EnvVar string + Hidden bool + Value *Int64Slice + CustomFlagPredictor complete.Predictor } // String returns a readable representation of this value @@ -408,6 +465,12 @@ func (f Int64SliceFlag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f Int64SliceFlag) GetPredictor() complete.Predictor { + return f.CustomFlagPredictor +} + // Int64Slice looks up the value of a local Int64SliceFlag, returns // nil if not found func (c *Context) Int64Slice(name string) []int64 { @@ -437,12 +500,13 @@ func lookupInt64Slice(name string, set *flag.FlagSet) []int64 { // StringFlag is a flag with type string type StringFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value string - Destination *string + Name string + Usage string + EnvVar string + Hidden bool + Value string + Destination *string + CustomFlagPredictor complete.Predictor } // String returns a readable representation of this value @@ -456,6 +520,12 @@ func (f StringFlag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f StringFlag) GetPredictor() complete.Predictor { + return f.CustomFlagPredictor +} + // String looks up the value of a local StringFlag, returns // "" if not found func (c *Context) String(name string) string { @@ -485,11 +555,12 @@ func lookupString(name string, set *flag.FlagSet) string { // StringSliceFlag is a flag with type *StringSlice type StringSliceFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value *StringSlice + Name string + Usage string + EnvVar string + Hidden bool + Value *StringSlice + CustomFlagPredictor complete.Predictor } // String returns a readable representation of this value @@ -503,6 +574,12 @@ func (f StringSliceFlag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f StringSliceFlag) GetPredictor() complete.Predictor { + return f.CustomFlagPredictor +} + // StringSlice looks up the value of a local StringSliceFlag, returns // nil if not found func (c *Context) StringSlice(name string) []string { @@ -532,12 +609,13 @@ func lookupStringSlice(name string, set *flag.FlagSet) []string { // Uint64Flag is a flag with type uint64 type Uint64Flag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value uint64 - Destination *uint64 + Name string + Usage string + EnvVar string + Hidden bool + Value uint64 + Destination *uint64 + CustomFlagPredictor complete.Predictor } // String returns a readable representation of this value @@ -551,6 +629,12 @@ func (f Uint64Flag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f Uint64Flag) GetPredictor() complete.Predictor { + return f.CustomFlagPredictor +} + // Uint64 looks up the value of a local Uint64Flag, returns // 0 if not found func (c *Context) Uint64(name string) uint64 { @@ -580,12 +664,13 @@ func lookupUint64(name string, set *flag.FlagSet) uint64 { // UintFlag is a flag with type uint type UintFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value uint - Destination *uint + Name string + Usage string + EnvVar string + Hidden bool + Value uint + Destination *uint + CustomFlagPredictor complete.Predictor } // String returns a readable representation of this value @@ -599,6 +684,12 @@ func (f UintFlag) GetName() string { return f.Name } +// GetPredictor returns the predictor to use for shell completion +// of this flag's value +func (f UintFlag) GetPredictor() complete.Predictor { + return f.CustomFlagPredictor +} + // Uint looks up the value of a local UintFlag, returns // 0 if not found func (c *Context) Uint(name string) uint { diff --git a/funcs.go b/funcs.go index cba5e6cb0c..2451eedada 100644 --- a/funcs.go +++ b/funcs.go @@ -1,8 +1,5 @@ package cli -// BashCompleteFunc is an action to execute when the bash-completion flag is set -type BashCompleteFunc func(*Context) - // BeforeFunc is an action to execute before any subcommands are run, but after // the context is ready if a non-nil error is returned, no subcommands are run type BeforeFunc func(*Context) error diff --git a/generate-flag-types b/generate-flag-types.py similarity index 88% rename from generate-flag-types rename to generate-flag-types.py index 75acc88e54..bb3d891705 100755 --- a/generate-flag-types +++ b/generate-flag-types.py @@ -103,7 +103,7 @@ def main(sysargs=sys.argv[:]): def _generate_flag_types(writefunc, output_go, input_json): types = json.load(input_json) - tmp = tempfile.NamedTemporaryFile(suffix='.go', delete=False) + tmp = tempfile.NamedTemporaryFile(mode='w', suffix='.go', delete=False) writefunc(tmp, types) tmp.close() @@ -129,6 +129,10 @@ def _write_cli_flag_types(outfile, types): _fwrite(outfile, """\ package cli + import ( + "github.com/posener/complete" + ) + // WARNING: This file is generated! """) @@ -155,6 +159,11 @@ def _write_cli_flag_types(outfile, types): Destination *{type} """.format(**typedef)) + if typedef['value']: + _fwrite(outfile, """\ + CustomFlagPredictor complete.Predictor + """.format(**typedef)) + _fwrite(outfile, "\n}\n\n") _fwrite(outfile, """\ @@ -169,6 +178,22 @@ def _write_cli_flag_types(outfile, types): return f.Name }} + """.format(**typedef)) + + predictor_body = ( + "return f.CustomFlagPredictor" if typedef['value'] + else "return complete.PredictNothing" + ) + _fwrite(outfile, """\ + // GetPredictor returns the predictor to use for shell completion + // of this flag's value + func (f {name}Flag) GetPredictor() complete.Predictor {{ + {predictor_body} + }} + + """.format(name=typedef['name'], predictor_body=predictor_body)) + + _fwrite(outfile, """\ // {name} looks up the value of a local {name}Flag, returns // {context_default} if not found func (c *Context) {name}(name string) {context_type} {{ @@ -197,7 +222,6 @@ def _write_cli_flag_types(outfile, types): }} """.format(**typedef)) - def _fwrite(outfile, text): print(textwrap.dedent(text), end='', file=outfile) diff --git a/go.mod b/go.mod index 015388303b..8717495523 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,10 @@ module github.com/minio/cli/v2 go 1.22 + +require github.com/posener/complete v1.2.3 + +require ( + github.com/hashicorp/errwrap v1.0.0 // indirect + github.com/hashicorp/go-multierror v1.0.0 // indirect +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000000..e6ca257962 --- /dev/null +++ b/go.sum @@ -0,0 +1,16 @@ +github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= +github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= +github.com/hashicorp/go-multierror v1.0.0 h1:iVjPR7a6H0tWELX5NxNe7bYopibicUzc7uPribsnS6o= +github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/posener/complete v1.2.3 h1:NP0eAhjcjImqslEwo/1hq7gpajME0fTLTezBKDqfXqo= +github.com/posener/complete v1.2.3/go.mod h1:WZIdtGGp+qx0sLrYKtIRAruyNpv6hFCicSgv7Sy7s/s= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= +github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/help.go b/help.go index ce0c431156..7d20b77060 100644 --- a/help.go +++ b/help.go @@ -150,18 +150,6 @@ func ShowAppHelp(c *Context) (err error) { return nil } -// DefaultAppComplete prints the list of subcommands as the default app completion method -func DefaultAppComplete(c *Context) { - for _, command := range c.App.Commands { - if command.Hidden { - continue - } - for _, name := range command.NamesWithHiddenAliases() { - fmt.Fprintln(c.App.Writer, name) - } - } -} - // ShowCommandHelpAndExit - exits with code after showing help func ShowCommandHelpAndExit(c *Context, command string, code int) { ShowCommandHelp(c, command) @@ -232,22 +220,6 @@ func printVersion(c *Context) { fmt.Fprintf(c.App.Writer, "%v version %v\n", c.App.Name, c.App.Version) } -// ShowCompletions prints the lists of commands within a given context -func ShowCompletions(c *Context) { - a := c.App - if a != nil && a.BashComplete != nil { - a.BashComplete(c) - } -} - -// ShowCommandCompletions prints the custom completions for a given command -func ShowCommandCompletions(ctx *Context, command string) { - c := ctx.App.Command(command) - if c != nil && c.BashComplete != nil { - c.BashComplete(ctx) - } -} - func printHelpCustom(out io.Writer, templ string, data interface{}, customFunc map[string]interface{}) { funcMap := template.FuncMap{ "join": strings.Join, @@ -317,44 +289,3 @@ func checkSubcommandHelp(c *Context) bool { return false } - -func checkShellCompleteFlag(a *App, arguments []string) (bool, []string) { - if !a.EnableBashCompletion { - return false, arguments - } - - pos := len(arguments) - 1 - lastArg := arguments[pos] - - if lastArg != "--"+BashCompletionFlag.GetName() { - return false, arguments - } - - return true, arguments[:pos] -} - -func checkCompletions(c *Context) bool { - if !c.shellComplete { - return false - } - - if args := c.Args(); args.Present() { - name := args.First() - if cmd := c.App.Command(name); cmd != nil { - // let the command handle the completion - return false - } - } - - ShowCompletions(c) - return true -} - -func checkCommandCompletions(c *Context, name string) bool { - if !c.shellComplete { - return false - } - - ShowCommandCompletions(c, name) - return true -} diff --git a/install.go b/install.go new file mode 100644 index 0000000000..f132a198cf --- /dev/null +++ b/install.go @@ -0,0 +1,121 @@ +package cli + +import ( + "errors" + "os" + "os/exec" + "path/filepath" + "runtime" + "strconv" + "strings" + + completeinstall "github.com/posener/complete/cmd/install" +) + +// Errors returned by SetupShellCompletion so callers can switch on them to +// format their own user-facing messages. The framework itself prints nothing. +var ( + // ErrCompletionUnsupportedOS is returned when shell completion cannot be + // installed on the current operating system (e.g. Windows). + ErrCompletionUnsupportedOS = errors.New("shell completion is not supported on this operating system") + // ErrCompletionUnsupportedShell is returned when the detected shell is not + // one of the supported shells (bash, zsh, fish). + ErrCompletionUnsupportedShell = errors.New("unsupported shell") +) + +// supportedCompletionShells is the set of shells for which completion can be +// installed. posener/complete writes to whichever of these it finds present. +var supportedCompletionShells = map[string]bool{ + "bash": true, + "zsh": true, + "fish": true, +} + +// DetectShell reports the user's shell name (lowercased base name, e.g. "bash") +// and whether it was determined from the $SHELL environment variable. When +// $SHELL is unset it falls back to inspecting the parent process; fromEnv is +// false in that case. On Windows it returns early with fromEnv true and an +// empty name. +func DetectShell() (name string, fromEnv bool, err error) { + shellName := os.Getenv("SHELL") + if shellName != "" || runtime.GOOS == "windows" { + return strings.ToLower(filepath.Base(shellName)), true, nil + } + + ppid := os.Getppid() + out, err := exec.Command("ps", "-p", strconv.Itoa(ppid), "-o", "comm=").Output() + if err != nil { + return "", false, err + } + shellName = strings.TrimSpace(string(out)) + return strings.ToLower(filepath.Base(shellName)), false, nil +} + +// installer and isInstalled are seams over posener/complete's real Install +// and IsInstalled, for tests. +var ( + installer = completeinstall.Install + isInstalled = completeinstall.IsInstalled +) + +// InstallShellCompletion registers cmd for shell completion in the user's shell +// rc files (bash/zsh/fish — whichever are present). It is a thin wrapper over +// posener/complete's installer. +func InstallShellCompletion(cmd string) error { return installer(cmd) } + +// UninstallShellCompletion removes cmd's shell-completion registration. +func UninstallShellCompletion(cmd string) error { return completeinstall.Uninstall(cmd) } + +// IsShellCompletionInstalled reports whether cmd's completion is already +// registered in any of the user's shell rc files. +func IsShellCompletionInstalled(cmd string) bool { return isInstalled(cmd) } + +// ShellCompletionResult describes the outcome of SetupShellCompletion so the +// caller can render an appropriate message. +type ShellCompletionResult struct { + // Shell is the detected shell name (e.g. "bash"). + Shell string + // DetectedFromEnv reports whether Shell came from $SHELL (vs. a fallback). + DetectedFromEnv bool + // AlreadyInstalled reports whether completion was already registered, in + // which case no changes were made. Best-effort: if InstallShellCompletion + // partially fails on an unrelated shell config, this may report true even + // though Shell's own config was just freshly written. + AlreadyInstalled bool +} + +// SetupShellCompletion is the one-call convenience that installs shell +// completion for cmd: it checks OS support, detects and validates the shell, +// and registers completion. It performs no output; callers inspect the +// returned result and error to present their own messages. Returns +// ErrCompletionUnsupportedOS or ErrCompletionUnsupportedShell for the +// respective unsupported cases. +func SetupShellCompletion(cmd string) (ShellCompletionResult, error) { + var res ShellCompletionResult + + if runtime.GOOS == "windows" { + return res, ErrCompletionUnsupportedOS + } + + shell, fromEnv, err := DetectShell() + if err != nil { + return res, err + } + res.Shell = shell + res.DetectedFromEnv = fromEnv + + if !supportedCompletionShells[shell] { + return res, ErrCompletionUnsupportedShell + } + + if installErr := InstallShellCompletion(cmd); installErr != nil { + // Install() may have partially failed (e.g. some shell config + // already had it) while still succeeding for shell; re-check actual + // disk state rather than trust the error alone. + if !isInstalled(cmd) { + return res, installErr + } + res.AlreadyInstalled = true + } + return res, nil +} diff --git a/install_test.go b/install_test.go new file mode 100644 index 0000000000..4ace849e2c --- /dev/null +++ b/install_test.go @@ -0,0 +1,90 @@ +package cli + +import ( + "errors" + "testing" +) + +// withInstaller/withIsInstalled swap in fn for the real posener/complete +// call for the test's duration. Never let the real ones run here: they +// resolve the home dir via os/user.Current(), which ignores $HOME, so they'd +// touch your actual shell rc files. +func withInstaller(t *testing.T, fn func(cmd string) error) { + t.Helper() + old := installer + installer = fn + t.Cleanup(func() { installer = old }) +} + +func withIsInstalled(t *testing.T, fn func(cmd string) bool) { + t.Helper() + old := isInstalled + isInstalled = fn + t.Cleanup(func() { isInstalled = old }) +} + +func TestSetupShellCompletionFreshInstall(t *testing.T) { + t.Setenv("SHELL", "/bin/bash") + withInstaller(t, func(cmd string) error { return nil }) + + res, err := SetupShellCompletion("prog") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res.Shell != "bash" { + t.Errorf("res.Shell = %q, want %q", res.Shell, "bash") + } + if res.AlreadyInstalled { + t.Error("res.AlreadyInstalled = true, want false on a fresh install") + } +} + +// Regression test: SetupShellCompletion must not skip installing just +// because some *other* shell's config already had it — Install() always +// runs, and only a post-install disk check decides AlreadyInstalled. +func TestSetupShellCompletionAlreadyInstalled(t *testing.T) { + t.Setenv("SHELL", "/bin/bash") + withInstaller(t, func(cmd string) error { + return errors.New("1 error occurred: * already installed in /home/user/.bashrc") + }) + withIsInstalled(t, func(cmd string) bool { return true }) + + res, err := SetupShellCompletion("prog") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !res.AlreadyInstalled { + t.Error("res.AlreadyInstalled = false, want true") + } +} + +// A genuine install failure (not just some shell already having it) must +// propagate: the post-install disk check finds nothing installed either. +func TestSetupShellCompletionPropagatesGenuineFailure(t *testing.T) { + t.Setenv("SHELL", "/bin/bash") + wantErr := errors.New("open /home/user/.bashrc: permission denied") + withInstaller(t, func(cmd string) error { return wantErr }) + withIsInstalled(t, func(cmd string) bool { return false }) + + res, err := SetupShellCompletion("prog") + if !errors.Is(err, wantErr) { + t.Errorf("err = %v, want %v", err, wantErr) + } + if res.AlreadyInstalled { + t.Error("res.AlreadyInstalled = true, want false when install genuinely failed") + } +} + +func TestSetupShellCompletionUnsupportedShell(t *testing.T) { + t.Setenv("SHELL", "/usr/bin/tcsh") + // installer must not even be consulted for an unsupported shell. + withInstaller(t, func(cmd string) error { + t.Fatal("installer should not be called for an unsupported shell") + return nil + }) + + _, err := SetupShellCompletion("prog") + if !errors.Is(err, ErrCompletionUnsupportedShell) { + t.Errorf("err = %v, want %v", err, ErrCompletionUnsupportedShell) + } +} From 0ce40a7e626c55e1a3b13890929d61dee6304c3c Mon Sep 17 00:00:00 2001 From: Taran Pelkey Date: Mon, 31 Aug 2026 23:20:26 -0500 Subject: [PATCH 2/3] Rename --- app_test.go | 2 +- command.go | 6 +- complete.go | 8 +- complete_test.go | 14 +-- flag.go | 2 +- flag_generated.go | 246 ++++++++++++++++++++--------------------- generate-flag-types.py | 10 +- 7 files changed, 144 insertions(+), 144 deletions(-) diff --git a/app_test.go b/app_test.go index 74d2cdd44e..6460a811a4 100644 --- a/app_test.go +++ b/app_test.go @@ -1549,7 +1549,7 @@ func (c *customBoolFlag) Apply(set *flag.FlagSet) { set.String(c.Nombre, c.Nombre, "") } -func (c *customBoolFlag) GetPredictor() complete.Predictor { +func (c *customBoolFlag) GetCompleter() complete.Predictor { return complete.PredictNothing } diff --git a/command.go b/command.go index 9af3829cf0..86cb0dfea8 100644 --- a/command.go +++ b/command.go @@ -28,9 +28,9 @@ type Command struct { ArgsUsage string // The category the command is part of Category string - // CustomCompletePredictor predicts positional-argument completions for - // this command during shell completion. - CustomCompletePredictor complete.Predictor + // Completer predicts this command's positional arguments during shell + // completion. + Completer complete.Predictor // An action to execute before any sub-subcommands are run, but after the context is ready // If a non-nil error is returned, no sub-subcommands are run Before BeforeFunc diff --git a/complete.go b/complete.go index ded5d94494..73f3d21f4b 100644 --- a/complete.go +++ b/complete.go @@ -22,7 +22,7 @@ func flagsToCompleteFlags(flags []Flag) complete.Flags { } else { flagName = "--" + s } - complFlags[flagName] = f.GetPredictor() + complFlags[flagName] = f.GetCompleter() } } return complFlags @@ -31,8 +31,8 @@ func flagsToCompleteFlags(flags []Flag) complete.Flags { // cmdToCompleteCmd recursively transforms a Command (and its Subcommands) into // a complete.Command understood by the posener/complete library. Hidden // commands are skipped; aliases are registered alongside the primary name. The -// argument and flag-value predictors come from the command's own -// CustomCompletePredictor / CustomFlagPredictor fields. +// argument and flag-value predictors come from the Completer fields on the +// command and on its flags. func cmdToCompleteCmd(cmd Command, parentSubcommandMap complete.Commands) { if cmd.Hidden { return @@ -45,7 +45,7 @@ func cmdToCompleteCmd(cmd Command, parentSubcommandMap complete.Commands) { compCmd := complete.Command{ Sub: sub, - Args: cmd.CustomCompletePredictor, + Args: cmd.Completer, Flags: flagsToCompleteFlags(cmd.Flags), } parentSubcommandMap[cmd.Name] = compCmd diff --git a/complete_test.go b/complete_test.go index 3465cdc976..3286dca780 100644 --- a/complete_test.go +++ b/complete_test.go @@ -26,11 +26,11 @@ func newCompletionTestApp() *App { app.HideVersion = true app.EnableBashCompletion = true app.Flags = []Flag{ - StringFlag{Name: "verbosity", CustomFlagPredictor: complete.PredictSet("debug", "info")}, + StringFlag{Name: "verbosity", Completer: complete.PredictSet("debug", "info")}, StringFlag{Name: "secret-app-flag", Hidden: true}, } app.GlobalFlags = []Flag{ - StringFlag{Name: "profile", CustomFlagPredictor: complete.PredictSet("dev", "prod")}, + StringFlag{Name: "profile", Completer: complete.PredictSet("dev", "prod")}, StringFlag{Name: "secret-global-flag", Hidden: true}, } app.Commands = []Command{ @@ -46,15 +46,15 @@ func newCompletionTestApp() *App { }, }, { - Name: "pick", - Aliases: []string{"pk"}, - HiddenAliases: true, - CustomCompletePredictor: complete.PredictSet("alpha", "beta"), + Name: "pick", + Aliases: []string{"pk"}, + HiddenAliases: true, + Completer: complete.PredictSet("alpha", "beta"), }, { Name: "paint", Flags: []Flag{ - StringFlag{Name: "color", CustomFlagPredictor: complete.PredictSet("green", "red")}, + StringFlag{Name: "color", Completer: complete.PredictSet("green", "red")}, StringFlag{Name: "secret-color-flag", Hidden: true}, }, }, diff --git a/flag.go b/flag.go index 8b8fbfc483..6f84bc0528 100644 --- a/flag.go +++ b/flag.go @@ -56,7 +56,7 @@ type Flag interface { // Apply Flag settings to the given flag set Apply(*flag.FlagSet) GetName() string - GetPredictor() complete.Predictor + GetCompleter() complete.Predictor } // errorableFlag is an interface that allows us to return errors during apply diff --git a/flag_generated.go b/flag_generated.go index 8890556259..7e22351be7 100644 --- a/flag_generated.go +++ b/flag_generated.go @@ -30,9 +30,9 @@ func (f BoolFlag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f BoolFlag) GetPredictor() complete.Predictor { +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f BoolFlag) GetCompleter() complete.Predictor { return complete.PredictNothing } @@ -83,9 +83,9 @@ func (f BoolTFlag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f BoolTFlag) GetPredictor() complete.Predictor { +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f BoolTFlag) GetCompleter() complete.Predictor { return complete.PredictNothing } @@ -118,13 +118,13 @@ func lookupBoolT(name string, set *flag.FlagSet) bool { // DurationFlag is a flag with type time.Duration (see https://golang.org/pkg/time/#ParseDuration) type DurationFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value time.Duration - Destination *time.Duration - CustomFlagPredictor complete.Predictor + Name string + Usage string + EnvVar string + Hidden bool + Value time.Duration + Destination *time.Duration + Completer complete.Predictor } // String returns a readable representation of this value @@ -138,10 +138,10 @@ func (f DurationFlag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f DurationFlag) GetPredictor() complete.Predictor { - return f.CustomFlagPredictor +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f DurationFlag) GetCompleter() complete.Predictor { + return f.Completer } // Duration looks up the value of a local DurationFlag, returns @@ -173,13 +173,13 @@ func lookupDuration(name string, set *flag.FlagSet) time.Duration { // Float64Flag is a flag with type float64 type Float64Flag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value float64 - Destination *float64 - CustomFlagPredictor complete.Predictor + Name string + Usage string + EnvVar string + Hidden bool + Value float64 + Destination *float64 + Completer complete.Predictor } // String returns a readable representation of this value @@ -193,10 +193,10 @@ func (f Float64Flag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f Float64Flag) GetPredictor() complete.Predictor { - return f.CustomFlagPredictor +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f Float64Flag) GetCompleter() complete.Predictor { + return f.Completer } // Float64 looks up the value of a local Float64Flag, returns @@ -228,12 +228,12 @@ func lookupFloat64(name string, set *flag.FlagSet) float64 { // GenericFlag is a flag with type Generic type GenericFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value Generic - CustomFlagPredictor complete.Predictor + Name string + Usage string + EnvVar string + Hidden bool + Value Generic + Completer complete.Predictor } // String returns a readable representation of this value @@ -247,10 +247,10 @@ func (f GenericFlag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f GenericFlag) GetPredictor() complete.Predictor { - return f.CustomFlagPredictor +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f GenericFlag) GetCompleter() complete.Predictor { + return f.Completer } // Generic looks up the value of a local GenericFlag, returns @@ -282,13 +282,13 @@ func lookupGeneric(name string, set *flag.FlagSet) interface{} { // Int64Flag is a flag with type int64 type Int64Flag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value int64 - Destination *int64 - CustomFlagPredictor complete.Predictor + Name string + Usage string + EnvVar string + Hidden bool + Value int64 + Destination *int64 + Completer complete.Predictor } // String returns a readable representation of this value @@ -302,10 +302,10 @@ func (f Int64Flag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f Int64Flag) GetPredictor() complete.Predictor { - return f.CustomFlagPredictor +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f Int64Flag) GetCompleter() complete.Predictor { + return f.Completer } // Int64 looks up the value of a local Int64Flag, returns @@ -337,13 +337,13 @@ func lookupInt64(name string, set *flag.FlagSet) int64 { // IntFlag is a flag with type int type IntFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value int - Destination *int - CustomFlagPredictor complete.Predictor + Name string + Usage string + EnvVar string + Hidden bool + Value int + Destination *int + Completer complete.Predictor } // String returns a readable representation of this value @@ -357,10 +357,10 @@ func (f IntFlag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f IntFlag) GetPredictor() complete.Predictor { - return f.CustomFlagPredictor +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f IntFlag) GetCompleter() complete.Predictor { + return f.Completer } // Int looks up the value of a local IntFlag, returns @@ -392,12 +392,12 @@ func lookupInt(name string, set *flag.FlagSet) int { // IntSliceFlag is a flag with type *IntSlice type IntSliceFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value *IntSlice - CustomFlagPredictor complete.Predictor + Name string + Usage string + EnvVar string + Hidden bool + Value *IntSlice + Completer complete.Predictor } // String returns a readable representation of this value @@ -411,10 +411,10 @@ func (f IntSliceFlag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f IntSliceFlag) GetPredictor() complete.Predictor { - return f.CustomFlagPredictor +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f IntSliceFlag) GetCompleter() complete.Predictor { + return f.Completer } // IntSlice looks up the value of a local IntSliceFlag, returns @@ -446,12 +446,12 @@ func lookupIntSlice(name string, set *flag.FlagSet) []int { // Int64SliceFlag is a flag with type *Int64Slice type Int64SliceFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value *Int64Slice - CustomFlagPredictor complete.Predictor + Name string + Usage string + EnvVar string + Hidden bool + Value *Int64Slice + Completer complete.Predictor } // String returns a readable representation of this value @@ -465,10 +465,10 @@ func (f Int64SliceFlag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f Int64SliceFlag) GetPredictor() complete.Predictor { - return f.CustomFlagPredictor +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f Int64SliceFlag) GetCompleter() complete.Predictor { + return f.Completer } // Int64Slice looks up the value of a local Int64SliceFlag, returns @@ -500,13 +500,13 @@ func lookupInt64Slice(name string, set *flag.FlagSet) []int64 { // StringFlag is a flag with type string type StringFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value string - Destination *string - CustomFlagPredictor complete.Predictor + Name string + Usage string + EnvVar string + Hidden bool + Value string + Destination *string + Completer complete.Predictor } // String returns a readable representation of this value @@ -520,10 +520,10 @@ func (f StringFlag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f StringFlag) GetPredictor() complete.Predictor { - return f.CustomFlagPredictor +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f StringFlag) GetCompleter() complete.Predictor { + return f.Completer } // String looks up the value of a local StringFlag, returns @@ -555,12 +555,12 @@ func lookupString(name string, set *flag.FlagSet) string { // StringSliceFlag is a flag with type *StringSlice type StringSliceFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value *StringSlice - CustomFlagPredictor complete.Predictor + Name string + Usage string + EnvVar string + Hidden bool + Value *StringSlice + Completer complete.Predictor } // String returns a readable representation of this value @@ -574,10 +574,10 @@ func (f StringSliceFlag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f StringSliceFlag) GetPredictor() complete.Predictor { - return f.CustomFlagPredictor +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f StringSliceFlag) GetCompleter() complete.Predictor { + return f.Completer } // StringSlice looks up the value of a local StringSliceFlag, returns @@ -609,13 +609,13 @@ func lookupStringSlice(name string, set *flag.FlagSet) []string { // Uint64Flag is a flag with type uint64 type Uint64Flag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value uint64 - Destination *uint64 - CustomFlagPredictor complete.Predictor + Name string + Usage string + EnvVar string + Hidden bool + Value uint64 + Destination *uint64 + Completer complete.Predictor } // String returns a readable representation of this value @@ -629,10 +629,10 @@ func (f Uint64Flag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f Uint64Flag) GetPredictor() complete.Predictor { - return f.CustomFlagPredictor +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f Uint64Flag) GetCompleter() complete.Predictor { + return f.Completer } // Uint64 looks up the value of a local Uint64Flag, returns @@ -664,13 +664,13 @@ func lookupUint64(name string, set *flag.FlagSet) uint64 { // UintFlag is a flag with type uint type UintFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value uint - Destination *uint - CustomFlagPredictor complete.Predictor + Name string + Usage string + EnvVar string + Hidden bool + Value uint + Destination *uint + Completer complete.Predictor } // String returns a readable representation of this value @@ -684,10 +684,10 @@ func (f UintFlag) GetName() string { return f.Name } -// GetPredictor returns the predictor to use for shell completion -// of this flag's value -func (f UintFlag) GetPredictor() complete.Predictor { - return f.CustomFlagPredictor +// GetCompleter returns the predictor for this flag's value +// during shell completion +func (f UintFlag) GetCompleter() complete.Predictor { + return f.Completer } // Uint looks up the value of a local UintFlag, returns diff --git a/generate-flag-types.py b/generate-flag-types.py index bb3d891705..8febba3f3b 100755 --- a/generate-flag-types.py +++ b/generate-flag-types.py @@ -161,7 +161,7 @@ def _write_cli_flag_types(outfile, types): if typedef['value']: _fwrite(outfile, """\ - CustomFlagPredictor complete.Predictor + Completer complete.Predictor """.format(**typedef)) _fwrite(outfile, "\n}\n\n") @@ -181,13 +181,13 @@ def _write_cli_flag_types(outfile, types): """.format(**typedef)) predictor_body = ( - "return f.CustomFlagPredictor" if typedef['value'] + "return f.Completer" if typedef['value'] else "return complete.PredictNothing" ) _fwrite(outfile, """\ - // GetPredictor returns the predictor to use for shell completion - // of this flag's value - func (f {name}Flag) GetPredictor() complete.Predictor {{ + // GetCompleter returns the predictor for this flag's value + // during shell completion + func (f {name}Flag) GetCompleter() complete.Predictor {{ {predictor_body} }} From b7a673b78e1994206921cda88c74a47e2a31565e Mon Sep 17 00:00:00 2001 From: Taran Pelkey Date: Tue, 1 Sep 2026 12:34:40 -0500 Subject: [PATCH 3/3] coderabbit fixes --- complete_test.go | 15 +++- flag_generated.go | 55 ++++++++++++--- generate-flag-types.py | 9 ++- install.go | 149 ++++++++++++++++++++++++++++++++++++---- install_test.go | 152 ++++++++++++++++++++++++++++++++++++++++- 5 files changed, 351 insertions(+), 29 deletions(-) diff --git a/complete_test.go b/complete_test.go index 3286dca780..4ed6525c7e 100644 --- a/complete_test.go +++ b/complete_test.go @@ -14,9 +14,10 @@ import ( // newCompletionTestApp builds an app exercising every completion path: // commands with aliases (including a hidden top-level alias and a hidden // nested alias), a hidden command, a subcommand tree (with a hidden child), -// a leaf command carrying an arg predictor, a command with both a visible -// and a hidden flag-value predictor, and app-level (root-only) and global -// (inherited) flags, each with a visible and a hidden one. Help/version are +// a leaf command carrying an arg predictor, a command with a visible and a +// hidden flag-value predictor, a value flag with no predictor at all, and +// app-level (root-only) and global (inherited) flags, each with a visible and +// a hidden one. Help/version are // hidden to keep predictions clean. func newCompletionTestApp() *App { app := NewApp() @@ -37,6 +38,9 @@ func newCompletionTestApp() *App { { Name: "widget", Aliases: []string{"w"}, + // size takes a value but has no Completer. Its value must be left + // to the user, not filled in with widget's subcommand names. + Flags: []Flag{StringFlag{Name: "size"}}, Subcommands: Commands{ // mk is a hidden alias of make: exercises the nested // HiddenAliases path alongside the top-level one on pick. @@ -119,6 +123,11 @@ func TestShellCompletion(t *testing.T) { line: "prog paint --color ", want: []string{"green", "red"}, }, + { + name: "value flag without a Completer predicts nothing, not subcommand names", + line: "prog widget --size ", + want: nil, + }, { name: "global flag value predicted inside a child command", line: "prog widget --profile ", diff --git a/flag_generated.go b/flag_generated.go index 7e22351be7..a200d3d938 100644 --- a/flag_generated.go +++ b/flag_generated.go @@ -141,7 +141,10 @@ func (f DurationFlag) GetName() string { // GetCompleter returns the predictor for this flag's value // during shell completion func (f DurationFlag) GetCompleter() complete.Predictor { - return f.Completer + if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything } // Duration looks up the value of a local DurationFlag, returns @@ -196,7 +199,10 @@ func (f Float64Flag) GetName() string { // GetCompleter returns the predictor for this flag's value // during shell completion func (f Float64Flag) GetCompleter() complete.Predictor { - return f.Completer + if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything } // Float64 looks up the value of a local Float64Flag, returns @@ -250,7 +256,10 @@ func (f GenericFlag) GetName() string { // GetCompleter returns the predictor for this flag's value // during shell completion func (f GenericFlag) GetCompleter() complete.Predictor { - return f.Completer + if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything } // Generic looks up the value of a local GenericFlag, returns @@ -305,7 +314,10 @@ func (f Int64Flag) GetName() string { // GetCompleter returns the predictor for this flag's value // during shell completion func (f Int64Flag) GetCompleter() complete.Predictor { - return f.Completer + if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything } // Int64 looks up the value of a local Int64Flag, returns @@ -360,7 +372,10 @@ func (f IntFlag) GetName() string { // GetCompleter returns the predictor for this flag's value // during shell completion func (f IntFlag) GetCompleter() complete.Predictor { - return f.Completer + if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything } // Int looks up the value of a local IntFlag, returns @@ -414,7 +429,10 @@ func (f IntSliceFlag) GetName() string { // GetCompleter returns the predictor for this flag's value // during shell completion func (f IntSliceFlag) GetCompleter() complete.Predictor { - return f.Completer + if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything } // IntSlice looks up the value of a local IntSliceFlag, returns @@ -468,7 +486,10 @@ func (f Int64SliceFlag) GetName() string { // GetCompleter returns the predictor for this flag's value // during shell completion func (f Int64SliceFlag) GetCompleter() complete.Predictor { - return f.Completer + if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything } // Int64Slice looks up the value of a local Int64SliceFlag, returns @@ -523,7 +544,10 @@ func (f StringFlag) GetName() string { // GetCompleter returns the predictor for this flag's value // during shell completion func (f StringFlag) GetCompleter() complete.Predictor { - return f.Completer + if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything } // String looks up the value of a local StringFlag, returns @@ -577,7 +601,10 @@ func (f StringSliceFlag) GetName() string { // GetCompleter returns the predictor for this flag's value // during shell completion func (f StringSliceFlag) GetCompleter() complete.Predictor { - return f.Completer + if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything } // StringSlice looks up the value of a local StringSliceFlag, returns @@ -632,7 +659,10 @@ func (f Uint64Flag) GetName() string { // GetCompleter returns the predictor for this flag's value // during shell completion func (f Uint64Flag) GetCompleter() complete.Predictor { - return f.Completer + if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything } // Uint64 looks up the value of a local Uint64Flag, returns @@ -687,7 +717,10 @@ func (f UintFlag) GetName() string { // GetCompleter returns the predictor for this flag's value // during shell completion func (f UintFlag) GetCompleter() complete.Predictor { - return f.Completer + if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything } // Uint looks up the value of a local UintFlag, returns diff --git a/generate-flag-types.py b/generate-flag-types.py index 8febba3f3b..0cfcf51df3 100755 --- a/generate-flag-types.py +++ b/generate-flag-types.py @@ -180,8 +180,15 @@ def _write_cli_flag_types(outfile, types): """.format(**typedef)) + # A nil Predictor makes posener/complete fall through and predict + # subcommand and flag names after "--flag ". Value flags without + # an explicit Completer fall back to PredictAnything, which is non-nil + # and predicts no options, so the flag's value is left alone. predictor_body = ( - "return f.Completer" if typedef['value'] + """if f.Completer != nil { + return f.Completer + } + return complete.PredictAnything""" if typedef['value'] else "return complete.PredictNothing" ) _fwrite(outfile, """\ diff --git a/install.go b/install.go index f132a198cf..06e97a48f7 100644 --- a/install.go +++ b/install.go @@ -1,9 +1,12 @@ package cli import ( + "bufio" "errors" + "fmt" "os" "os/exec" + "os/user" "path/filepath" "runtime" "strconv" @@ -51,11 +54,12 @@ func DetectShell() (name string, fromEnv bool, err error) { return strings.ToLower(filepath.Base(shellName)), false, nil } -// installer and isInstalled are seams over posener/complete's real Install -// and IsInstalled, for tests. +// installer, isInstalled and shellIsInstalled are seams over the real +// installation checks, for tests. var ( - installer = completeinstall.Install - isInstalled = completeinstall.IsInstalled + installer = completeinstall.Install + isInstalled = completeinstall.IsInstalled + shellIsInstalled = shellCompletionInstalled ) // InstallShellCompletion registers cmd for shell completion in the user's shell @@ -77,10 +81,8 @@ type ShellCompletionResult struct { Shell string // DetectedFromEnv reports whether Shell came from $SHELL (vs. a fallback). DetectedFromEnv bool - // AlreadyInstalled reports whether completion was already registered, in - // which case no changes were made. Best-effort: if InstallShellCompletion - // partially fails on an unrelated shell config, this may report true even - // though Shell's own config was just freshly written. + // AlreadyInstalled reports whether completion was already registered for + // Shell specifically, in which case no changes were made to its config. AlreadyInstalled bool } @@ -109,13 +111,136 @@ func SetupShellCompletion(cmd string) (ShellCompletionResult, error) { } if installErr := InstallShellCompletion(cmd); installErr != nil { - // Install() may have partially failed (e.g. some shell config - // already had it) while still succeeding for shell; re-check actual - // disk state rather than trust the error alone. - if !isInstalled(cmd) { + // Install() writes every shell config it finds and reports a + // combined error, so a failure here may belong to a shell the user + // does not run. Re-check the detected shell's own config on disk: + // only if that one is registered is the error harmless. + if !shellIsInstalled(shell, cmd) { return res, installErr } res.AlreadyInstalled = true } return res, nil } + +// The helpers below mirror the per-shell logic in posener/complete's +// cmd/install package, which exports only a package-wide IsInstalled that +// answers "is this registered for *any* shell". SetupShellCompletion needs the +// per-shell answer. The rc-file candidates and the completion lines must stay +// byte-identical to that package's, or the check reports "not installed" for a +// config it wrote itself. + +// homeDir and completionBinaryPath are seams over the two pieces of ambient +// state the checks below depend on, for tests. +var ( + homeDir = userHomeDir + completionBinaryPath = executablePath +) + +// userHomeDir resolves the home directory through os/user, as posener does. +func userHomeDir() (string, error) { + u, err := user.Current() + if err != nil { + return "", err + } + return u.HomeDir, nil +} + +// executablePath returns the absolute path to the running executable. +func executablePath() (string, error) { + bin, err := os.Executable() + if err != nil { + return "", err + } + return filepath.Abs(bin) +} + +// shellCompletionInstalled reports whether cmd's completion is registered in +// the given shell's own configuration. +func shellCompletionInstalled(shell, cmd string) bool { + bin, err := completionBinaryPath() + if err != nil { + return false + } + + switch shell { + case "bash": + rc := bashRCFile() + return rc != "" && lineInFile(rc, fmt.Sprintf("complete -C %s %s", bin, cmd)) + case "zsh": + rc := rcFile(".zshrc") + return rc != "" && lineInFile(rc, fmt.Sprintf("complete -o nospace -C %s %s", bin, cmd)) + case "fish": + dir := fishConfigDir() + if dir == "" { + return false + } + _, err := os.Stat(filepath.Join(dir, "completions", fmt.Sprintf("%s.fish", cmd))) + return err == nil + } + return false +} + +// bashRCFile returns the first existing bash config file posener would have +// installed into, or "" if none exist. +func bashRCFile() string { + candidates := []string{".bashrc", ".bash_profile", ".bash_login", ".profile"} + if runtime.GOOS == "darwin" { + candidates = []string{".bash_profile"} + } + for _, name := range candidates { + if f := rcFile(name); f != "" { + return f + } + } + return "" +} + +// rcFile returns the path to name under the user's home directory if it +// exists, or "" otherwise. +func rcFile(name string) string { + home, err := homeDir() + if err != nil { + return "" + } + path := filepath.Join(home, name) + if _, err := os.Stat(path); err != nil { + return "" + } + return path +} + +// fishConfigDir returns the user's fish configuration directory if it exists, +// or "" otherwise. +func fishConfigDir() string { + home, err := homeDir() + if err != nil { + return "" + } + configHome := os.Getenv("XDG_CONFIG_HOME") + if configHome == "" { + configHome = filepath.Join(home, ".config") + } + dir := filepath.Join(configHome, "fish") + if info, err := os.Stat(dir); err != nil || !info.IsDir() { + return "" + } + return dir +} + +// lineInFile reports whether name contains a line exactly equal to lookFor. +func lineInFile(name, lookFor string) bool { + f, err := os.Open(name) + if err != nil { + return false + } + defer f.Close() + + scanner := bufio.NewScanner(f) + for scanner.Scan() { + if scanner.Text() == lookFor { + return true + } + } + return false +} diff --git a/install_test.go b/install_test.go index 4ace849e2c..2fb4f5c70c 100644 --- a/install_test.go +++ b/install_test.go @@ -2,6 +2,10 @@ package cli import ( "errors" + "os" + "path/filepath" + "runtime" + "strings" "testing" ) @@ -23,6 +27,35 @@ func withIsInstalled(t *testing.T, fn func(cmd string) bool) { t.Cleanup(func() { isInstalled = old }) } +func withShellIsInstalled(t *testing.T, fn func(shell, cmd string) bool) { + t.Helper() + old := shellIsInstalled + shellIsInstalled = fn + t.Cleanup(func() { shellIsInstalled = old }) +} + +// withFakeHome points the rc-file lookups at a temp directory and pins the +// binary path, so the shell config checks can be exercised without touching +// the real home directory. +func withFakeHome(t *testing.T, bin string) string { + t.Helper() + dir := t.TempDir() + + oldHome := homeDir + homeDir = func() (string, error) { return dir, nil } + t.Cleanup(func() { homeDir = oldHome }) + + oldBin := completionBinaryPath + completionBinaryPath = func() (string, error) { return bin, nil } + t.Cleanup(func() { completionBinaryPath = oldBin }) + + // fishConfigDir consults XDG_CONFIG_HOME before falling back to + // $home/.config; clear it so the fake home wins. + t.Setenv("XDG_CONFIG_HOME", "") + + return dir +} + func TestSetupShellCompletionFreshInstall(t *testing.T) { t.Setenv("SHELL", "/bin/bash") withInstaller(t, func(cmd string) error { return nil }) @@ -47,7 +80,7 @@ func TestSetupShellCompletionAlreadyInstalled(t *testing.T) { withInstaller(t, func(cmd string) error { return errors.New("1 error occurred: * already installed in /home/user/.bashrc") }) - withIsInstalled(t, func(cmd string) bool { return true }) + withShellIsInstalled(t, func(shell, cmd string) bool { return true }) res, err := SetupShellCompletion("prog") if err != nil { @@ -64,7 +97,7 @@ func TestSetupShellCompletionPropagatesGenuineFailure(t *testing.T) { t.Setenv("SHELL", "/bin/bash") wantErr := errors.New("open /home/user/.bashrc: permission denied") withInstaller(t, func(cmd string) error { return wantErr }) - withIsInstalled(t, func(cmd string) bool { return false }) + withShellIsInstalled(t, func(shell, cmd string) bool { return false }) res, err := SetupShellCompletion("prog") if !errors.Is(err, wantErr) { @@ -88,3 +121,118 @@ func TestSetupShellCompletionUnsupportedShell(t *testing.T) { t.Errorf("err = %v, want %v", err, ErrCompletionUnsupportedShell) } } + +// A failure to write the detected shell's config must propagate even when some +// *other* shell is already registered — the package-wide IsInstalled would +// report true there and hide the failure. +func TestSetupShellCompletionFailureForDetectedShell(t *testing.T) { + t.Setenv("SHELL", "/bin/bash") + wantErr := errors.New("open /home/user/.bashrc: permission denied") + withInstaller(t, func(cmd string) error { return wantErr }) + // Some other shell (zsh, say) is registered... + withIsInstalled(t, func(cmd string) bool { return true }) + // ...but bash, the shell actually in use, is not. + withShellIsInstalled(t, func(shell, cmd string) bool { return shell != "bash" }) + + res, err := SetupShellCompletion("prog") + if !errors.Is(err, wantErr) { + t.Errorf("err = %v, want %v", err, wantErr) + } + if res.AlreadyInstalled { + t.Error("res.AlreadyInstalled = true, want false when the detected shell's install failed") + } +} + +func TestShellCompletionInstalledBash(t *testing.T) { + home := withFakeHome(t, "/usr/local/bin/prog") + rc := ".bashrc" + if runtime.GOOS == "darwin" { + rc = ".bash_profile" + } + writeLines(t, filepath.Join(home, rc), + "# some other config", + "complete -C /usr/local/bin/prog prog", + ) + + if !shellCompletionInstalled("bash", "prog") { + t.Error("shellCompletionInstalled(bash, prog) = false, want true") + } + if shellCompletionInstalled("bash", "other") { + t.Error("shellCompletionInstalled(bash, other) = true, want false") + } + // zsh's rc file is absent, so it must not inherit bash's answer. + if shellCompletionInstalled("zsh", "prog") { + t.Error("shellCompletionInstalled(zsh, prog) = true, want false") + } +} + +func TestShellCompletionInstalledZsh(t *testing.T) { + home := withFakeHome(t, "/usr/local/bin/prog") + writeLines(t, filepath.Join(home, ".zshrc"), + "autoload -U +X bashcompinit && bashcompinit", + "complete -o nospace -C /usr/local/bin/prog prog", + ) + + if !shellCompletionInstalled("zsh", "prog") { + t.Error("shellCompletionInstalled(zsh, prog) = false, want true") + } +} + +// zsh's line carries -o nospace; a bash-shaped line must not satisfy it. +func TestShellCompletionInstalledZshRejectsBashLine(t *testing.T) { + home := withFakeHome(t, "/usr/local/bin/prog") + writeLines(t, filepath.Join(home, ".zshrc"), "complete -C /usr/local/bin/prog prog") + + if shellCompletionInstalled("zsh", "prog") { + t.Error("shellCompletionInstalled(zsh, prog) = true, want false for a bash-shaped line") + } +} + +func TestShellCompletionInstalledFish(t *testing.T) { + home := withFakeHome(t, "/usr/local/bin/prog") + dir := filepath.Join(home, ".config", "fish", "completions") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + writeLines(t, filepath.Join(dir, "prog.fish"), "function __complete_prog") + + if !shellCompletionInstalled("fish", "prog") { + t.Error("shellCompletionInstalled(fish, prog) = false, want true") + } + if shellCompletionInstalled("fish", "other") { + t.Error("shellCompletionInstalled(fish, other) = true, want false") + } +} + +// A different binary path means posener wrote the line for a different +// install, so it does not count as installed for this one. +func TestShellCompletionInstalledDifferentBinary(t *testing.T) { + home := withFakeHome(t, "/opt/prog") + rc := ".bashrc" + if runtime.GOOS == "darwin" { + rc = ".bash_profile" + } + writeLines(t, filepath.Join(home, rc), "complete -C /usr/local/bin/prog prog") + + if shellCompletionInstalled("bash", "prog") { + t.Error("shellCompletionInstalled(bash, prog) = true, want false for a different binary path") + } +} + +func TestShellCompletionInstalledMissingConfig(t *testing.T) { + withFakeHome(t, "/usr/local/bin/prog") + + for _, shell := range []string{"bash", "zsh", "fish", "tcsh"} { + if shellCompletionInstalled(shell, "prog") { + t.Errorf("shellCompletionInstalled(%s, prog) = true, want false with no config present", shell) + } + } +} + +func writeLines(t *testing.T, path string, lines ...string) { + t.Helper() + content := strings.Join(lines, "\n") + "\n" + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } +}