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

Fix short option with an embedded = later in the argument #6

Merged
merged 1 commit into from Feb 9, 2023

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Oct 16, 2022

Problem

A short option with an embedded = later in the argument is parsed inconsistently and with data loss.

In particular:

-a=b=c gives same result as -a=b (lost trailing =c)

-ab=c gives same result as -a=c (lost leading b)

Fixes #5

Solution

Make code more explicit and test for = at start when looking for possible value.

-a=b=c makes option a have value b=c.

-ab=c makes option a true and option b have value c.

(In my first commit I made -ab=c give option b the value =c by only treating the = as special at the start. I checked Yargs and found it set value to c. I can see a rationale for the parsing to treat it like an expansion, so -ab=c => -a -b=c, and option b has value c. More comfortable matching Yargs with this behaviour.)

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Seems like a clear semver-patch, thanks

@codecov-commenter
Copy link

codecov-commenter commented Feb 9, 2023

Codecov Report

Merging #6 (b62a195) into main (3124ed3) will not change coverage.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main       #6   +/-   ##
=======================================
  Coverage   98.75%   98.75%           
=======================================
  Files           1        1           
  Lines         161      161           
  Branches       71       71           
=======================================
  Hits          159      159           
  Misses          2        2           
Impacted Files Coverage Δ
index.js 98.75% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ljharb ljharb merged commit 9c7dc85 into minimistjs:main Feb 9, 2023
@shadowspawn shadowspawn deleted the feature/multiple-equals branch February 19, 2023 00:26
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.

Repeated occurence of = isnt handled correctly
3 participants