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

yargs not passing string correctly #93

Closed
hisabimbola opened this issue Jul 13, 2017 · 7 comments
Closed

yargs not passing string correctly #93

hisabimbola opened this issue Jul 13, 2017 · 7 comments

Comments

@hisabimbola
Copy link

describe('TokenizeArgString', function () {
  it.only('....handles quoted string', function () {
    var args = tokenizeArgString('-q sku="invalid"')
    args[0].should.equal('-q') // passes
    args[1].should.equal('sku="invalid"') //fails
  })
})

This test fails on this module. Not sure how to fix it.

@bcoe
Copy link
Member

bcoe commented Aug 17, 2017

@hisabimbola good catch; looks like somewhere in the parsing process we drop the ", this is definitely a bug. If you get a chance to dig into this yourself, would happily accept a patch. Either way, will definitely try to get this fixed soon.

@bcoe
Copy link
Member

bcoe commented Sep 3, 2017

@hisabimbola @antoniom unfortunately, I thought of a couple edge-cases where the logic for handling quoted strings would break the parse. Specifically if you had a boolean argument configured, followed by a string:

-f "hello world" in this scenario, if -f is configured as a boolean, you would end up with "hello world" in your _ -- which is unexpected behavior and would put the burden of more parsing on the consumer.

I think we should leave this behavior as is, i.e., eating ' and " -- this is the default behavior of the operating system:

 node test.js -f "world-hello"

will result in the following process.argv:

[ '/Users/benjamincoe/.nvm/versions/node/v7.1.0/bin/node',
  '/Users/benjamincoe/bcoe/yargs/test.js',
  '-f',
  'world-hello' ]

so even if we do add this parsing feature to yargs-parser the operating system will eat the quotes before they get to the parser.

perhaps start a conversation in our community slack and we can discuss other potential solutions to the problem -- perhaps you could JSON encode the data being passed in?

@antoniom
Copy link
Contributor

antoniom commented Sep 5, 2017

@bcoe Since this is the default behavior from OS side, its OK for me. Feel free to revert it.

@ilatypov
Copy link

ilatypov commented Feb 28, 2018

Passing an undefined variable as args to the parser (which happens a mistake in the parser's caller) throws a cryptic message, Should this create another issue?

TypeError: Cannot read property 'match' of undefined

@bcoe
Copy link
Member

bcoe commented Mar 4, 2018

@ilatypov 👍 yeah, would appreciate it if you create an issue specifically related to this.

@malhotraashna
Copy link

malhotraashna commented Nov 30, 2018

@bcoe Can I please ask you to again have a look at the case for allowing strings with quotes to be passed? It becomes very important to have it when string with spaces need to be passed.

Consider this scenario
./nodeFile.js -n "my node module"

If I don't use double-quotes with my string, then the OS itself will consider the first word as the argument value. But it doesn't strips them out itself. This comment explains it better.

I completely understand your concern when random string is passed after boolean arguments. But I guess passing string with spaces is a very common case too. And if possible, if the option has type specified as string, at least allow the double-quotes then?

@bcoe
Copy link
Member

bcoe commented Dec 25, 2018

@malhotraashna please open a new issue with an example of a reproduction in test if possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants