Skip to content
This repository was archived by the owner on Aug 22, 2023. It is now read-only.

Stop processing flag multiple when a non-option flag is found - #137

Open
firefish5000 wants to merge 4 commits into
oclif:mainfrom
firefish5000:patch-flag-multiple
Open

Stop processing flag multiple when a non-option flag is found#137
firefish5000 wants to merge 4 commits into
oclif:mainfrom
firefish5000:patch-flag-multiple

Conversation

@firefish5000

@firefish5000 firefish5000 commented Nov 16, 2020

Copy link
Copy Markdown

Currently, if we have a flag that accepts multiple values followed by a boolean flag and then positional arguments, the multiple flag will erroneously receive the arguments that come after the boolean flag. Example

myProg --extensions .js .ts --strict ./myDir1 ./myDir2
# extensions is set to [ '.js', '.ts', './myDir1', './myDir2' ]

This PR changes that behavior so boolean flags end the parsing just like option flags do.

myProg --extensions .js .ts --strict ./myDir1 ./myDir2
# extensions is set to [ '.js', '.ts' ]

Bug is due to this.currentFlag not being set for non-option (bool) flags in parse.ts lines 109-147

@salesforce-cla

Copy link
Copy Markdown

Thanks for the contribution! Before we can merge this, we need @firefish5000 to sign the Salesforce.com Contributor License Agreement.

@firefish5000 firefish5000 changed the title Stop processing Flag multiple when another flag is found Stop processing flag multiple when a non-option flag is found Nov 16, 2020
@firefish5000

firefish5000 commented Nov 16, 2020

Copy link
Copy Markdown
Author

Unrelated, but while reading the test I had to question why we have --foo '=bar' resolve to {foo: 'bar'}. It seems like an unintuitive edge case for the end users who are trying to pass strings that may or may not start with an = sign to flags.
Parsing the equal out of --foo=bar is fine, standard even. But that space, imo, clearly signifies that whatever string comes after it is the argument, equal sign included.

Also unrelated. Commits 120e121 and 76fa11c are just expansion of test cases. Not really part of this issue. These were things I needed to assure myself occurred in general and didn't see in the tests

@firefish5000
firefish5000 marked this pull request as ready for review November 16, 2020 09:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant