From 91ccfd5d6329292cc1fa80cc3bd8171c4ea8d733 Mon Sep 17 00:00:00 2001 From: John Gee Date: Mon, 6 Sep 2021 18:27:30 +1200 Subject: [PATCH] Suggestion for unknown command and unknown option (#1590) * Proof of concept suggestion for unknown command * Leave length check to similarity test * Fix JSDoc * Add tests * Fix import * Offer multiple suggestions * Add search for similar option * Add global options to suggestions * Show unknown (global) option rather than help * Add tests for help command and option suggestions * Fix option suggestions for subcommands, and first raft of tests for option suggestions * Do not suggest hidden candidates. Remove duplicates. * Tiny comment change * Add test for fixed behaviour, unknown option before subcommand * Remove low value local variable * Suppress output from test * Add showSuggestionAfterError * Fix arg for parse * Suggestions off by default for now * Add to README * Remove development trace statement * Describe scenario using same terms as error * Add test that command:* listener blocks command suggestion --- Readme.md | 12 + lib/command.js | 48 +++- lib/suggestSimilar.js | 100 +++++++++ tests/command.chain.test.js | 6 + .../command.showSuggestionAfterError.test.js | 50 +++++ tests/command.unknownOption.test.js | 15 ++ tests/help.suggestion.test.js | 210 ++++++++++++++++++ typings/index.d.ts | 7 +- typings/index.test-d.ts | 4 + 9 files changed, 449 insertions(+), 3 deletions(-) create mode 100644 lib/suggestSimilar.js create mode 100644 tests/command.showSuggestionAfterError.test.js create mode 100644 tests/help.suggestion.test.js diff --git a/Readme.md b/Readme.md index 6e84460ce..a43dfc058 100644 --- a/Readme.md +++ b/Readme.md @@ -692,6 +692,18 @@ error: unknown option '--unknown' (add --help for additional information) ``` +You can also show suggestions after an error for an unknown command or option. + +```js +program.showSuggestionAfterError(); +``` + +```sh +$ pizza --hepl +error: unknown option '--hepl' +(Did you mean --help?) +``` + ### Display help from code `.help()`: display help information and exit immediately. You can optionally pass `{ error: true }` to display on stderr and exit with an error status. diff --git a/lib/command.js b/lib/command.js index 4b44f4568..184bc0b79 100644 --- a/lib/command.js +++ b/lib/command.js @@ -7,6 +7,7 @@ const { Argument, humanReadableArgName } = require('./argument.js'); const { CommanderError } = require('./error.js'); const { Help } = require('./help.js'); const { Option, splitOptionFlags } = require('./option.js'); +const { suggestSimilar } = require('./suggestSimilar'); // @ts-check @@ -51,6 +52,7 @@ class Command extends EventEmitter { this._lifeCycleHooks = {}; // a hash of arrays /** @type {boolean | string} */ this._showHelpAfterError = false; + this._showSuggestionAfterError = false; // see .configureOutput() for docs this._outputConfiguration = { @@ -99,6 +101,7 @@ class Command extends EventEmitter { this._allowExcessArguments = sourceCommand._allowExcessArguments; this._enablePositionalOptions = sourceCommand._enablePositionalOptions; this._showHelpAfterError = sourceCommand._showHelpAfterError; + this._showSuggestionAfterError = sourceCommand._showSuggestionAfterError; return this; } @@ -233,6 +236,17 @@ class Command extends EventEmitter { return this; } + /** + * Display suggestion of similar commands for unknown commands, or options for unknown options. + * + * @param {boolean} [displaySuggestion] + * @return {Command} `this` command for chaining + */ + showSuggestionAfterError(displaySuggestion = true) { + this._showSuggestionAfterError = !!displaySuggestion; + return this; + } + /** * Add a prepared subcommand. * @@ -1213,6 +1227,7 @@ Expecting one of '${allowedValues.join("', '")}'`); this._processArguments(); } } else if (this.commands.length) { + checkForUnknownOptions(); // This command has subcommands and nothing hooked up at this level, so display help (and exit). this.help({ error: true }); } else { @@ -1500,7 +1515,23 @@ Expecting one of '${allowedValues.join("', '")}'`); unknownOption(flag) { if (this._allowUnknownOption) return; - const message = `error: unknown option '${flag}'`; + let suggestion = ''; + + if (flag.startsWith('--') && this._showSuggestionAfterError) { + // Looping to pick up the global options too + let candidateFlags = []; + let command = this; + do { + const moreFlags = command.createHelp().visibleOptions(command) + .filter(option => option.long) + .map(option => option.long); + candidateFlags = candidateFlags.concat(moreFlags); + command = command.parent; + } while (command && !command._enablePositionalOptions); + suggestion = suggestSimilar(flag, candidateFlags); + } + + const message = `error: unknown option '${flag}'${suggestion}`; this._displayError(1, 'commander.unknownOption', message); }; @@ -1528,7 +1559,20 @@ Expecting one of '${allowedValues.join("', '")}'`); */ unknownCommand() { - const message = `error: unknown command '${this.args[0]}'`; + const unknownName = this.args[0]; + let suggestion = ''; + + if (this._showSuggestionAfterError) { + const candidateNames = []; + this.createHelp().visibleCommands(this).forEach((command) => { + candidateNames.push(command.name()); + // just visible alias + if (command.alias()) candidateNames.push(command.alias()); + }); + suggestion = suggestSimilar(unknownName, candidateNames); + } + + const message = `error: unknown command '${unknownName}'${suggestion}`; this._displayError(1, 'commander.unknownCommand', message); }; diff --git a/lib/suggestSimilar.js b/lib/suggestSimilar.js new file mode 100644 index 000000000..9a4066c71 --- /dev/null +++ b/lib/suggestSimilar.js @@ -0,0 +1,100 @@ +const maxDistance = 3; + +function editDistance(a, b) { + // https://en.wikipedia.org/wiki/Damerau–Levenshtein_distance + // Calculating optimal string alignment distance, no substring is edited more than once. + // (Simple implementation.) + + // Quick early exit, return worst case. + if (Math.abs(a.length - b.length) > maxDistance) return Math.max(a.length, b.length); + + // distance between prefix substrings of a and b + const d = []; + + // pure deletions turn a into empty string + for (let i = 0; i <= a.length; i++) { + d[i] = [i]; + } + // pure insertions turn empty string into b + for (let j = 0; j <= b.length; j++) { + d[0][j] = j; + } + + // fill matrix + for (let j = 1; j <= b.length; j++) { + for (let i = 1; i <= a.length; i++) { + let cost = 1; + if (a[i - 1] === b[j - 1]) { + cost = 0; + } else { + cost = 1; + } + d[i][j] = Math.min( + d[i - 1][j] + 1, // deletion + d[i][j - 1] + 1, // insertion + d[i - 1][j - 1] + cost // substitution + ); + // transposition + if (i > 1 && j > 1 && a[i - 1] === b[j - 2] && a[i - 2] === b[j - 1]) { + d[i][j] = Math.min(d[i][j], d[i - 2][j - 2] + 1); + } + } + } + + return d[a.length][b.length]; +} + +/** + * Find close matches, restricted to same number of edits. + * + * @param {string} word + * @param {string[]} candidates + * @returns {string} + */ + +function suggestSimilar(word, candidates) { + if (!candidates || candidates.length === 0) return ''; + // remove possible duplicates + candidates = Array.from(new Set(candidates)); + + const searchingOptions = word.startsWith('--'); + if (searchingOptions) { + word = word.slice(2); + candidates = candidates.map(candidate => candidate.slice(2)); + } + + let similar = []; + let bestDistance = maxDistance; + const minSimilarity = 0.4; + candidates.forEach((candidate) => { + if (candidate.length <= 1) return; // no one character guesses + + const distance = editDistance(word, candidate); + const length = Math.max(word.length, candidate.length); + const similarity = (length - distance) / length; + if (similarity > minSimilarity) { + if (distance < bestDistance) { + // better edit distance, throw away previous worse matches + bestDistance = distance; + similar = [candidate]; + } else if (distance === bestDistance) { + similar.push(candidate); + } + } + }); + + similar.sort((a, b) => a.localeCompare(b)); + if (searchingOptions) { + similar = similar.map(candidate => `--${candidate}`); + } + + if (similar.length > 1) { + return `\n(Did you mean one of ${similar.join(', ')}?)`; + } + if (similar.length === 1) { + return `\n(Did you mean ${similar[0]}?)`; + } + return ''; +} + +exports.suggestSimilar = suggestSimilar; diff --git a/tests/command.chain.test.js b/tests/command.chain.test.js index e80c2b292..09fc6e5a3 100644 --- a/tests/command.chain.test.js +++ b/tests/command.chain.test.js @@ -184,6 +184,12 @@ describe('Command methods that should return this for chaining', () => { expect(result).toBe(program); }); + test('when call .showSuggestionAfterError() then returns this', () => { + const program = new Command(); + const result = program.showSuggestionAfterError(); + expect(result).toBe(program); + }); + test('when call .copyInheritedSettings() then returns this', () => { const program = new Command(); const cmd = new Command(); diff --git a/tests/command.showSuggestionAfterError.test.js b/tests/command.showSuggestionAfterError.test.js new file mode 100644 index 000000000..2e596f408 --- /dev/null +++ b/tests/command.showSuggestionAfterError.test.js @@ -0,0 +1,50 @@ +const { Command } = require('../'); + +function getSuggestion(program, arg) { + let message = ''; + program + .exitOverride() + .configureOutput({ + writeErr: (str) => { message = str; } + }); + + try { + program.parse([arg], { from: 'user' }); + } catch (err) { + } + + const match = message.match(/Did you mean (one of )?(.*)\?/); + return match ? match[2] : null; +}; + +test('when unknown command and showSuggestionAfterError() then show suggestion', () => { + const program = new Command(); + program.showSuggestionAfterError(); + program.command('example'); + const suggestion = getSuggestion(program, 'exampel'); + expect(suggestion).toBe('example'); +}); + +test('when unknown command and showSuggestionAfterError(false) then do not show suggestion', () => { + const program = new Command(); + program.showSuggestionAfterError(false); + program.command('example'); + const suggestion = getSuggestion(program, 'exampel'); + expect(suggestion).toBe(null); +}); + +test('when unknown option and showSuggestionAfterError() then show suggestion', () => { + const program = new Command(); + program.showSuggestionAfterError(); + program.option('--example'); + const suggestion = getSuggestion(program, '--exampel'); + expect(suggestion).toBe('--example'); +}); + +test('when unknown option and showSuggestionAfterError(false) then do not show suggestion', () => { + const program = new Command(); + program.showSuggestionAfterError(false); + program.option('--example'); + const suggestion = getSuggestion(program, '--exampel'); + expect(suggestion).toBe(null); +}); diff --git a/tests/command.unknownOption.test.js b/tests/command.unknownOption.test.js index 7ab72237d..608f3f148 100644 --- a/tests/command.unknownOption.test.js +++ b/tests/command.unknownOption.test.js @@ -95,4 +95,19 @@ describe('unknownOption', () => { } expect(caughtErr.code).toBe('commander.unknownOption'); }); + + test('when specify unknown global option before subcommand then error', () => { + const program = new commander.Command(); + program + .exitOverride(); + program.command('sub'); + + let caughtErr; + try { + program.parse(['--NONSENSE', 'sub'], { from: 'user' }); + } catch (err) { + caughtErr = err; + } + expect(caughtErr.code).toBe('commander.unknownOption'); + }); }); diff --git a/tests/help.suggestion.test.js b/tests/help.suggestion.test.js new file mode 100644 index 000000000..711eb1411 --- /dev/null +++ b/tests/help.suggestion.test.js @@ -0,0 +1,210 @@ +const { Command, Option } = require('../'); + +// Note: setting up shared command configuration in getSuggestion, +// and looking for possible subcommand 'sub'. + +function getSuggestion(program, arg) { + let message = ''; + program + .showSuggestionAfterError() // make sure on + .exitOverride() + .configureOutput({ + writeErr: (str) => { message = str; } + }); + // Do the same setup for subcommand. + const sub = program._findCommand('sub'); + if (sub) sub.copyInheritedSettings(program); + + try { + // Passing in an array for a few of the tests. + const args = Array.isArray(arg) ? arg : [arg]; + program.parse(args, { from: 'user' }); + } catch (err) { + } + + const match = message.match(/Did you mean (one of )?(.*)\?/); + return match ? match[2] : null; +}; + +test.each([ + ['yyy', ['zzz'], null, 'none similar'], + ['a', ['b'], null, 'one edit away but not similar'], + ['a', ['ab'], 'ab', 'one edit away'], + ['ab', ['a'], null, 'one edit away'], + ['at', ['cat'], 'cat', '1 insertion'], + ['cat', ['at'], 'at', '1 deletion'], + ['bat', ['cat'], 'cat', '1 substitution'], + ['act', ['cat'], 'cat', '1 transposition'], + ['cxx', ['cat'], null, '2 edits away and short string'], + ['caxx', ['cart'], 'cart', '2 edits away and longer string'], + ['1234567', ['1234567890'], '1234567890', '3 edits away is similar for long string'], + ['123456', ['1234567890'], null, '4 edits is too far'], + ['xat', ['rat', 'cat', 'bat'], 'bat, cat, rat', 'sorted possibles'], + ['cart', ['camb', 'cant', 'bard'], 'cant', 'only closest of different edit distances'] +])('when cli of %s and commands %j then suggest %s because %s', (arg, commandNames, expected) => { + const program = new Command(); + commandNames.forEach(name => { program.command(name); }); + const suggestion = getSuggestion(program, arg); + expect(suggestion).toBe(expected); +}); + +test('when similar alias then suggest alias', () => { + const program = new Command(); + program.command('xyz') + .alias('car'); + const suggestion = getSuggestion(program, 'bar'); + expect(suggestion).toBe('car'); +}); + +test('when similar hidden alias then not suggested', () => { + const program = new Command(); + program.command('xyz') + .alias('visible') + .alias('silent'); + const suggestion = getSuggestion(program, 'slent'); + expect(suggestion).toBe(null); +}); + +test('when similar command and alias then suggest both', () => { + const program = new Command(); + program.command('aaaaa') + .alias('cat'); + program.command('bat'); + program.command('ccccc'); + const suggestion = getSuggestion(program, 'mat'); + expect(suggestion).toBe('bat, cat'); +}); + +test('when implicit help command then help is candidate for suggestion', () => { + const program = new Command(); + program.command('sub'); + const suggestion = getSuggestion(program, 'hepl'); + expect(suggestion).toBe('help'); +}); + +test('when help command disabled then not candidate for suggestion', () => { + const program = new Command(); + program.addHelpCommand(false); + program.command('sub'); + const suggestion = getSuggestion(program, 'hepl'); + expect(suggestion).toBe(null); +}); + +test('when default help option then --help is candidate for suggestion', () => { + const program = new Command(); + const suggestion = getSuggestion(program, '--hepl'); + expect(suggestion).toBe('--help'); +}); + +test('when custom help option then --custom-help is candidate for suggestion', () => { + const program = new Command(); + program.helpOption('-H, --custom-help'); + const suggestion = getSuggestion(program, '--custom-hepl'); + expect(suggestion).toBe('--custom-help'); +}); + +test('when help option disabled then not candidate for suggestion', () => { + const program = new Command(); + program.helpOption(false); + const suggestion = getSuggestion(program, '--hepl'); + expect(suggestion).toBe(null); +}); + +test('when command:* listener and unknown command then no suggestion', () => { + // Because one use for command:* was to handle unknown commands. + // Listener actually stops error being thrown, but we just care about affect on suggestion in this test. + const program = new Command(); + program.on('command:*', () => {}); + program.command('rat'); + const suggestion = getSuggestion(program, 'cat'); + expect(suggestion).toBe(null); +}); + +// Easy to just run same tests as for commands with cut and paste! +// Note: length calculations disregard the leading -- +test.each([ + ['--yyy', ['--zzz'], null, 'none similar'], + ['--a', ['--b'], null, 'one edit away but not similar'], + ['--a', ['--ab'], '--ab', 'one edit away'], + ['--ab', ['--a'], null, 'one edit away'], + ['--at', ['--cat'], '--cat', '1 insertion'], + ['--cat', ['--at'], '--at', '1 deletion'], + ['--bat', ['--cat'], '--cat', '1 substitution'], + ['--act', ['--cat'], '--cat', '1 transposition'], + ['--cxx', ['--cat'], null, '2 edits away and short string'], + ['--caxx', ['--cart'], '--cart', '2 edits away and longer string'], + ['--1234567', ['--1234567890'], '--1234567890', '3 edits away is similar for long string'], + ['--123456', ['--1234567890'], null, '4 edits is too far'], + ['--xat', ['--rat', '--cat', '--bat'], '--bat, --cat, --rat', 'sorted possibles'], + ['--cart', ['--camb', '--cant', '--bard'], '--cant', 'only closest of different edit distances'] +])('when cli of %s and options %j then suggest %s because %s', (arg, commandNames, expected) => { + const program = new Command(); + commandNames.forEach(name => { program.option(name); }); + const suggestion = getSuggestion(program, arg); + expect(suggestion).toBe(expected); +}); + +test('when no options then no suggestion', () => { + // Checking nothing blows up as much as no suggestion! + const program = new Command(); + program + .helpOption(false); + const suggestion = getSuggestion(program, '--option'); + expect(suggestion).toBe(null); +}); + +test('when subcommand option then candidate for subcommand option suggestion', () => { + const program = new Command(); + program.command('sub') + .option('-l,--local'); + const suggestion = getSuggestion(program, ['sub', '--loca']); + expect(suggestion).toBe('--local'); +}); + +test('when global option then candidate for subcommand option suggestion', () => { + const program = new Command(); + program.option('-g, --global'); + program.command('sub'); + const suggestion = getSuggestion(program, ['sub', '--globla']); + expect(suggestion).toBe('--global'); +}); + +test('when global option but positionalOptions then not candidate for subcommand suggestion', () => { + const program = new Command(); + program + .enablePositionalOptions(); + program.option('-g, --global'); + program.command('sub'); + const suggestion = getSuggestion(program, ['sub', '--globla']); + expect(suggestion).toBe(null); +}); + +test('when global and local options then both candidates', () => { + const program = new Command(); + program.option('--cat'); + program.command('sub') + .option('--rat'); + const suggestion = getSuggestion(program, ['sub', '--bat']); + expect(suggestion).toBe('--cat, --rat'); +}); + +test('when command hidden then not suggested as candidate', () => { + const program = new Command(); + program.command('secret', { hidden: true }); + const suggestion = getSuggestion(program, 'secrt'); + expect(suggestion).toBe(null); +}); + +test('when option hidden then not suggested as candidate', () => { + const program = new Command(); + program.addOption(new Option('--secret').hideHelp()); + const suggestion = getSuggestion(program, '--secrt'); + expect(suggestion).toBe(null); +}); + +test('when may be duplicate identical candidates then only return one', () => { + const program = new Command(); + program.command('sub'); + const suggestion = getSuggestion(program, ['sub', '--hepl']); + expect(suggestion).toBe('--help'); +}); diff --git a/typings/index.d.ts b/typings/index.d.ts index 725764b6a..43eeae563 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -412,7 +412,7 @@ export class Command { * * (Used internally when adding a command using `.command()` so subcommands inherit parent settings.) */ - copyInheritedSettings(sourceCommand: Command): this; + copyInheritedSettings(sourceCommand: Command): this; /** * Display the help or a custom message after an error occurs. @@ -420,6 +420,11 @@ export class Command { showHelpAfterError(displayHelp?: boolean | string): this; /** + * Display suggestion of similar commands for unknown commands, or options for unknown options. + */ + showSuggestionAfterError(displaySuggestion?: boolean): this; + + /** * Register callback `fn` for the command. * * @example diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index d37a0e386..ba1051092 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -293,6 +293,10 @@ expectType(program.showHelpAfterError()); expectType(program.showHelpAfterError(true)); expectType(program.showHelpAfterError('See --help')); +// showSuggestionAfterError +expectType(program.showSuggestionAfterError()); +expectType(program.showSuggestionAfterError(false)); + // configureOutput expectType(program.configureOutput({ })); expectType(program.configureOutput());