yargs / yargs-parser

:muscle: the mighty option parser used by yargs
http://yargs.js.org/
ISC License
493 stars 118 forks source link

fix: parser should preserve inner quotes #407

Closed jly36963 closed 2 years ago

jly36963 commented 3 years ago

Addresses: https://github.com/yargs/yargs/issues/1324

Problem

Inner quotes are handled differently by parse, depending on the argsInput argument's type.

tokenizeArgString wraps argString with extra quotes if it is a string. tokenizeArgString does not do the same for argString, if it is an array. The first code block in processValue removes quotes from the value.

TLDR

If parse was given a string, extra quotes are added (tokenizeArgString) and then removed (processValue). If parse was given an array, quotes are removed (processValue). This causes loss of inner quotes.

Example (using tests)

logs for string with inner quotes

{ argsInput: 'cmd --foo ""Hello"" --bar ""World"" --baz "":)""' }

during tokenizeArgString 
{ 
  argString: 'cmd --foo ""Hello"" --bar ""World"" --baz "":)""' }
  args: [  'cmd',  '--foo',  '""Hello""',  '--bar',  '""World""',  '--baz', '"":)""'  ]
}

after processValue 
{
  argv: [Object: null prototype] {
    _: [ 'cmd' ],
    foo: '"Hello"',
    bar: '"World"',
    baz: '":)"'
  }
}

result
{ args: { _: [ 'cmd' ], foo: '"Hello"', bar: '"World"', baz: '":)"' } }

logs for array with inner quotes

{ argsInput: [  'cmd',   '--foo',  '"Hello"', '--bar',  '"World"',   '--baz',   '":)"'  ] }

during tokenizeArgString 
{
  argString: [  'cmd',   '--foo',  '"Hello"', '--bar',  '"World"', '--baz',  '":)"'  ]
  args: [  'cmd',  '--foo',  '"Hello"', '--bar', '"World"', '--baz',  '":)"'  ]
}

after processValue
{
  argv: [Object: null prototype] {
    _: [ 'cmd' ],
    foo: 'Hello',
    bar: 'World',
    baz: ':)' // *** inner quotes missing ***
  }
}
result 
{ args: { _: [ 'cmd' ], foo: 'Hello', bar: 'World', baz: ':)' } }

Solution

I added conditional logic around removing quotes. I feel like my current solution is messy. I would love to hear your thoughts/advice on this. I know you're busy, so no worries if you don't have time. I will try to find a better solution.

There might be follow up work in yargs for handling positionals. The parsed object that gets returned by yargs-parser still has the positional arguments in argv._,

const parsed = this.#shim.Parser.detailed(...)

bcoe commented 2 years ago

@jly36963 thanks for the patch 👌