I read through the full diff and the core new files carefully. The feature — "did you mean?" suggestions for mistyped commands and flags — is well-designed and well-tested. Here's what I found.
What the PR does
It introduces a Damerau-Levenshtein edit-distance function in suggest.go, wires it through three new error types in errors.go (UnknownCommandError, UnknownFlagError, UnexpectedArgsError), and updates dispatcher.go and mflags.go to generate and return those errors instead of plain fmt.Errorf strings.
Correctness
The rolling-row edit-distance implementation in suggest.go is correct — the prevPrev/prev/curr rotation is the standard Damerau-Levenshtein approach. The threshold table (suggestThreshold) is conservative and sensible: no distance-based guessing for 1–2 character inputs, capping out at 3 for longer words. Prefix matching bypasses the threshold intentionally, and the comment explains why. I traced the unknownCommandError → unknownSubCommandError → suggestNames path and confirmed it correctly finds the deepest valid namespace before blaming a word.
Edge cases handled
The test suite in unknown_command_test.go covers the scenarios that matter most: prefix typos, sub-command typos, section-style commands with unknown flags, commands with positional args (which must not fire), rest-field commands (also must not fire), the help keyword, and the case where no close match exists. The TestUnknownCommandDoesNotFireOnValidInput table is a particularly good regression guard.
TestUnexpectedArgsWithoutSubCommands confirms that UnexpectedArgsError is returned unwrapped (no "error parsing flags:" prefix) even when there are no sub-commands to suggest.
The mistypedSubCommand guard — checking fs.PositionalCount() > 0 || fs.restField != nil before blaming an unknown word — is the right place to avoid false positives.
Breaking changes
The old unknown command: foo bar string is gone from the dispatcher; callers who relied on strings.Contains(err.Error(), "unknown command:") will now see unknown command "foo" format. That's a semantically better error, but it is a message-format change. The new UnknownCommandError and UnknownFlagError types give callers a structured alternative, which is a net improvement for programmatic use. ErrUnknownFlag remains unwrapped through UnknownFlagError.Unwrap(), so errors.Is callers are unaffected. This is the right call.
One small observation — not a blocker, but worth noting: unknownLongFlagError in mflags.go compares the typed name (stripped of --) against all candidate names (also stripped), which is correct. But suggestNames for flags is called only for long flags; short flags intentionally get no suggestions (the comment explains: a single letter is too little to guess from). That's a deliberate design choice and is tested.
Overall this is clean, thoroughly considered work. I'm happy to see it merged.
Verdict: ready
{
"owner": "mirendev",
"repo": "mflags",
"number": 17,
"verdict": "ready",
"event": "approve",
"summary": "I read through the full diff and the core new files carefully. The feature — \"did you mean?\" suggestions for mistyped commands and flags — is well-designed and well-tested. Here's what I found.\n\n**What the PR does**\n\nIt introduces a Damerau-Levenshtein edit-distance function in `suggest.go`, wires it through three new error types in `errors.go` (`UnknownCommandError`, `UnknownFlagError`, `UnexpectedArgsError`), and updates `dispatcher.go` and `mflags.go` to generate and return those errors instead of plain `fmt.Errorf` strings.\n\n**Correctness**\n\nThe rolling-row edit-distance implementation in `suggest.go` is correct — the `prevPrev`/`prev`/`curr` rotation is the standard Damerau-Levenshtein approach. The threshold table (`suggestThreshold`) is conservative and sensible: no distance-based guessing for 1–2 character inputs, capping out at 3 for longer words. Prefix matching bypasses the threshold intentionally, and the comment explains why. I traced the `unknownCommandError` → `unknownSubCommandError` → `suggestNames` path and confirmed it correctly finds the deepest valid namespace before blaming a word.\n\n**Edge cases handled**\n\nThe test suite in `unknown_command_test.go` covers the scenarios that matter most: prefix typos, sub-command typos, section-style commands with unknown flags, commands with positional args (which must *not* fire), rest-field commands (also must not fire), the `help` keyword, and the case where no close match exists. The `TestUnknownCommandDoesNotFireOnValidInput` table is a particularly good regression guard.\n\n`TestUnexpectedArgsWithoutSubCommands` confirms that `UnexpectedArgsError` is returned unwrapped (no \"error parsing flags:\" prefix) even when there are no sub-commands to suggest.\n\nThe `mistypedSubCommand` guard — checking `fs.PositionalCount() \u003e 0 || fs.restField != nil` before blaming an unknown word — is the right place to avoid false positives.\n\n**Breaking changes**\n\nThe old `unknown command: foo bar` string is gone from the dispatcher; callers who relied on `strings.Contains(err.Error(), \"unknown command:\")` will now see `unknown command \"foo\"` format. That's a semantically better error, but it is a message-format change. The new `UnknownCommandError` and `UnknownFlagError` types give callers a structured alternative, which is a net improvement for programmatic use. `ErrUnknownFlag` remains unwrapped through `UnknownFlagError.Unwrap()`, so `errors.Is` callers are unaffected. This is the right call.\n\n**One small observation** — not a blocker, but worth noting: `unknownLongFlagError` in `mflags.go` compares the typed name (stripped of `--`) against all candidate names (also stripped), which is correct. But `suggestNames` for flags is called only for long flags; short flags intentionally get no suggestions (the comment explains: a single letter is too little to guess from). That's a deliberate design choice and is tested.\n\nOverall this is clean, thoroughly considered work. I'm happy to see it merged.",
"posted_to_pr": true
}