yargs / yargs/yargs-parser

Hopefully ending any and all discussion about how to handle quotes....

Open
#300 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

feature
Dominant language
JavaScript
Stars
519
Forks
129
Avg merge
1h 34m
Merged PRs (30d)
1

Description

So whether or not to remove quotes and where seems to have been plaguing this repo with endless issues and PRs.
Sorry to wake up the dead here but see #145, #82, #93, #140, #138, #153, #180, #201, #222 and probably more I didn't find.

Overall the goal of this issue, discussion, and potential future PR is to making typing a command into your favorite shell or into your online bot interface more consistent, and also to make quote handling easier to test and more predictable.

I have two examples that well demonstrate what I think the main issues currently are:

I loaded this test.js into the node REPL before both of these examples to set up the command line and tokenizers.

//To grab arguments as passed by a shell if we recieve args
if (process.argv.length>2){
  console.log(JSON.stringify(process.argv.slice(2)))
  process.exit()
}

const fs = require('fs');
const {execSync} = require('child_process')
const util = require('util')
const parse = require('yargs-parser')

//Tokenizers
function extTokenizer(exe, ext, prefix=""){
  return command=>{
    tkfile = `tokenize.${ext}`
    fs.writeFileSync(tkfile, `${prefix}node test.js ${command}`)
    result = execSync(`${exe}${tkfile}`);
    return JSON.parse(result)
  }
}
const basicTokenizer = str=>str.split(" ") //Simple whitespace split
const yargsTokenizer = require('./tokenize-arg-string')
const cmdTokenizer = extTokenizer('cmd /C ', 'bat', "@echo off\n")
const powershellTokenizer = extTokenizer('powershell -ExecutionPolicy Bypass -File ', 'ps1')
const bashTokenizer = extTokenizer('bash ', 'sh', "/home/mike/.nvm/versions/node/v14.8.0/bin/") //Tested with WSL

const argfile = "command.txt"
const commandline = fs.readFileSync(argfile).toString()

Here I created a gist containing a quick and dirty tool for trying this out for yourself

1. Inconsistent removal of quotes depending on context of token

Post tokenizer, the argument parser seems to pick and choose which arguments aught to be stripped of quotes.

> commandline
`"positional" -a 'flag'`
> tokens = yargsTokenizer(commandline)
[ '"positional"', '-a', "'flag'" ]
> parse(tokens)
{ _: [ '"positional"' ], a: 'flag' }

Notice How the flag argument gets its quotes removed, but the positional argument does not. This inconsistency is confusing.

Now you may ask "then how do I enter a command like domything "to this other thing" from a string statement. Well that brings me to problem 2.

2. Tokenizer inconsistency with shell interfaces.

So if tokenizeArgString is supposed to be the middleware that converts your text string to the argument (token) array in the case where the shell you are in hasn't done it for you, arguably the output of tokenizeArgString should emulate the same expected results as you would get by reading process.argv. Since process.argv is controlled by the various shells, lets see how tokenArgString stacks up against the most popular ones (that I could easily test).

> commandline
`'single quote' "double quote" \\"escaped quote\\"`
> yargsTokenizer(commandline)
[ "'single quote'", '"double quote"', '\\"escaped quote\\"' ]
> cmdTokenizer(commandline)
[ "'single", "quote'", 'double quote', '"escaped', 'quote"' ]
> powershellTokenizer(commandline)
[ 'single quote', 'double quote', '\\escaped quote"' ]
> bashTokenizer(commandline)
[ 'single quote', 'double quote', '"escaped', 'quote"' ]

There are a few observations to be made here:

  • yargs nicely handles both single quotes and double quotes
  • cmd does not support single quotes, however it supports " escapes and strips the double quotes
  • powershell is a bit weirder, it supports both single and double quotes and strips both, but its behavior is strange when using \ escapes... (it also supports using backticks as escapes, or double quote marks to escape)
  • bash where everything is nice and makes sense supports single and double quotes and \ escapes

Which one of these methods is correct? None of them, they are all different flavors on the same end goal. However people are used to them and generally expect similar behavior (whether or not they get it) and more importantly, they are testable.

My Proposal

  1. Do not strip quotes from arguments passed as an array, ever. Assume they have already been tokenized by a shell or tokenizer. I would argue that parse with an argument of any[] should leave every single argument unmolested as it should assume that it is receiving the arguments directly from process.argv. Otherwise it is difficult to figure out when and where quotations need to be added or removed. Therefore it should assume that the inputted quotes are intended to remain. This is probably a small breaking change, you could probably fix this with a deprecated option as well, but in my opinion maintaining both modes seems like a bit of a pain.

    In the case where the user has specified in options that they want quotes removed the question becomes how many times to you strip the quotes, this needs further discussion as this seems to be a post processing measure.

  2. Have the tokenizer handle removing quotes in accordance with how users would expect it from their shell. That means supporting three different tokenizers that are flavored like cmd, powershell, and bash. This way not only can the programmer select which style they want, it also means that there is no longer a "but i want my quotes removed here but not here" issue because you can respond, "you selected the powershell style and thats how powershell does it", removing any opinion on how the quotes are removed and ending this plague of issues.

    You also get the benefit of being able to unit test directly against the shells themselves. Im not totally confident that the exact method to test them I wrote above is very CI friendly, however a similar concept could be used. Just take a bunch of predefined argument strings and see if the relate shell has the same output as what the flavored tokenizer outputs.

    You could layout the top level API like the following:

//If you know you are tokenizing, don't accept tokens
export type Tokenizer = (arg: string) => string[];
export type CommandLineParser = (arg: string, opts?: Partial<Options>) =>Arguments;

//Same stuff as before for backwards compatibility
export interface Parser {
  (args: ArgsInput, opts?: Partial<Options>): Arguments; //Will now by default use tokenize.default to tokenize
  detailed(args: ArgsInput, opts?: Partial<Options>): DetailedArguments;
  camelCase(str: string): string;
  decamelize(str: string, joinString?: string): string;
  //new stuff
  //included tokenizers
  tokenize {
       (arg: string) => string[]; //Calling require('yargs-parser').tokenize("") should just use the default tokenizer
       default: Tokenizer; //freeze the current tokenizer for backward compat
       cmd: Tokenizer;
       powershell: Tokenizer;
       bash: Tokenizer;
  };
  //Include tokenizer flavors as full parsers
  cmd: CommandLineParser;
  powershell: CommandLineParser;
  bash: CommandLineParser;
  //Allow programmers to feed it their own tokenizer? So less complaining about format
  parser: (tokenizer: Tokenizer) => CommandLineParser;
}

I understand this goes somewhat against previous efforts (See #153), but ultimatley the tokenizer is meant to take the place of a shell that does it for you, so it doesn't make sense not to try to emulate the function of the shell in this regard.

This can also maybe open up an opportunity to seperate the tokenizers into their own npm package. It would be neat to have a package that is just designed to emulate how shells tokenize to argv. yargs-tokenizer anybody?

Let me know what yalls thoughts are on this, I'd be willing to do some of the ground work for this. I also believe that most (not all) of this can be introduced without any breaking changes...

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with tokenize-arg-string and the parser entry point, using the test.js shell experiments and the linked examples to compare tokenization behavior. Done requires an agreed quote-handling design, tokenizer behavior for the discussed shells, and tests that verify the selected behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
cli
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.