Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non positional arguments of main command usage does not allow to use any other commands #133

Open
1 task done
sausir1 opened this issue Apr 5, 2024 · 1 comment
Open
1 task done

Comments

@sausir1
Copy link

sausir1 commented Apr 5, 2024

Describe the feature

Did not exactly knew is this a feature to implement, or a bug to fix, but marked as a feature.

Description

Seems like this part of the program could be improved - if main command have args and some sub-commands, citty fails to execute the underlying commands when any argument is passed. To better understand what I am trying to say is:

  1. I have a command that both have args that it accepts, and sub-commands that it can launch:
const subCommand = defineCommand({
  meta: {
    name: "subcommand",
    version: "0.0.1",
  },
  args: {
    world: {
      type: "string",
      required: true,
    },
  },
  run(context) {
    console.log("sub: citizen of planet %s!", context.args.world);
  },
});

const main = defineCommand({
  meta: {
    name: "play",
  },
  args: {
    name: {
      type: "string",
      required: false,
    },
  },
  setup(context) {
    console.log("main: hello %s", context.args?.name);
  },
  subCommands: {
    subCommand,
  },
});

If I want to launch the subCommand I would do:

$ ./example.js subCommand --world "earth"

this works fine, but if i would like to launch this:

$ ./example.js --name "Citty" subCommand --world "earth"

This would fail with the error message, "Unknown command Citty".

I think this can be easily fixed by changing the logic where the next command's name is being parsed in command.ts file's runCommand method.

   ...
      const subCommandArgIndex = opts.rawArgs.findIndex(
        (arg) => !arg.startsWith("-"),
      );
  ....

I came up with this solution for getting the next command's index:

const subCommandArgIndex = opts.rawArgs.findIndex((arg, index) => {
        const currentIsNotArg = !arg.startsWith("-"); // checks that the current arg is not a argument
        if (index === 0) {
          return currentIsNotArg;
        }
        const prevArg = opts.rawArgs[index - 1];
        // if current `arg` is not an argument, then we need to check if this is a value of a previous `arg` argument or not.
        return (
          currentIsNotArg && !prevArg.startsWith("-") && arg in subCommands
        );
      });

P.S. There's still a caveat left here - if positional argument's value matches the command's name, command might get called with bad parameters.

It also seems that positional argument with default value followed by a sub-command is also not working. Is this the desired behavior? If so, maybe ArgDef type should be extended, to always require positional arguments? Or it this too much of an edge case?

Additional information

  • Would you be willing to help implement this feature?
@sausir1
Copy link
Author

sausir1 commented Apr 5, 2024

I've checked with all types of allowed argument types to date, seems like these changes addresses the issue, except that one with positional arguments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant