tj / commander.js

node.js command-line interfaces made easy
MIT License
26.75k stars 1.69k forks source link

Commands cannot be reused #2246

Closed gaohoward closed 4 weeks ago

gaohoward commented 2 months ago

Hi I'm trying to create a cli app (interactive) that uses commands to perform tasks. For each task I create a command instance. For example I provide a get command for user to use.

const getCmd = program.command('get')
.description('get information from a endpoint')
.argument('<path>', 'path of the component, [endpointName/componentType]')
.argument('[compName]', 'name of the component', '')
.option('-a, --attributes [attributeNames...]', 'get attributes from component', '')
.action(async (path, compName, options, cmd) => {
...
});

In the interactive shell (I use readline lib) if user input get args opts the getCmd will called getCmd.parseAsync(allargs [])

then after the command is executed, the shell waiting for next command, if user inputs the same get command but with some options removed, when it calls the getCmd.parseAsync(allargs) again I found some of the options from the first execution remains.

So basically I assume each time I have to create a new instance of the get command. I wonder if there is a way to reset the command for reuse?

gaohoward commented 2 months ago

looks like I can use my own options parser function... but still this is something can be improved.

shadowspawn commented 2 months ago

So basically I assume each time I have to create a new instance of the get command.

Yes, that is the intended approach.

See #2036 for some reference to past issues, and adding an explicit mention to the README.

shadowspawn commented 4 weeks ago

An answer was provided, and no further activity in a month. Closing this as resolved.

Feel free to open a new issue if it comes up again, with new information and renewed interest.

(I am wondering whether Commander should display an error on a second call to parse, but that would be a very breaking change. I will continue to monitor issues where people trip over this.)

pillowfication commented 3 weeks ago

I just ran into this issue, and it was very annoying to debug. I realize now that it's mentioned in the README, but I glossed over it and expected parse()/parseAsync() to simply be able to be called multiple times, which seemed to work just fine until I noticed options mysteriously being remembered from previous calls.

Throwing an error on a second call to parse() would be great. It seems that the README states that parse() shouldn't be called multiple times, and I'm not sure there is a great use case for its current behavior. So it seems that multiple parse() calls isn't supported at all currently and throwing an error wouldn't be a breaking change.

shadowspawn commented 3 weeks ago

For interest @pillowfication , why were you calling parse multiple times? Was it unit tests, or a long running process using same program multiple times, or something else?

pillowfication commented 3 weeks ago

For interest @pillowfication , why were you calling parse multiple times? Was it unit tests, or a long running process using same program multiple times, or something else?

I'm developing a CLI where I will be prompting the user multiple times and passing input into Commander. This is because my CLI is stateful, and commands can affect future commands.

Example:

> say-hi
Hello, stranger!
> set-name pillowfication
> say-hi
Hello, pillowfication!

The general structure of the program looks something like this:

import { Command as CommandBase, CommanderError } from 'commander'

class Command extends CommandBase {
  constructor (...args: ConstructorParameters<typeof CommandBase>) {
    super(...args)
    this.exitOverride() // important that all commands don't exit
  }
}

class Cli {
  #rootCommand: Command
  #needsReset = false

  constructor () {
    this.#rootCommand = this.#createRootCommand()
  }

  #createRootCommand (): Command {
    // Create root command and all sub-commands here
    return new Command('my-program')
  }

  async execute (input: string): Promise<void> {
    try {
      if (this.#needsReset) {
        this.#rootCommand = this.#createRootCommand()
      }
      await this.#rootCommand.parseAsync(parseArgs(input), { from: 'user' })
    } catch (error) {
      if (error instanceof CommanderError) {
        // Commander already displays its own errors
      } else {
        console.error(error)
      }
    } finally {
      this.#needsReset = true
    }
  }
}

// Main program body
;(async () => {
  const cli = new Cli()

  while (true) {
    const input = await prompt('> ')
    await cli.execute(input)
  }
})()
shadowspawn commented 3 weeks ago

Thanks for explanation and detail @pillowfication