node-dashdash icon indicating copy to clipboard operation
node-dashdash copied to clipboard

docs bug: unclear how to use env property for non-boolean arguments

Open ORESoftware opened this issue 7 years ago • 3 comments

Say we have this:

  {
    names: ['json'],
    type: 'bool',
    env: 'nlu_setting_json'
  },

it's clear that if we use:

export nlu_setting_json=1

but what if we have an array of string or something:

  {
    names: ['search-root', 'search'],
    type: 'arrayOfString',
    help: 'Path to use to begin searching for relevant NPM packages; overrides config. ' +
    'To add multiple search-root\'s, use "--search-root x --search-root y".'
  },

it's unclear to me how to use an env variable for that.

On another note, I don't see why

export nlu_setting_json=false

and this

export nlu_setting_json=0

are not equivalent, when it comes to dashdash converting them to booleans. 0 and false are both strings, so why does dashdash not interpret "false" as false, but it does interpret "0" as false?

ORESoftware avatar Aug 04 '18 04:08 ORESoftware

It would be neat if this convention could be followed:

console.log(JSON.parse('false') === false);
console.log(JSON.parse('1') === 1);
console.log(JSON.parse(2) === 2);
console.log(JSON.parse('true') === true);
console.log(JSON.parse(true) === true);
console.log(JSON.parse(false) === false);

so for env variables:

export nlu_setting_json=false

would then be equivalent to

export nlu_setting_json=0

because dashdash would do:

Boolean(0) === Boolean(JSON.parse('0'));
Boolean(false) === Boolean(JSON.parse('false'));

ORESoftware avatar Aug 04 '18 05:08 ORESoftware

@trentm bump please..I think it would help if it were documented how to use env variables for strings, arrayOfString, arrayOfBool, etc.

It also would be very nice if this worked for booleans:

export foo=true
export foo=false

right now only these work for booleans TMK:

export foo=1
export foo=0

the1mills avatar Aug 31 '18 08:08 the1mills

I think the docs are quite clear:

Boolean options will interpret the empty string as unset, '0' as false and anything else as true.

Although I agree it might be unexpected, changing this would be a breaking change. it could be worth wile to print a warning to stderr that the value 'false' will be treated as true.

Sadly this issue doesn't really allow people to find it easily since it addresses multiple things of which one of them, are already addressed in the docs at least to some degree.

karfau avatar Jan 31 '20 17:01 karfau