Skip to content
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: Added default fallback for CLI #271

Merged
merged 3 commits into from
Aug 8, 2016
Merged

Conversation

adampash
Copy link
Contributor

@adampash adampash commented Aug 5, 2016

Addresses #263

I considered automatically printing the help after the error message, but I decided it would get annoying to see the whole error message if you just entered a typo. It also made it more difficult to focus in on the fact that there was and error in the first place. So I went with this.

I pulled in chalk, since you're using it elsewhere, but if you don't like it, I can remove it!

Last: In an ideal world, we'd look at the text of the command and see if it's close to an existing lux command and present that to the user, like a "did you mean..." — but I'm trying to avoid letting this creep too far for a simple feature addition. 😄

@zacharygolba
Copy link
Contributor

That's a good call and I totally agree with you. It's important that we gracefully handle this error and it's more or less bonus points if we implement fuzziness like the git cli down the road.

@@ -32,6 +33,14 @@ function inLuxProject() {
}
}

function commandNotFound(cmd) {
console.log();
Copy link
Contributor

@zacharygolba zacharygolba Aug 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These empty console.log's are a little hard to understand. I'm assuming this is to add a line break? If so, I suggest we do the following (It's not any more pretty but it's a bit more obvious what's going on imo):

const { EOL } = require('os');

function commandNotFound(cmd) {
  console.log(`${EOL}  ${red(cmd)} isn't a valid command${EOL}`);
  console.log(`  Type ${green('lux --help')} for a full list of commands${EOL}`);
}

os.EOL is used to determine the appropriate line breaks for the platform Lux is installed on.

@zacharygolba
Copy link
Contributor

👏 This works great! I just made a few minor suggestions above.

@zacharygolba zacharygolba added this to the 1.0 milestone Aug 5, 2016
@adampash adampash force-pushed the feat-add-cli-fallback branch from 2ce2da5 to 21c8480 Compare August 8, 2016 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants