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: Ignore multiple spaces between arguments. #100

Merged
merged 1 commit into from Oct 5, 2017

Conversation

Mike111177
Copy link
Contributor

@Mike111177 Mike111177 commented Aug 29, 2017

Fixes #99

@Mike111177 Mike111177 changed the title Ignore multiple spaces between arguments. Fix: Ignore multiple spaces between arguments. Aug 29, 2017
@bcoe
Copy link
Member

bcoe commented Sep 3, 2017

@Mike111177 hey 👋 do to some other potential parsing issues I thought of with the parsing around maintaining quotes, I opted to rollback the logic surrounding && (prevC === ' ' || args[i].startsWith('-').

tldr; since most command lines already strip " and ' I'm tempted to advice people to think of alternative ways to pass quoted strings to an application, vs., trying to cleverly maintain quotes strings.

Do you mind potentially rebasing your work with master, taking into account that this logic has been removed?

@Mike111177
Copy link
Contributor Author

@bcoe Done.

@bcoe bcoe merged commit d137227 into yargs:master Oct 5, 2017
@bcoe
Copy link
Member

bcoe commented Oct 5, 2017

@Mike111177 thanks for the contribution.

@bcoe
Copy link
Member

bcoe commented Oct 5, 2017

@Mike111177 this is now released in yargs-parser@8.0.0 mind trying it out and making sure it works for you?

@Mike111177
Copy link
Contributor Author

@bcoe It now works as I was hoping, multiple space between arguments are now ignored and undefined are not inserted in the middle of the token array.

I did notice however that when the are spaces at the beginning of the argstring, it will still add undefined's to the beginning. I did not test it for endings, however it could be solved by doing a trim() on the argstring before tokenizing it.

@bcoe
Copy link
Member

bcoe commented Oct 6, 2017

@Mike111177 👍 I would happily accept a patch for the follow on .trim().

I have all of yargs' car parts spread out over the garage right now working on this, if you have any feedback on this front.

@Mike111177 Mike111177 deleted the fix-double-spaces branch October 7, 2017 20:57
@Mike111177 Mike111177 restored the fix-double-spaces branch October 7, 2017 20:58
@Mike111177 Mike111177 deleted the fix-double-spaces branch October 7, 2017 20:59
elsbree pushed a commit to elsbree/yargs-parser that referenced this pull request Dec 1, 2017
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.

None yet

2 participants