core icon indicating copy to clipboard operation
core copied to clipboard

argument not read, if after a flag with multiple: true

Open shazron opened this issue 3 years ago • 1 comments
trafficstars

Describe the bug I have a command that has 1 arg, and 1 multiple flag. If I specify the arg after the flag, I cannot access the arg in my command.

To Reproduce Steps to reproduce the behavior:

const { Command, Flags } = require('@oclif/core')

class FlagsTestCommand extends Command {
  async run () {
    const { args, flags } = await this.parse(FlagsTestCommand)

    this.log('args.path', args.path)
    this.log('flags.myflags', flags.myflags)
  }
}
FlagsTestCommand.description = `issue 496 in oclif/core`

FlagsTestCommand.flags = {
  myflags: Flags.string({
    description: 'my flag for testing',
    char: 'm',
    multiple: true
  })
}

FlagsTestCommand.args = [
  {
    name: 'path',
    description: 'Path to the test directory',
    default: '.'
  }
]

module.exports = FlagsTestCommand

run:

  1. oclif FlagsTest -m=abc123 Foobar (no args parsed)
  2. oclif FlagsTest -m abc123 Foobar (no args parsed)
  3. oclif FlagsTest Foobar -m=abc123 (arg is parsed since it occurs before the flag)

Expected behavior I can access my arg if specified after the multiple flag.

Actual behavior I cannot access my arg if specified after the multiple flag.

Workaround Specify the arg before the multiple flag.

Other context AFAIK you can put args before flags, or after. An argument is anything to the right of a command that is not a flag. An argument can come before or after flags. from https://oclif.io/blog/2019/02/20/cli-flags-explained

shazron avatar Sep 22 '22 04:09 shazron

@shazron Thanks for posting the issue. I validated that this is in fact a bug. However, the fix will require that we make a breaking change to the flag parser.

Right now both of the following are valid ways of providing multiple flag values:

  • my-command --multi-flag 1 --multi-flag 2 --multi-flag 3
  • my-command --multi-flag 1 2 3

Supporting the second way is what's causing this issue. The flag parser will assume that each subsequent value is part of the flag until the next flag is found. So in your case, it thinks that the argument is part of the value for the multiple flag.

To fix it, we'd have to drop support for that second method, which would be a breaking change.

I've labeled this as will-fix-in-v2 so that we can pick it up once we get started on the next major version. Until then, you'll have to provide the argument first before providing the values for the multiple flag.

mdonnalley avatar Sep 22 '22 15:09 mdonnalley

@shazron After more deliberation, we decided that this isn't something we're able to fix because it would eliminate support for separating values by spaces, which is widely used by our CLI and others.

Our recommendation is to avoid mixing multiple flags and arguments in your commands.

However, we're open to the idea of emitting some sort of warning to the user that directs them to put the argument before any flags, which would avoid the issue altogether. Let us know if you're interested in that solution.

mdonnalley avatar Jan 10 '23 21:01 mdonnalley

@shazron You can use -- after the multiple (or any other) flag to ensure that the following values are parsed as args and not as values.

oclif FlagsTest -m abc123 abc456 -- Foobar

@mdonnalley FYI.

jayree avatar Jan 10 '23 23:01 jayree

@mdonnalley a warning would be great 🙏 (almost like a runtime linter, where React warns you from doing certain things) so a dev won't be scratching their head for potentially hours wondering what was wrong

@jayree thanks! I'll add that to our docs regarding a workaround

shazron avatar Feb 22 '23 02:02 shazron