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

Cannot use the same @Options name for command and SubCommand #663

Closed
3 of 7 tasks
jubeal opened this issue Nov 23, 2022 · 4 comments · Fixed by #707
Closed
3 of 7 tasks

Cannot use the same @Options name for command and SubCommand #663

jubeal opened this issue Nov 23, 2022 · 4 comments · Fixed by #707
Labels
bug Something isn't working

Comments

@jubeal
Copy link

jubeal commented Nov 23, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

The issue manifest while trying to reach the SubCommand of a Command.

If both have the same @option name like --message then the SubCommand will never get the option in its code.

Is it a wanted behavior ?

Minimum reproduction code

The main command:

@Command({
  name: 'say',
  subCommands: [AppSubCommandNotWorking],
  description: 'Tells you what you need to hear',
})
export class AppCommand extends CommandRunner {
  constructor(private readonly appService: AppService) {
    super();
  }

  async run(
    _PassedParams: string[],
    options: AppCommandOptions,
  ): Promise<void> {
    const { message } = options;

    console.log('Command');

    console.log(this.appService.getMessage(message));
  }

  @Option({
    flags: '-m --message [string]',
    description: 'What you need to hear',
    required: false,
  })
  ParseMessage(val: string): string {
    return val.trim();
  }
}

The SubCommand:

@SubCommand({
  name: 'yellow',
  description: 'Tells you what you need to hear but in Yellow',
})
export class AppSubCommandNotWorking extends CommandRunner {
  constructor(private readonly appService: AppService) {
    super();
  }

  async run(
    _PassedParams: string[],
    options: AppCommandOptions,
  ): Promise<void> {
    const { message } = options;

    console.log('SubCommandNotWorking');

    console.log('\x1b[33m%s\x1b[0m', this.appService.getMessage(message));
  }

  @Option({
    flags: '-m --message [string]',
    description: 'What you need to hear',
    required: false,
  })
  ParseMessage(val: string): string {
    return val.trim();
  }
}

Expected behavior

expected output:

You really want me to say that: <CONTENT_OF_MESSAGE> ? wrote in Yellow.

real output:

You really want me to say that: undefined ? wrote in Yellow

Package

  • nest-commander
  • nest-commander-schematics
  • nest-commander-testing

Package version

3.3.0

Node.js version

18.8.0

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

exemple repository for you to reproduce: https://github.com/jubeal/nestBugExemple

@jubeal jubeal added the bug Something isn't working label Nov 23, 2022
@jmcdo29
Copy link
Owner

jmcdo29 commented Nov 23, 2022

Looks like this is a feature of how commander itself works. I created the following test files to make sure

index.js

File

const Commander = require("commander");

const subCommand = Commander.createCommand("yellow");
subCommand.addOption(new Commander.Option("-m, --messsage [message]"));
subCommand.action((args) => {
  console.log("Yellow Subcommand");
  console.log({ args });
  console.log({ opts: subCommand.opts() });
});

const sayHello = Commander.createCommand("say");
sayHello.addOption(new Commander.Option("-m, --message [message]"));
sayHello.addCommand(subCommand);
sayHello.action((args) => {
  console.log("Main command");
  console.log({ args });
  console.log({ opts: sayHello.opts() });
});

Commander.program.addCommand(sayHello);
Commander.program.parse();

Main Input

node index.js say -m hello

Main Output

Main command
{ args: { message: 'hello' } }
{ opts: { message: 'hello' } }

Yellow input

node index.js say yellow -m hello

Yellow Output

Yellow Subcommand
{ args: {} }
{ opts: {} }
main.js

File

const Commander = require("commander");

const subCommand = Commander.createCommand("yellow");
subCommand.addOption(new Commander.Option("-y, --messsage-yellow [message]"));
subCommand.action((args) => {
  console.log("Yellow Subcommand");
  console.log({ args });
  console.log({ opts: subCommand.opts() });
});

const sayHello = Commander.createCommand("say");
sayHello.addOption(new Commander.Option("-m, --message [message]"));
sayHello.addCommand(subCommand);
sayHello.action((args) => {
  console.log("Main command");
  console.log({ args });
  console.log({ opts: sayHello.opts() });
});

Commander.program.addCommand(sayHello);
Commander.program.parse();

Main Input

node main.js say -m hello

Main Output

Main command
{ args: { message: 'hello' } }
{ opts: { message: 'hello' } }

Yellow Input

node main.js say yellow -y hello

Yellow Output

Yellow Subcommand
{ args: { messsageYellow: 'hello' } }
{ opts: { messsageYellow: 'hello' } }

If you think this isn't the right logic here, we should probably report this up the chain to comamnder and see what they think

I ended up creating an issue in commander's repository. Let's see what they say

@smolinari
Copy link
Contributor

👀 - Commenting to follow.

Scott

@jubeal
Copy link
Author

jubeal commented Nov 24, 2022

While reading the answers on the issue you created on commander repo I was thinking:

  • Is .enablePositionalOptions() working for the problem we have ? And is it implementable in nest-commander ?

  • Is it possible to implement default subCommands ? Like you have a primary Command which has a bunch of subCommands and one of them is the default. And if you call the primary Command it leads you to the default subCommand automatically.
    For now the primary Command execute its run function if we call it without a specified subCommand. And we cannot have a primary Command without a run function.

@jmcdo29
Copy link
Owner

jmcdo29 commented Dec 24, 2022

Sorry for the delay on getting back to this! Been a very busy month!

Is .enablePositionalOptions()` working for the problem we have ? And is it implementable in nest-commander ?

Yes, but it is an option we have to set across the entire commander instance, which could lead to some unexpected side effects of stricter options parsing. We'd probably want to make this an option that is passed via CommandFactory.run() so it's very clear that it effects everything and not just one command

Is it possible to implement default subCommands ?

I haven't looked into this yet. Let me get back to you on it. I could see it being useful. Something I might be able to do is add a new options for @SubCommand() that would override the implementation of the parent's run method to be the sub command's run instead. That could possibly end up being an issue though if class specific methods get involved so it may not be a good idea. I'll keep thinking on it and doing some research around if it's possible.

Actually, looks like yes it should be possible. I'll double check my implementation and see if the values are getting passed as expected.

Okay, looks like something I'll need to add implementation for. Would be possible though. I could probably get both of these added in a PR by EOY if they sound like something that would be useful for the library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants