New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: adds recommendCommands() for command suggestions #580
Conversation
TODO: sort by length before applying algorithm, so that we take the least-changes/longest word first. |
@@ -707,6 +713,10 @@ function Yargs (processArgs, cwd, parentRequire) { | |||
} | |||
} | |||
|
|||
// recommend a command if recommendCommands() has | |||
// been enabled, and no commands were found to execute | |||
if (recommendCommands && argv._.length) validation.recommendCommands(argv._[argv._.length - 1], handlerKeys) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this is called even when argv._
only contains valid commands or arguments (called as part of running the command); it just potentially doesn't have any commands to match against.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The argv._.length
check is actually redundant.
I would probably change lines 706 - 718 to this: // if there's a handler associated with a
// command defer processing to it.
var handlerKeys = command.getCommands()
if (handlerKeys.length) {
var firstUnknownCommand
for (var i = 0, cmd; (cmd = argv._[i]) !== undefined; i++) {
if (~handlerKeys.indexOf(cmd) && cmd !== completionCommand) {
setPlaceholderKeys(argv)
return command.runCommand(cmd, self, parsed)
} else if (!firstUnknownCommand && cmd !== completionCommand) {
firstUnknownCommand = cmd
}
}
// recommend a command if recommendCommands() has
// been enabled, and no commands were found to execute
if (recommendCommands && firstUnknownCommand) {
validation.recommendCommands(firstUnknownCommand, handlerKeys)
}
} What do you think? |
…end longest first
working from @maxrimue's branch for
recommendCommands()
, and implementing @nexdrew's code-review.This pull request gets us to
v1
of command recommendations (thanks @maxrimue for the hard work) addressing some of the nits in #562 (@maxrimue sorry for @nexdrew and I providing such a difficult review).Here's a breakdown of some of the changes made:
usage.js
, andvalidation.js
.strict()
forrecommendCommands()
rather than trying to be overly fancy with argument overriding.reviewers: @maxrimue @nexdrew I plan on stubbornly marching us towards
yargs@5.x
over the next week, appreciate your patience.