Allow setting the default command separately from the construction

[felds]

In a project I'm working on, the commands are added dynamically, like so:

const commandsDir = join(__dirname, "src/commands");
const commandFiles = readdirSync(commandsDir);
await Promise.all(
  commandFiles.map(async (file) => {
    const path = join(commandsDir, file);
    const { default: command } = await import(path);
    program.addCommand(command());
  }),
);

This makes pretty hard (or at least harder than it should be) to set the default command, as I would have to mess with the loading, mixing concerns.

const commandsDir = join(__dirname, "src/commands");
const commandFiles = readdirSync(commandsDir);
await Promise.all(
  commandFiles.map(async (file) => {
    const path = join(commandsDir, file);
    const command = (await import(path)).default();
    program.addCommand(command, {
      isDefault: command.name() === "list",
    });
  }),
);

This pull request implements the following interface:

program
  .description("Help text")
  .default("list");

To me, it looks more natural, since I think this configuration belongs on the parent command, not the command itself, as it is even coded like that internally.

[shadowspawn]

First thoughts: looks reasonable. Will be first time we modify behaviour referencing a command by name. I think default is a bit too simple as a method name (don't have a suggestion yet).

(This is the second mention in last week of loading commands from directory. That might be getting more common!)

[shadowspawn]

In case I am missing something, I don't think this line in the last example is needed or correct? .help("Help text")

[felds]

@shadowspawn

You're right about the .help call. It was supposed to be .description to exemplify the chaining. I already fixed it on the pull request.

I also thought so about the .default name. It fells too abstract, but I tried to follow the other setters.
.defaultCommand is clearer, but departs from the simplicity of .name, .version etc.

Whatever you folks choose is fine by me, and I can refactor the code to fit. :)

[shadowspawn]

I have mixed feelings about this. I do like it slightly better than the existing approach, but we do already have the existing approach!

Pros:

• allows separation of concerns, with defining/loading commands and setting default command

Cons:

• adds a second way of setting default command
• Commander API does not currently reference commands by name

Would you please open an issue suggesting adding this to see if gets some support as an enhancement? (And assuming does, we can reopen this.)

The code is simple, the naming and implications are the hard part! Playing with routine names. .defaultCommand(), or .setDefaultCommand(), or .defaultCommandName(). The first is the simplest but I fear potential for confusion over whether defining a command of that name. Compare with behaviour of .option() and .requiredOption().

[shadowspawn]

For interest, I looked for naming in other packages. yargs does this by using command name or alias of * (or $0). argparse does not appear to have direct support for a default command.

[felds]

@shadowspawn

A wildcard would introduce more complexity as we would have to implement a priority mechanism, only looking for wildcards when the exact name is not a match.

I would also argue that having a dedicated function (either on the child or the parent command) would make it clearer for a reader and more discoverable, due to autocomplete support.

[shadowspawn]

I saw setting default command separately mentioned in a by-the-way in another PR: #1186 (comment)