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(cli): error on conflicting options #72

Merged
merged 2 commits into from Feb 20, 2020

Conversation

lirantal
Copy link
Owner

Description

BREAKING CHANGE: CLI may show an error when arguments conflict
and the order of short and long options was reversed to be
more descriptive on CLI options errors.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Related Issue

#64

@lirantal lirantal added the enhancement New feature or request label Feb 19, 2020
@lirantal lirantal self-assigned this Feb 19, 2020
@lirantal
Copy link
Owner Author

@XhmikosR unfortunately there doesn't seem to be any way to figure out which argument was used with yargs so even trying to set a custom failure message would not work.

The most improvement I could find at the moment is specifying the long name of the option instead of the short one, for which I also flipped them in the help/usage message.

I'll merge this as a breaking change to bump a major version.

@codecov-io
Copy link

codecov-io commented Feb 19, 2020

Codecov Report

Merging #72 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #72   +/-   ##
=======================================
  Coverage   95.08%   95.08%           
=======================================
  Files          11       11           
  Lines         183      183           
  Branches       29       29           
=======================================
  Hits          174      174           
  Misses          8        8           
  Partials        1        1
Impacted Files Coverage Δ
...s/lockfile-lint-api/src/validators/ValidateHost.js 100% <100%> (ø) ⬆️
...lockfile-lint-api/src/validators/ValidateScheme.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53d57ba...f773455. Read the comment docs.

@XhmikosR
Copy link
Contributor

@lirantal no worries, thanks for looking into it :)

PS. it does sound weird that you can't access the used arg, but I haven't spent a lot of time with yargs to be able to help

@lirantal
Copy link
Owner Author

I'll merge this in as a major change anyway and I also pinged @bcoe for his take on the yargs setup. We can always push a fix if we find a better way and friendlier way.

@lirantal lirantal merged commit 3e0eb59 into master Feb 20, 2020
@lirantal lirantal deleted the fix/conflicting-cli-arguments branch July 7, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants