mow.cli icon indicating copy to clipboard operation
mow.cli copied to clipboard

Possible bug with option specs

Open skyzyx opened this issue 3 years ago • 2 comments

I've discovered something which violates the Principle of Least Surprise and think that it may be a bug.

I have a spec which looks like this:

cmd.Spec = `[--cluster=<value> [--only-env=<value>] | --all]`

When I run the command with an option, I get the following:

$ mkit module newrelic plan --all
Error: incorrect usage

Usage: mkit module newrelic plan [--cluster=<value> [--only-env=<value>] | --all]

Executes `terraform plan` for the specified Terraform module.

Options:
  -c, --cluster    The friendly name of the cluster that you want to execute against. (Not compatible with --all.)
  -e, --only-env   Used for limiting the execution to a single environment. (Not compatible with --all; requires --cluster.)
  -a, --all        Execute for all clusters at once. (Not compatible with --cluster.)

However, if I reverse the top level options in a way that is semantically identical:

cmd.Spec = `[--all | --cluster=<value> [--only-env=<value>]]`

…it works without issue.

I've read through the text about this in the README, and I didn't find this specifically called-out. As a result, I think it's a parser issue. It's taking position into account in a place where it shouldn't.

skyzyx avatar Feb 16 '21 23:02 skyzyx

@skyzyx Here's how mow.cli parses the spec string [--cluster=<value> [--only-env=<value>] | --all]:

Screenshot 2021-02-19 at 09 27 44

However,by adding parenthesis to get this spec string: [(--cluster=<value> [--only-env=<value>]) | --all], this nudges the parser into what you want:

Screenshot 2021-02-19 at 09 29 18

You are right that the doc does not explain precedence/associativity rules, which needs to be fixed.

jawher avatar Feb 19 '21 08:02 jawher

Ahhh. I had assumed that the left side of | was auto-grouped.

[ natural-group-1 | natural-group-2 ]

skyzyx avatar Feb 19 '21 20:02 skyzyx