thenativeweb / command-line-interface Goto Github PK

Problem

When using getUsage within a handle-Function, I get the following error:

TypeError: Cannot read property 'map' of undefined
    at Object.getCommandsByPath (/Users/davidlosert/Projects/yabai-configurator/node_modules/command-line-interface/build/lib/getCommandsByPath.js:6:22)
    at Object.getCommandLineUsageConfiguration (/Users/davidlosert/Projects/yabai-configurator/node_modules/command-line-interface/build/lib/usage/getCommandLineUsageConfiguration.js:12:48)

this is my handle-Function:

  async handle({ options, getUsage, ancestors }) {
    const subCommandName = await buntstift.select('Select Subcommand', Object.values(this.subcommands).map(command => command.name));

    if (subCommandName === 'help') {
      buntstift.info(getUsage([...ancestors, 'yco']));
      process.exit(0);
    }

    await runCli({ rootCommand: this.subcommands[subCommandName], argv: process.argv });
  }

Cause

I investigated the issue and found the reason:
The getUsage-Function (defined within getGetUsage) actually expects an object with the property commandPath rather than an array (as documented):

const getGetUsage = function ({ rootCommand }: {
  rootCommand: Command<any>;
}): GetUsageFn {
  return ({ commandPath }): string => {
    const commandLineUsageConfiguration = getCommandLineUsageConfiguration({ rootCommand, commandPath });

    return commandLineUsage(commandLineUsageConfiguration);
  };
};

As I am not sure which is supposed to be right (the documented way or the programmed), I did not send a PR yet.
But I am happy to once you tell me which I should fix. 😃

When using a command option with multiple: 'on' and also define subcommands, there is ambiguity.
Consider the following example, where we've got some tool which has some option -e that can be repeated and also a subcommand sub.

some-tool -e argsAreFun suchANiceArg sub

In this example, sub could either refer to a value for -e, or be the name of a subcommand.
The current behavior is to consume as many arguments as possible before the next flag is matched.

@dotKuro and I had a discussion about how this could be resolved and we currently see two options:

1. Do away with the current syntax and use the flag repeatedly

With this, the new syntax would be:

some-tool -e argsAreFun -e suchANiceArg sub

This way, each occurence of a flag only consumes exactly one argument. I believe this is also how many shell-based tools handle this.

2. Implement non-deterministic parsing

We could also, whenever there is ambiguity, try to construct a valid command from all possible interpretations and see if any one of those actually creates a valid command. This could also lead us to situations, where more than one interpretation is valid. In this case we'd have to pick one of them, hence this being the non-deterministic option.

Also, for commands without subcommands, we could keep the old syntax, as there is no ambiguity there.

If there is an option missing, option unknown, or option invalid error, how can I print command-line-interface's well-generated default usage text?

Based on the prototypes of the handlers for those errors, I think it's impossible to use the default usage text.

Currently all options are optional, whether marked as such or not. In the synopsis marked as optional (by the square brackets) are all options that have a default value. If an option without a default value is not provided by a user, the value will be undefined. This might not reflect what developers think or even what they write in their interface for the options.

This is neither useful to developers nor helpful to users of cli applications.

I propose that we enforce the marked optional options by displaying an error if one of them is not provided. Then, when no default value is set for an option, it can be assumed to be present in the options. This way we also need to make no changes to the way the synopsis is displayed.

The readme currently uses the terms option, parameter and flag inconsistently. This needs to be adjusted to adhere to the following definitions:

• An option is anything passed to a command. In nifty-tool --color red, color is an option. This is reflected in the code base, since commands have an array of OptionDefinitions.
• A flag is an option of type boolean. In nifty-tool --verbose, verbose is an option and a flag. This terms is correct and has a clear definition, but should be used sparingly.
• A parameter is a value given to an option. In nifty-tool --color red, red is the parameter given to the option color. This is reflected in the code base, since OptionDefinition contains the field parameterName, which specifies the name of the parameter in a command's synopsis.

Corollary:

• flags can not have a parameter.
• All options that are not flags have a parameter, even if it is omitted on the CLI and specified via a defaultValue.

Currently options are not validated based on the type that is configured for them.

command-line-args, the package we use under the hood, just calls converter functions. So when entering a string for a number-option, it just comes back as NaN.

We need to expand on this and do some default validation.

As emerged from discussion in #245, if a non-leaf command's option has defaultOption set to true, the values given for this option might conflict with sub-commands. Take for example this CLI invocation:

$ nifty-tool --color red random-quote
# If --color has defaultOption set to true, this might be written as
$ nifty-tool red random-quote

If nifty-tool now had a sub-command called red, the above would be ambiguous.

In #245 we discussed a solution already and concluded that it would be best to only allow defaultOption: true in leaf-commands, not in intermediary ones.

1. Try to integrate this limitation into the type system. This will likely not be possible.
2. Alternatively add runtime validation for options that crash a CLI if a non-leaf command has an option with defaultOption set to true
3. Adjust tests and examples to account for this
4. Update readme with this limitation