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

add colons to reduce ambiguity #485

Closed
wants to merge 1 commit into from

Conversation

arjunpdhaliwal
Copy link

Pull Request Template:

Describe what you did

Reduced ambiguity in an error message. A friend was having trouble as he typed "n install stable" rather than "n stable". An error message with a colon would have immediately indicated what was wrong, but the current message made him think that the version was incorrect. Note that adding "install" is the syntax in nvm, so people transitioning might be confused by this.

example:

Error: invalid version install

vs

Error: invalid version: install

How you did it

Added a colon to it.

How to verify it doesn't effect the functionality of n

The change is trivial, but can be verified by simply running the string with the abort function in a separate script.

Place description for the changelog in PR so we can tally all changes for any future release

Copy link

@dskrvk dskrvk left a comment

Choose a reason for hiding this comment

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

Looks good to me (not a maintainer though).

@shadowspawn
Copy link
Collaborator

Recent issue making same mistake: #519

@shadowspawn
Copy link
Collaborator

As this is a common (and reasonable) mistake I am considering just adding install as a functional command so no penalty for people using syntax from other utilities.

@shadowspawn shadowspawn self-assigned this Mar 30, 2019
@shadowspawn
Copy link
Collaborator

I have a couple of improvements in mind related to this confusion:

  • add install to match common usage (npm, brew, nvm, nvs)
  • consistently refer to parameters with single quotes 'parameter'

I won't pull in this Pull Request. If I have not released an improvement within a couple of months, feel free to call me on it!

Thank you for your contributions.

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.

3 participants