From fad72f0d8fc7880c3c6c099e844a981e56646c4b Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Fri, 11 Sep 2026 11:08:36 -0700 Subject: [PATCH 1/3] Suggest a command when one is mistyped A mistyped command used to produce "error parsing flags: unexpected arguments: [update]". Nothing about flags was wrong, the Go slice syntax leaked into user-facing output, and the dispatcher knew the real command names all along without offering any of them. Unknown commands now report the offending word and, when something is close enough, what the user probably meant: unknown command "update" for "miren app" Did you mean? upgrade Run 'miren app --help' to see available commands. Three paths fed into the old behavior, and all three are covered: - A word matching nothing at all. This previously joined every argument into the message, so "miren depoy myapp" blamed "depoy myapp". It now walks the typed words to find the deepest valid namespace and blames only the first word past it. - A leftover word under a command that has sub-commands. - A leftover word under a command that tolerates unknown flags. These skip the extra-argument check entirely, so a typo used to print help and exit 0 with no hint that anything was wrong. The last two share one rule: a leftover word under a command that has children and declares neither positionals nor a rest field can only be a misspelled sub-command. The check runs after Parse, so that a word which is really an unknown flag's value is claimed as such rather than blamed. Suggestions come from a Levenshtein distance over runes with a threshold that scales with word length, plus a prefix rule so "depl" reaches "deploy". Written by hand to keep this module free of dependencies. Errors are now typed. UnknownCommandError carries the program, parent path, bad word, and candidates so callers can render it themselves; UnexpectedArgsError replaces the "%v" slice formatting and no longer gets wrapped in "error parsing flags", which is reserved for real flag errors. --- dispatcher.go | 95 ++++++++++++++++++- dispatcher_test.go | 6 +- errors.go | 66 ++++++++++++++ mflags.go | 3 +- mflags_test.go | 6 +- positional_api_test.go | 8 +- suggest.go | 127 ++++++++++++++++++++++++++ suggest_test.go | 85 +++++++++++++++++ unknown_command_test.go | 196 ++++++++++++++++++++++++++++++++++++++++ 9 files changed, 579 insertions(+), 13 deletions(-) create mode 100644 errors.go create mode 100644 suggest.go create mode 100644 suggest_test.go create mode 100644 unknown_command_test.go diff --git a/dispatcher.go b/dispatcher.go index 7420e91..4812d76 100644 --- a/dispatcher.go +++ b/dispatcher.go @@ -296,7 +296,7 @@ func (d *Dispatcher) Execute(args []string) error { if hasHelp { return d.showHelp() } - return fmt.Errorf("unknown command: %s", strings.Join(args, " ")) + return d.unknownCommandError(nonFlagArgs) } // Check if remaining non-flag args form a namespace under the matched command. @@ -344,9 +344,26 @@ func (d *Dispatcher) Execute(args []string) error { fs.disableAutoHelp = true } if err := fs.Parse(allArgs); err != nil { + // Leftover positional arguments are not a flag-parsing problem, and + // saying so only misdirects the reader. + var unexpected *UnexpectedArgsError + if errors.As(err, &unexpected) { + if name, ok := d.mistypedSubCommand(entry, unexpected.Args); ok { + return d.unknownSubCommandError(entry.Path, name) + } + return err + } return fmt.Errorf("error parsing flags: %w", err) } + // A command that tolerates unknown flags skips Parse's extra-argument check + // entirely, so a mistyped sub-command under one would otherwise run as if + // nothing were wrong. Catch it here, once Parse has had its chance to claim + // those words as flag values. + if name, ok := d.mistypedSubCommand(entry, fs.Args()); ok { + return d.unknownSubCommandError(entry.Path, name) + } + // Execute the command with the parsed flagset and remaining args err := entry.Command.Run(fs, fs.Args()) @@ -363,6 +380,82 @@ func (d *Dispatcher) Run(args []string) error { return d.Execute(args) } +// unknownCommandError builds the error for command words that matched nothing. +// It first walks the words the user typed to find the deepest namespace among +// them, so that "miren debug wibble" blames "wibble" under "debug" rather than +// blaming "debug", which is perfectly valid. +func (d *Dispatcher) unknownCommandError(words []string) error { + parentPath := "" + unknownIdx := 0 + + for i := 1; i < len(words); i++ { + candidate := normalizeCommandPath(strings.Join(words[:i], " ")) + if d.isNamespace(candidate) { + parentPath = candidate + unknownIdx = i + } + } + + name := "" + if unknownIdx < len(words) { + name = words[unknownIdx] + } + + return d.unknownSubCommandError(parentPath, name) +} + +// mistypedSubCommand reports whether leftover arguments under entry can only be +// a misspelled sub-command: the command has children of its own and declares +// nowhere to put arguments. It returns the offending word. +func (d *Dispatcher) mistypedSubCommand(entry *CommandEntry, leftover []string) (string, bool) { + if len(leftover) == 0 { + return "", false + } + + fs := entry.Command.FlagSet() + if fs == nil || fs.PositionalCount() > 0 || fs.restField != nil { + return "", false + } + + children := d.GetDirectChildren(entry.Path) + if len(children) == 0 { + return "", false + } + + known := make(map[string]bool, len(children)) + for _, c := range children { + known[c.Name] = true + } + + for _, word := range leftover { + // "help" is a keyword the dispatcher handles for itself, and a word + // that really names a sub-command was already matched as one. + if word == "help" || known[word] { + continue + } + return word, true + } + + return "", false +} + +// unknownSubCommandError builds the error for a word that should have named a +// sub-command of parentPath but named nothing at all. +func (d *Dispatcher) unknownSubCommandError(parentPath, name string) error { + children := d.GetDirectChildren(parentPath) + candidates := make([]string, 0, len(children)) + for _, c := range children { + candidates = append(candidates, c.Name) + } + + return &UnknownCommandError{ + Program: d.name, + ParentPath: parentPath, + Name: name, + Suggestions: suggestNames(name, candidates), + } +} + // findCommand finds the best matching command for the given arguments func (d *Dispatcher) findCommand(args []string) (*CommandEntry, []string) { // Try progressively shorter command paths diff --git a/dispatcher_test.go b/dispatcher_test.go index ba3e5de..4625d59 100644 --- a/dispatcher_test.go +++ b/dispatcher_test.go @@ -1523,7 +1523,7 @@ func TestDispatcherRejectsExtraArgs(t *testing.T) { err := d.Execute([]string{"app", "services"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [services]") + assert.Contains(t, err.Error(), `unexpected argument "services"`) }) t.Run("nested command with no positional args rejects extra", func(t *testing.T) { @@ -1538,7 +1538,7 @@ func TestDispatcherRejectsExtraArgs(t *testing.T) { err := d.Execute([]string{"app", "services", "extra"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra]") + assert.Contains(t, err.Error(), `unexpected argument "extra"`) }) t.Run("command with positional args accepts correct count", func(t *testing.T) { @@ -1572,7 +1572,7 @@ func TestDispatcherRejectsExtraArgs(t *testing.T) { err := d.Execute([]string{"deploy", "production", "extra"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra]") + assert.Contains(t, err.Error(), `unexpected argument "extra"`) }) t.Run("command with rest field accepts all args", func(t *testing.T) { diff --git a/errors.go b/errors.go new file mode 100644 index 0000000..1243f6d --- /dev/null +++ b/errors.go @@ -0,0 +1,66 @@ +package mflags + +import ( + "fmt" + "strings" +) + +// UnknownCommandError reports a command word that matches nothing registered. +// It carries the surrounding context — which program, which parent command, and +// what the user might have meant — so callers can render it themselves rather +// than parsing the message back out of a string. +type UnknownCommandError struct { + // Program is the binary name, e.g. "miren". + Program string + // ParentPath is the command the unknown word appeared under, e.g. "app". + // Empty when the word was typed at the top level. + ParentPath string + // Name is the single word that matched nothing. + Name string + // Suggestions holds close matches, nearest first. May be empty. + Suggestions []string +} + +func (e *UnknownCommandError) Error() string { + helpTarget := strings.TrimSpace(e.Program + " " + e.ParentPath) + + var sections []string + + if e.ParentPath != "" { + sections = append(sections, fmt.Sprintf("unknown command %q for %q", e.Name, helpTarget)) + } else { + sections = append(sections, fmt.Sprintf("unknown command %q", e.Name)) + } + + if len(e.Suggestions) > 0 { + var b strings.Builder + b.WriteString("Did you mean?") + for _, s := range e.Suggestions { + fmt.Fprintf(&b, "\n %s", s) + } + sections = append(sections, b.String()) + } + + if helpTarget != "" { + sections = append(sections, fmt.Sprintf("Run '%s --help' to see available commands.", helpTarget)) + } + + return strings.Join(sections, "\n\n") +} + +// UnexpectedArgsError reports positional arguments a command has nowhere to put. +type UnexpectedArgsError struct { + Args []string +} + +func (e *UnexpectedArgsError) Error() string { + if len(e.Args) == 1 { + return fmt.Sprintf("unexpected argument %q", e.Args[0]) + } + + quoted := make([]string, 0, len(e.Args)) + for _, a := range e.Args { + quoted = append(quoted, fmt.Sprintf("%q", a)) + } + return "unexpected arguments: " + strings.Join(quoted, " ") +} diff --git a/mflags.go b/mflags.go index 09ec1cc..73ce93d 100644 --- a/mflags.go +++ b/mflags.go @@ -655,8 +655,7 @@ func (f *FlagSet) Parse(arguments []string) error { // Check for unexpected extra arguments when no rest field is defined // Skip validation if allowUnknownFlags is enabled (pass-through mode) if f.restField == nil && !f.allowUnknownFlags && len(f.args) > f.PositionalCount() { - extra := f.args[f.PositionalCount():] - return fmt.Errorf("unexpected arguments: %v", extra) + return &UnexpectedArgsError{Args: f.args[f.PositionalCount():]} } // If we have a rest field, populate it with remaining args after positional ones diff --git a/mflags_test.go b/mflags_test.go index 4ebc805..7d8c39e 100644 --- a/mflags_test.go +++ b/mflags_test.go @@ -1092,7 +1092,7 @@ func TestInvalidRestFieldType(t *testing.T) { // Since the rest field is invalid and ignored, extra args should be rejected err = fs.Parse([]string{"arg1", "arg2"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments") + assert.Contains(t, err.Error(), `unexpected arguments: "arg1" "arg2"`) // The rest field should be ignored since it's not []string assert.Equal(t, "", config.RestField) @@ -1287,7 +1287,7 @@ func TestInvalidPositionTag(t *testing.T) { // so any args should be rejected err = fs.Parse([]string{"value"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments") + assert.Contains(t, err.Error(), `unexpected argument "value"`) assert.Equal(t, "", config.Item) // Field is ignored due to invalid position } @@ -1307,7 +1307,7 @@ func TestNegativePositionTag(t *testing.T) { // so any args should be rejected err = fs.Parse([]string{"value"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments") + assert.Contains(t, err.Error(), `unexpected argument "value"`) assert.Equal(t, "", config.Item) // Field is ignored due to negative position } diff --git a/positional_api_test.go b/positional_api_test.go index 6551862..99bc5c0 100644 --- a/positional_api_test.go +++ b/positional_api_test.go @@ -427,7 +427,7 @@ func TestExtraArgumentsRejected(t *testing.T) { err := fs.Parse([]string{"extra"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra]") + assert.Contains(t, err.Error(), `unexpected argument "extra"`) }) t.Run("multiple extra args rejected", func(t *testing.T) { @@ -436,7 +436,7 @@ func TestExtraArgumentsRejected(t *testing.T) { err := fs.Parse([]string{"extra1", "extra2", "extra3"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra1 extra2 extra3]") + assert.Contains(t, err.Error(), `unexpected arguments: "extra1" "extra2" "extra3"`) }) t.Run("positional args rejects extra args", func(t *testing.T) { @@ -446,7 +446,7 @@ func TestExtraArgumentsRejected(t *testing.T) { err := fs.Parse([]string{"a", "b", "extra"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra]") + assert.Contains(t, err.Error(), `unexpected argument "extra"`) }) t.Run("positional with gaps rejects extra args", func(t *testing.T) { @@ -459,7 +459,7 @@ func TestExtraArgumentsRejected(t *testing.T) { // 5 args fill all positions, 6th is extra err := fs.Parse([]string{"a", "b", "c", "d", "e", "extra"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra]") + assert.Contains(t, err.Error(), `unexpected argument "extra"`) }) t.Run("rest field accepts all args", func(t *testing.T) { diff --git a/suggest.go b/suggest.go new file mode 100644 index 0000000..41af12a --- /dev/null +++ b/suggest.go @@ -0,0 +1,127 @@ +package mflags + +import ( + "sort" + "strings" +) + +// maxSuggestions caps how many alternatives a "did you mean" block offers. +// Beyond a handful the list stops being a suggestion and starts being help +// output, which the closing "--help" line already points at. +const maxSuggestions = 3 + +// levenshtein returns the edit distance between a and b, counting insertions, +// deletions, and substitutions. It compares runes rather than bytes so that +// non-ASCII command names cost what they look like they cost. +func levenshtein(a, b string) int { + ar := []rune(a) + br := []rune(b) + + if len(ar) == 0 { + return len(br) + } + if len(br) == 0 { + return len(ar) + } + + // Two rolling rows of the distance matrix: prev holds the row for the + // previous character of a, curr the row being filled in. + prev := make([]int, len(br)+1) + curr := make([]int, len(br)+1) + + for j := range prev { + prev[j] = j + } + + for i := 1; i <= len(ar); i++ { + curr[0] = i + for j := 1; j <= len(br); j++ { + cost := 1 + if ar[i-1] == br[j-1] { + cost = 0 + } + curr[j] = min(prev[j]+1, min(curr[j-1]+1, prev[j-1]+cost)) + } + prev, curr = curr, prev + } + + return prev[len(br)] +} + +// suggestThreshold returns the largest edit distance still worth reporting for +// a word of the given length. Short words get a tight budget because at two +// edits every three-letter word is a neighbour of every other one. +func suggestThreshold(n int) int { + switch { + case n <= 2: + return 0 + case n <= 4: + return 1 + case n <= 8: + return 2 + default: + return 3 + } +} + +// suggestNames returns the candidates closest to unknown, nearest first, or nil +// when nothing is close enough to be worth guessing at. Prefix matches rank +// ahead of pure edit distance: someone who typed "depl" almost certainly meant +// "deploy" even though four insertions is a long way in edit-distance terms. +func suggestNames(unknown string, candidates []string) []string { + if unknown == "" { + return nil + } + + threshold := suggestThreshold(len([]rune(unknown))) + + type scored struct { + name string + prefix bool + dist int + } + + var matches []scored + for _, c := range candidates { + if c == "" { + continue + } + + // A prefix match is strong evidence on its own — an abbreviation or a + // half-typed word — so it bypasses the distance threshold. + if len(c) > len(unknown) && strings.HasPrefix(c, unknown) { + matches = append(matches, scored{name: c, prefix: true, dist: len(c) - len(unknown)}) + continue + } + + if d := levenshtein(unknown, c); d <= threshold { + matches = append(matches, scored{name: c, dist: d}) + } + } + + if len(matches) == 0 { + return nil + } + + // Alphabetical ties keep the output stable; the dispatcher's command map + // iterates in random order. + sort.Slice(matches, func(i, j int) bool { + if matches[i].prefix != matches[j].prefix { + return matches[i].prefix + } + if matches[i].dist != matches[j].dist { + return matches[i].dist < matches[j].dist + } + return matches[i].name < matches[j].name + }) + + if len(matches) > maxSuggestions { + matches = matches[:maxSuggestions] + } + + names := make([]string, 0, len(matches)) + for _, m := range matches { + names = append(names, m.name) + } + return names +} diff --git a/suggest_test.go b/suggest_test.go new file mode 100644 index 0000000..e62231a --- /dev/null +++ b/suggest_test.go @@ -0,0 +1,85 @@ +package mflags + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestLevenshtein(t *testing.T) { + cases := []struct { + a, b string + want int + }{ + {"", "", 0}, + {"", "abc", 3}, + {"abc", "", 3}, + {"deploy", "deploy", 0}, + {"depoy", "deploy", 1}, // deletion + {"deployy", "deploy", 1}, // insertion + {"deploj", "deploy", 1}, // substitution + {"depoly", "deploy", 2}, // transposition costs two here + {"kitten", "sitting", 3}, + {"café", "cafe", 1}, // one rune, not two bytes + } + + for _, c := range cases { + assert.Equal(t, c.want, levenshtein(c.a, c.b), "levenshtein(%q, %q)", c.a, c.b) + assert.Equal(t, c.want, levenshtein(c.b, c.a), "levenshtein(%q, %q)", c.b, c.a) + } +} + +func TestSuggestNames(t *testing.T) { + commands := []string{"app", "deploy", "logs", "rollback", "runner", "server", "upgrade", "version"} + + t.Run("one edit away", func(t *testing.T) { + assert.Equal(t, []string{"deploy"}, suggestNames("depoy", commands)) + }) + + t.Run("transposition", func(t *testing.T) { + assert.Equal(t, []string{"deploy"}, suggestNames("depoly", commands)) + }) + + t.Run("prefix beats distance", func(t *testing.T) { + assert.Equal(t, []string{"deploy"}, suggestNames("depl", commands)) + }) + + t.Run("prefix matches rank ahead of edit-distance matches", func(t *testing.T) { + got := suggestNames("serve", []string{"server", "serve-it", "sever"}) + assert.Equal(t, []string{"server", "serve-it", "sever"}, got) + }) + + t.Run("shorter prefix completion wins", func(t *testing.T) { + assert.Equal(t, []string{"apply", "applesauce"}, suggestNames("appl", []string{"apply", "applesauce"})) + }) + + t.Run("nothing close returns nil", func(t *testing.T) { + assert.Nil(t, suggestNames("zzzzzz", commands)) + }) + + t.Run("short words are not guessed at on distance alone", func(t *testing.T) { + // "vp" is one edit from "up"; at two characters that is too weak a + // signal to guess on. + assert.Nil(t, suggestNames("vp", []string{"up", "down"})) + }) + + t.Run("but a short prefix still counts", func(t *testing.T) { + // "ap" is only two characters, yet it can only be reaching for "app". + assert.Equal(t, []string{"app"}, suggestNames("ap", commands)) + }) + + t.Run("empty input", func(t *testing.T) { + assert.Nil(t, suggestNames("", commands)) + assert.Nil(t, suggestNames("deploy", nil)) + }) + + t.Run("capped at three", func(t *testing.T) { + got := suggestNames("aaa", []string{"aab", "aac", "aad", "aae", "aaf"}) + assert.Len(t, got, maxSuggestions) + }) + + t.Run("ties are alphabetical", func(t *testing.T) { + got := suggestNames("aaa", []string{"aaf", "aab", "aad"}) + assert.Equal(t, []string{"aab", "aad", "aaf"}, got) + }) +} diff --git a/unknown_command_test.go b/unknown_command_test.go new file mode 100644 index 0000000..629d6da --- /dev/null +++ b/unknown_command_test.go @@ -0,0 +1,196 @@ +package mflags + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// unknownCommandDispatcher builds a dispatcher shaped like a real CLI: a plain +// top-level command, a leaf command that also has children, a section-style +// command that tolerates unknown flags, and a command that takes free-form +// arguments. +func unknownCommandDispatcher() *Dispatcher { + d := NewDispatcher("myapp") + + noArgs := func() *FlagSet { return NewFlagSet("x") } + run := func(fs *FlagSet, args []string) error { return nil } + + d.Dispatch("deploy", NewCommand(noArgs(), run, WithUsage("Deploy an application"))) + d.Dispatch("upgrade", NewCommand(noArgs(), run, WithUsage("Upgrade"))) + + // "app" is a real command that also has sub-commands under it. + d.Dispatch("app", NewCommand(noArgs(), run, WithUsage("Manage applications"))) + d.Dispatch("app list", NewCommand(noArgs(), run, WithUsage("List applications"))) + d.Dispatch("app destroy", NewCommand(noArgs(), run, WithUsage("Destroy an application"))) + + // "runner" stands in for a Section: no flags of its own, but it tolerates + // unknown ones so that global flags may precede it. + section := NewFlagSet("runner") + section.AllowUnknownFlags(true) + d.Dispatch("runner", NewCommand(section, run, WithUsage("Manage runners"))) + d.Dispatch("runner upgrade", NewCommand(noArgs(), run, WithUsage("Upgrade a runner"))) + + // "logs" takes a positional argument, so a stray word is a value, not a typo. + logs := NewFlagSet("logs") + logs.StringPos("app", 0, "", "Application name") + d.Dispatch("logs", NewCommand(logs, run, WithUsage("View logs"))) + d.Dispatch("logs build", NewCommand(noArgs(), run, WithUsage("View build logs"))) + + // "exec" passes everything through. + type execOpts struct { + Args []string `rest:"true"` + } + exec := NewFlagSet("exec") + exec.FromStruct(&execOpts{}) + d.Dispatch("exec", NewCommand(exec, run, WithUsage("Run a command"))) + d.Dispatch("exec shell", NewCommand(noArgs(), run, WithUsage("Open a shell"))) + + return d +} + +func TestUnknownCommandSuggestions(t *testing.T) { + t.Run("top-level typo suggests a top-level command", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"depoy"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Equal(t, "depoy", uce.Name) + assert.Equal(t, "", uce.ParentPath) + assert.Equal(t, []string{"deploy"}, uce.Suggestions) + + assert.Equal(t, `unknown command "depoy" + +Did you mean? + deploy + +Run 'myapp --help' to see available commands.`, err.Error()) + }) + + t.Run("typo under a command with sub-commands", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"app", "destory"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Equal(t, "destory", uce.Name) + assert.Equal(t, "app", uce.ParentPath) + + assert.Equal(t, `unknown command "destory" for "myapp app" + +Did you mean? + destroy + +Run 'myapp app --help' to see available commands.`, err.Error()) + }) + + t.Run("typo under a section errors instead of silently showing help", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"runner", "upgrde"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Equal(t, "upgrde", uce.Name) + assert.Equal(t, "runner", uce.ParentPath) + assert.Equal(t, []string{"upgrade"}, uce.Suggestions) + }) + + t.Run("typo under an unregistered namespace blames the right word", func(t *testing.T) { + d := NewDispatcher("myapp") + run := func(fs *FlagSet, args []string) error { return nil } + // "debug" itself is never registered; it exists only as a prefix. + d.Dispatch("debug entity", NewCommand(NewFlagSet("x"), run, WithUsage("Entities"))) + d.Dispatch("debug etcd", NewCommand(NewFlagSet("x"), run, WithUsage("Etcd"))) + + err := d.Execute([]string{"debug", "entty"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Equal(t, "entty", uce.Name, "should blame the bad word, not the valid namespace") + assert.Equal(t, "debug", uce.ParentPath) + assert.Equal(t, []string{"entity"}, uce.Suggestions) + }) + + t.Run("no close match still names the word", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"zzzzzzzz"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Empty(t, uce.Suggestions) + + assert.Equal(t, `unknown command "zzzzzzzz" + +Run 'myapp --help' to see available commands.`, err.Error()) + assert.NotContains(t, err.Error(), "Did you mean") + }) + + t.Run("only the first word is blamed", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"depoy", "myapp"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Equal(t, "depoy", uce.Name) + }) +} + +func TestUnknownCommandDoesNotFireOnValidInput(t *testing.T) { + cases := []struct { + name string + args []string + }{ + {"command with a positional accepts it", []string{"logs", "myapp"}}, + {"rest field accepts arbitrary words", []string{"exec", "echo", "hello"}}, + {"real sub-command still runs", []string{"app", "list"}}, + {"real nested sub-command still runs", []string{"runner", "upgrade"}}, + {"bare command with children shows help", []string{"app"}}, + // The dispatcher handles "help" itself; it is never a mistyped + // sub-command, even under a command that tolerates unknown flags. + {"trailing help keyword", []string{"runner", "help"}}, + {"leading help keyword", []string{"help", "runner"}}, + {"help keyword before a real sub-command", []string{"runner", "help", "upgrade"}}, + // An unknown flag's value is claimed by Parse, not blamed as a word. + {"unknown value flag before a section", []string{"runner", "-C", "prod"}}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.NoError(t, unknownCommandDispatcher().Execute(c.args)) + }) + } +} + +func TestUnexpectedArgsWithoutSubCommands(t *testing.T) { + // "version" has no children, so there is no sub-command to suggest. The + // message should still avoid both the Go slice syntax and the misleading + // "error parsing flags" prefix. + d := NewDispatcher("myapp") + d.Dispatch("version", NewCommand(NewFlagSet("version"), + func(fs *FlagSet, args []string) error { return nil }, WithUsage("Print the version"))) + + err := d.Execute([]string{"version", "foo"}) + require.Error(t, err) + + var uae *UnexpectedArgsError + require.ErrorAs(t, err, &uae) + assert.Equal(t, `unexpected argument "foo"`, err.Error()) + assert.NotContains(t, err.Error(), "error parsing flags") +} + +func TestUnknownFlagStillReportsParseError(t *testing.T) { + // Genuine flag problems keep the flag-parsing prefix. + d := NewDispatcher("myapp") + fs := NewFlagSet("deploy") + fs.Bool("verbose", 'v', false, "Verbose output") + d.Dispatch("deploy", NewCommand(fs, + func(fs *FlagSet, args []string) error { return nil }, WithUsage("Deploy"))) + + err := d.Execute([]string{"deploy", "--nope"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "error parsing flags") + assert.ErrorIs(t, err, ErrUnknownFlag) +} From 0b1a73e4205a0f80c6866deac45815b7626f35fb Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Fri, 11 Sep 2026 16:00:14 -0700 Subject: [PATCH 2/3] Suggest a flag name when one is mistyped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the command suggestions to flags, which fail the same way and were just as silent about it: unknown flag: --naem Did you mean? --name Only long flags get a guess. A single letter is too little to reason from, and offering long flags that merely start with it would be noise. Two supporting changes: Edit distance now counts a transposition of adjacent characters as one edit rather than two. Swapping two letters is among the most common ways to mistype a word, and the old cost put "naem" two edits from "name" — outside the threshold for a word that short, so the obvious suggestion was never offered. This improves the command suggestions too. UnknownFlagError is no longer wrapped in "error parsing flags". The message already says a flag is the problem, and the prefix only pushed the suggestions further from the summary. Genuine value errors, such as a non-numeric argument to --port, keep it. The error still unwraps to ErrUnknownFlag, so existing errors.Is checks are unaffected. --- dispatcher.go | 9 +++++ errors.go | 28 +++++++++++++++ mflags.go | 20 +++++++++-- suggest.go | 34 +++++++++++++----- suggest_test.go | 10 +++--- unknown_command_test.go | 76 +++++++++++++++++++++++++++++++++++------ 6 files changed, 151 insertions(+), 26 deletions(-) diff --git a/dispatcher.go b/dispatcher.go index 4812d76..907a849 100644 --- a/dispatcher.go +++ b/dispatcher.go @@ -353,6 +353,15 @@ func (d *Dispatcher) Execute(args []string) error { } return err } + + // "unknown flag: --naem" already says which part of the command line + // went wrong; prefixing it adds nothing and pushes the suggestions + // further from the summary. + var unknownFlag *UnknownFlagError + if errors.As(err, &unknownFlag) { + return err + } + return fmt.Errorf("error parsing flags: %w", err) } diff --git a/errors.go b/errors.go index 1243f6d..6cc2acc 100644 --- a/errors.go +++ b/errors.go @@ -48,6 +48,34 @@ func (e *UnknownCommandError) Error() string { return strings.Join(sections, "\n\n") } +// UnknownFlagError reports a flag the command does not define. It unwraps to +// ErrUnknownFlag, so existing errors.Is checks keep working. +type UnknownFlagError struct { + // Flag is the flag as the user typed it, dashes included. + Flag string + // Suggestions holds close long-flag names, dashes included, nearest first. + // Short flags carry no suggestions: a single letter is too little to guess + // from. + Suggestions []string +} + +func (e *UnknownFlagError) Error() string { + sections := []string{"unknown flag: " + e.Flag} + + if len(e.Suggestions) > 0 { + var b strings.Builder + b.WriteString("Did you mean?") + for _, s := range e.Suggestions { + fmt.Fprintf(&b, "\n %s", s) + } + sections = append(sections, b.String()) + } + + return strings.Join(sections, "\n\n") +} + +func (e *UnknownFlagError) Unwrap() error { return ErrUnknownFlag } + // UnexpectedArgsError reports positional arguments a command has nowhere to put. type UnexpectedArgsError struct { Args []string diff --git a/mflags.go b/mflags.go index 73ce93d..36c1ca2 100644 --- a/mflags.go +++ b/mflags.go @@ -708,6 +708,22 @@ func (f *FlagSet) validateRequired() error { return nil } +// unknownLongFlagError reports an undefined long flag, offering the defined +// flags whose names are closest to what was typed. +func (f *FlagSet) unknownLongFlagError(name string) error { + candidates := make([]string, 0, len(f.flags)) + for defined := range f.flags { + candidates = append(candidates, defined) + } + + suggestions := suggestNames(name, candidates) + for i, s := range suggestions { + suggestions[i] = "--" + s + } + + return &UnknownFlagError{Flag: "--" + name, Suggestions: suggestions} +} + func (f *FlagSet) parseLongFlag(name string, args []string, index *int) (bool, error) { var value string hasValue := false @@ -727,7 +743,7 @@ func (f *FlagSet) parseLongFlag(name string, args []string, index *int) (bool, e *index = len(args) - 1 // Skip to end return true, nil } - return false, fmt.Errorf("%w: --%s", ErrUnknownFlag, name) + return false, f.unknownLongFlagError(name) } if flag.Value.IsBool() { @@ -767,7 +783,7 @@ func (f *FlagSet) parseShortFlags(shortFlags string, args []string, index *int) *index = len(args) - 1 // Skip to end return nil } - return fmt.Errorf("%w: -%c", ErrUnknownFlag, r) + return &UnknownFlagError{Flag: fmt.Sprintf("-%c", r)} } if flag.Value.IsBool() { diff --git a/suggest.go b/suggest.go index 41af12a..0612d83 100644 --- a/suggest.go +++ b/suggest.go @@ -10,10 +10,16 @@ import ( // output, which the closing "--help" line already points at. const maxSuggestions = 3 -// levenshtein returns the edit distance between a and b, counting insertions, -// deletions, and substitutions. It compares runes rather than bytes so that -// non-ASCII command names cost what they look like they cost. -func levenshtein(a, b string) int { +// editDistance returns the number of edits between a and b, counting an +// insertion, a deletion, a substitution, or a transposition of two adjacent +// characters as one each. Transpositions matter: swapping two letters is among +// the most common ways to mistype a word, and charging two edits for it would +// put "recieve" as far from "receive" as a word with two genuinely wrong +// letters. +// +// It compares runes rather than bytes so that non-ASCII names cost what they +// look like they cost. +func editDistance(a, b string) int { ar := []rune(a) br := []rune(b) @@ -24,8 +30,10 @@ func levenshtein(a, b string) int { return len(ar) } - // Two rolling rows of the distance matrix: prev holds the row for the - // previous character of a, curr the row being filled in. + // Three rolling rows of the distance matrix: curr is the row being filled + // in, prev the row above it, and prevPrev the one above that, which only + // the transposition case reaches back for. + prevPrev := make([]int, len(br)+1) prev := make([]int, len(br)+1) curr := make([]int, len(br)+1) @@ -40,9 +48,17 @@ func levenshtein(a, b string) int { if ar[i-1] == br[j-1] { cost = 0 } - curr[j] = min(prev[j]+1, min(curr[j-1]+1, prev[j-1]+cost)) + + best := min(prev[j]+1, min(curr[j-1]+1, prev[j-1]+cost)) + + // The last two characters of each are the same pair, swapped. + if i > 1 && j > 1 && ar[i-1] == br[j-2] && ar[i-2] == br[j-1] { + best = min(best, prevPrev[j-2]+1) + } + + curr[j] = best } - prev, curr = curr, prev + prevPrev, prev, curr = prev, curr, prevPrev } return prev[len(br)] @@ -94,7 +110,7 @@ func suggestNames(unknown string, candidates []string) []string { continue } - if d := levenshtein(unknown, c); d <= threshold { + if d := editDistance(unknown, c); d <= threshold { matches = append(matches, scored{name: c, dist: d}) } } diff --git a/suggest_test.go b/suggest_test.go index e62231a..cb0ab21 100644 --- a/suggest_test.go +++ b/suggest_test.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestLevenshtein(t *testing.T) { +func TestEditDistance(t *testing.T) { cases := []struct { a, b string want int @@ -18,14 +18,16 @@ func TestLevenshtein(t *testing.T) { {"depoy", "deploy", 1}, // deletion {"deployy", "deploy", 1}, // insertion {"deploj", "deploy", 1}, // substitution - {"depoly", "deploy", 2}, // transposition costs two here + {"depoly", "deploy", 1}, // transposition + {"naem", "name", 1}, // transposition + {"recieve", "receive", 1}, {"kitten", "sitting", 3}, {"café", "cafe", 1}, // one rune, not two bytes } for _, c := range cases { - assert.Equal(t, c.want, levenshtein(c.a, c.b), "levenshtein(%q, %q)", c.a, c.b) - assert.Equal(t, c.want, levenshtein(c.b, c.a), "levenshtein(%q, %q)", c.b, c.a) + assert.Equal(t, c.want, editDistance(c.a, c.b), "editDistance(%q, %q)", c.a, c.b) + assert.Equal(t, c.want, editDistance(c.b, c.a), "editDistance(%q, %q)", c.b, c.a) } } diff --git a/unknown_command_test.go b/unknown_command_test.go index 629d6da..7d1982e 100644 --- a/unknown_command_test.go +++ b/unknown_command_test.go @@ -181,16 +181,70 @@ func TestUnexpectedArgsWithoutSubCommands(t *testing.T) { assert.NotContains(t, err.Error(), "error parsing flags") } -func TestUnknownFlagStillReportsParseError(t *testing.T) { - // Genuine flag problems keep the flag-parsing prefix. - d := NewDispatcher("myapp") - fs := NewFlagSet("deploy") - fs.Bool("verbose", 'v', false, "Verbose output") - d.Dispatch("deploy", NewCommand(fs, - func(fs *FlagSet, args []string) error { return nil }, WithUsage("Deploy"))) +func TestUnknownFlagSuggestions(t *testing.T) { + newDispatcher := func() *Dispatcher { + d := NewDispatcher("myapp") + fs := NewFlagSet("deploy") + fs.Bool("verbose", 'v', false, "Verbose output") + fs.String("name", 'n', "", "Application name") + fs.String("namespace", 0, "", "Namespace") + d.Dispatch("deploy", NewCommand(fs, + func(fs *FlagSet, args []string) error { return nil }, WithUsage("Deploy"))) + return d + } - err := d.Execute([]string{"deploy", "--nope"}) - require.Error(t, err) - assert.Contains(t, err.Error(), "error parsing flags") - assert.ErrorIs(t, err, ErrUnknownFlag) + t.Run("suggests a close long flag", func(t *testing.T) { + err := newDispatcher().Execute([]string{"deploy", "--naem", "x"}) + require.Error(t, err) + + var ufe *UnknownFlagError + require.ErrorAs(t, err, &ufe) + assert.Equal(t, "--naem", ufe.Flag) + assert.Equal(t, []string{"--name"}, ufe.Suggestions) + + assert.Equal(t, `unknown flag: --naem + +Did you mean? + --name`, err.Error()) + }) + + t.Run("still unwraps to ErrUnknownFlag", func(t *testing.T) { + err := newDispatcher().Execute([]string{"deploy", "--naem", "x"}) + assert.ErrorIs(t, err, ErrUnknownFlag) + }) + + t.Run("drops the redundant flag-parsing prefix", func(t *testing.T) { + err := newDispatcher().Execute([]string{"deploy", "--naem", "x"}) + require.Error(t, err) + assert.NotContains(t, err.Error(), "error parsing flags") + }) + + t.Run("nothing close offers no guess", func(t *testing.T) { + err := newDispatcher().Execute([]string{"deploy", "--zzzzzzzz"}) + require.Error(t, err) + assert.Equal(t, "unknown flag: --zzzzzzzz", err.Error()) + }) + + t.Run("short flags carry no suggestions", func(t *testing.T) { + err := newDispatcher().Execute([]string{"deploy", "-q"}) + require.Error(t, err) + + var ufe *UnknownFlagError + require.ErrorAs(t, err, &ufe) + assert.Equal(t, "-q", ufe.Flag) + assert.Empty(t, ufe.Suggestions) + assert.Equal(t, "unknown flag: -q", err.Error()) + }) + + t.Run("a value flag keeps the parse-error prefix", func(t *testing.T) { + d := NewDispatcher("myapp") + fs := NewFlagSet("deploy") + fs.Int("port", 'p', 0, "Port") + d.Dispatch("deploy", NewCommand(fs, + func(fs *FlagSet, args []string) error { return nil }, WithUsage("Deploy"))) + + err := d.Execute([]string{"deploy", "--port", "notanumber"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "error parsing flags") + }) } From 163db4e8274b4c41bd2293d7d01b6644b66a6bd0 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Fri, 11 Sep 2026 16:24:58 -0700 Subject: [PATCH 3/3] Address review: flag-only input, hidden flags, rune counts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes from review on #17. Flag-only input reported `unknown command ""`. Execute filters flag tokens out before building the error, so `miren --bogus` had no word left to name — a regression against the old `unknown command: --bogus`. It now reports the flag instead, without suggestions, since no command matched and there is no flag set to draw candidates from. `--bogus=value` reports just `--bogus`. Hidden flags no longer appear in suggestions. help.go already keeps them out of help output, and offering one on a near-miss typo would undo that. Prefix completions rank by characters rather than bytes, so a short non-ASCII completion is no longer sorted behind a longer ASCII one. The rest of the matching was already rune-based; this line was the holdout. --- dispatcher.go | 17 +++++++++++++++++ mflags.go | 7 ++++++- suggest.go | 8 +++++++- suggest_test.go | 8 ++++++++ unknown_command_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 2 deletions(-) diff --git a/dispatcher.go b/dispatcher.go index 907a849..a2c59f0 100644 --- a/dispatcher.go +++ b/dispatcher.go @@ -296,6 +296,23 @@ func (d *Dispatcher) Execute(args []string) error { if hasHelp { return d.showHelp() } + + // Nothing but flags was typed, so there is no word to call an unknown + // command. Name the flag instead. No command matched, so there is no + // flag set to draw suggestions from. + if len(nonFlagArgs) == 0 { + for _, arg := range args { + if arg == "--" { + break + } + if strings.HasPrefix(arg, "-") && arg != "-" { + // Report "--bogus", not "--bogus=value". + name, _, _ := strings.Cut(arg, "=") + return &UnknownFlagError{Flag: name} + } + } + } + return d.unknownCommandError(nonFlagArgs) } diff --git a/mflags.go b/mflags.go index 36c1ca2..7b313b7 100644 --- a/mflags.go +++ b/mflags.go @@ -712,7 +712,12 @@ func (f *FlagSet) validateRequired() error { // flags whose names are closest to what was typed. func (f *FlagSet) unknownLongFlagError(name string) error { candidates := make([]string, 0, len(f.flags)) - for defined := range f.flags { + for defined, flag := range f.flags { + // Hidden flags stay out of help output, so advertising one here on a + // near-miss typo would undo that. + if flag.Hidden { + continue + } candidates = append(candidates, defined) } diff --git a/suggest.go b/suggest.go index 0612d83..31d017f 100644 --- a/suggest.go +++ b/suggest.go @@ -106,7 +106,13 @@ func suggestNames(unknown string, candidates []string) []string { // A prefix match is strong evidence on its own — an abbreviation or a // half-typed word — so it bypasses the distance threshold. if len(c) > len(unknown) && strings.HasPrefix(c, unknown) { - matches = append(matches, scored{name: c, prefix: true, dist: len(c) - len(unknown)}) + // Rank by how much is left to type, in characters — bytes would + // sort a short non-ASCII completion behind a longer ASCII one. + matches = append(matches, scored{ + name: c, + prefix: true, + dist: len([]rune(c)) - len([]rune(unknown)), + }) continue } diff --git a/suggest_test.go b/suggest_test.go index cb0ab21..df742f2 100644 --- a/suggest_test.go +++ b/suggest_test.go @@ -55,6 +55,14 @@ func TestSuggestNames(t *testing.T) { assert.Equal(t, []string{"apply", "applesauce"}, suggestNames("appl", []string{"apply", "applesauce"})) }) + t.Run("prefix completions rank by characters, not bytes", func(t *testing.T) { + // Past the shared "a", "é" is one character but two bytes, while "bc" + // is two of each. Measured in bytes the two tie and sort + // alphabetically, putting the longer completion first. + got := suggestNames("a", []string{"abc", "aé"}) + assert.Equal(t, []string{"aé", "abc"}, got) + }) + t.Run("nothing close returns nil", func(t *testing.T) { assert.Nil(t, suggestNames("zzzzzz", commands)) }) diff --git a/unknown_command_test.go b/unknown_command_test.go index 7d1982e..93bf34e 100644 --- a/unknown_command_test.go +++ b/unknown_command_test.go @@ -128,6 +128,28 @@ Run 'myapp --help' to see available commands.`, err.Error()) assert.NotContains(t, err.Error(), "Did you mean") }) + t.Run("flag-only input names the flag, not an empty command", func(t *testing.T) { + for _, args := range [][]string{ + {"--bogus"}, + {"-x"}, + {"--bogus", "--other"}, + {"--bogus=value"}, + } { + err := unknownCommandDispatcher().Execute(args) + require.Error(t, err) + + var ufe *UnknownFlagError + require.ErrorAs(t, err, &ufe, "args %q", args) + assert.NotContains(t, err.Error(), `unknown command ""`) + } + }) + + t.Run("flag-only input reports the flag as typed", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"--bogus=value"}) + require.Error(t, err) + assert.Equal(t, "unknown flag: --bogus", err.Error()) + }) + t.Run("only the first word is blamed", func(t *testing.T) { err := unknownCommandDispatcher().Execute([]string{"depoy", "myapp"}) require.Error(t, err) @@ -236,6 +258,25 @@ Did you mean? assert.Equal(t, "unknown flag: -q", err.Error()) }) + t.Run("hidden flags are never suggested", func(t *testing.T) { + d := NewDispatcher("myapp") + fs := NewFlagSet("deploy") + fs.String("name", 'n', "", "Application name") + fs.String("internal-token", 0, "", "Internal token") + fs.flags["internal-token"].Hidden = true + d.Dispatch("deploy", NewCommand(fs, + func(fs *FlagSet, args []string) error { return nil }, WithUsage("Deploy"))) + + // One edit from the hidden flag, so only its hidden-ness keeps it out. + err := d.Execute([]string{"deploy", "--internal-tokn", "x"}) + require.Error(t, err) + + var ufe *UnknownFlagError + require.ErrorAs(t, err, &ufe) + assert.Empty(t, ufe.Suggestions) + assert.NotContains(t, err.Error(), "internal-token") + }) + t.Run("a value flag keeps the parse-error prefix", func(t *testing.T) { d := NewDispatcher("myapp") fs := NewFlagSet("deploy")