Problem when option is substring of other option

[LinusU]

I'm running into a little problem with one option that is an extension of the name of another option. I think this is best explained with an example:

Scandium

usage:
  scandium create [options]

options:
  --env=<key=value>...         Set environmental variables. Example: "--env NODE_ENV=production".
  --env-from-file=<path>       Read and set environmental variables from the specified file.

Trying to use as such:

scandium create --env TEST=yes --env-from-file stage.env

Results in:

{
  '--env': ['TEST=yes', '-from-file', 'stage.env']
}

When I expected it to parse into:

{
  '--env': ['TEST=yes'],
  '--env-from-file': 'stage.env'
}

[LinusU]

If anyone else runs into this, I'm currently using the following workaround 😂

const envFromFileIndex = process.argv.indexOf('--env-from-file', 2)

if (envFromFileIndex > 0 && envFromFileIndex < process.argv.length - 1) {
  process.argv.splice(envFromFileIndex, 1)
  process.argv[envFromFileIndex] = '--env-from-file=' + process.argv[envFromFileIndex]
}

[felixSchl]

Interesting :) Another less invasive workaround would be:

const args = neodoc.run(...)
if ((args['--env'] && args['--env'][0] == '-from-file') || args['--env-from-file']) {
  // ...
}

This works because if the user explicitly binds like this --env-from-file=foobar, then it will always assign to --env-from-file.

[felixSchl]

Does it seem reasonable to disallow this altogether? --env-x, so an argument cannot start with a - if it's not explicitly bound (i.e. via =). That would be a breaking change, but also make a lot of sense, to me at least.

[albinekb]

That won't work with them both at the same time though? 🤔 https://github.com/LinusU/scandium/blob/master/lib/tasks.js#L39-L45

Indeed that would be nice. It could still start with - if you have a space, bind it via = or wrap it in ""?

Is it currently supported to do --flagvalue without anything between the flag and value? 😱

[felixSchl]

Is it currently supported to do --flagvalue without anything between the flag and value? 😱

neodoc/src/Neodoc/ArgParser/Argument.purs

Lines 151 to 161 in af8ad85

	-- case 3:
	-- The name is a substring of the input and no explicit argument has been
	-- provdided.
	go (LOpt n' Nothing) _ | not isFlag
	= case stripPrefix (Pattern n) n' of
	Just s ->
	pure { rawValue: Just s
	, hasConsumedArg: false
	, explicitArg: false
	}
	_ -> fail "Invalid substring"

So, yes, that's the current behaviour and mirrors that of short options. What do you propose? From my perspective the easiest would be to remove the entire case above.

[albinekb]

That's neat. I've never used options like that, not that useful I think and it's also hard to read 🙂 Yes, I think it makes more sense to support more combinations of options rather than short options. @LinusU ?

Let's say you had a flag called --flag and then you typoed it to --flagg test, that would set g test to --flag? Better if that throws 👍

[felixSchl]

Let's say you had a flag called --flag and then you typoed it to --flagg test, that would set g test to --flag? Better if that throws 👍

Not entirely, you still need to specify the flag to take an argument. Also it would only consume a single argument, unless you specify that the argument is to be repeated:

usage: foo --flag
$ foo --flagd test
# Unknown option --flagd

usage: foo --flag=<arg>...
$ foo --flagd test
# flag → ["d","test"]

usage: foo --flag=<arg>
$ foo --flagd test
# Unknown command test

But regardless, I agree that this is counter-intuitive. I think removing that case would be a better user experience, but I'll have to do a major version bump. Will make the update shortly.

[albinekb]

Found another thing that is a bit wonky related to this.

We added an arg called --name-postfix, we also have --name.

 --name-postfix "--prod"

Will tell me there's no option called --name-postfix.

Using it with explicit binding works, --name-postfix=--prod

[felixSchl]

Are you able to share the usage string so I can see what's going on? While I am surprised about the error message you are getting, you won't get around having to use an explicit binding if the value looks like a flag.

[albinekb]

Ah, thats true 👍 I'll see
Here's the usage: https://github.com/LinusU/scandium/blob/master/app.js#L29

My bad, it actually said "option requires argument" 🙈 So that's correct

Shouldn't --name-postfix "--stage" work though? (maybe it does, haven't tested)

[felixSchl]

I've updated the code, it would be really helpful if you could lend a pair of eyes over the changes to the testcases, to make sure there's no oversight of anything: d6f2a0e. I've also published a prerelease version to npm for you to test out under neodoc@2.0.0-fix-94. I won't get around a major bump since it's not only changing the behaviour of library user's, but for users of library users.

[albinekb]

Awesome! I added a comment over there for the test cases I couldn't see. I'll try updating to that version and see if it works like expected, thanks!

[felixSchl]

Awesome, I've added a few more test cases as well, just to be sure

[albinekb]

❯ ./app.js create --env a=b d=e --env-from-file ./TEST
args { create: true,
  '--env': [ 'a=b', 'd=e' ],
  '--env-from-file': './TEST' }

neodoc@2.0.0-fix-94 fixes this issue correctly 👌 🙌

[felixSchl]

Ok cool, the new behaviour is now available in neodoc@2.0.0. Thank you for the report :)

[albinekb]

Thanks for fixing it 🙏 Love this lib!

[LinusU]

😍 wow

Awesome, to see this resolved so quickly, on vacation now but will take a look in a few days 🙌