Add missing check for broken passThrough

[aweebit]

Problem

const { Command } = require('commander');

try {
  new Command('parent')
    .command('sub')
    .passThroughOptions(); // not allowed without enablePositionalOptions for parent
} catch (err) {
  console.error(err);
}

try {
  const parent = new Command('parent');
  const sub = new Command('sub')
    .passThroughOptions();
  parent.addCommand(sub); // allowed without enablePositionalOptions for parent
} catch (err) {
  console.error(err);
}

A check for this case is considered unnecessary, see comment

try {
  const parent = new Command('parent')
    .enablePositionalOptions();
  parent.command('sub')
    .passThroughOptions();
  parent.enablePositionalOptions(false); // allowed despite passThroughOptions for sub
} catch (err) {
  console.error(err);
}

Solution

Throw errors in all demonstrated cases. Additionally make the error message more meaningful by including the command name (similar to #1923 and #1924).

ChangeLog

Changed

• Breaking: throw when a subcommand with passThroughOptions is added to a command without enablePositionalOptions

[shadowspawn]

I like catching the misconfiguration when using addCommand().

[shadowspawn]

This is the example case that convinced me this is worth doing, bypassed the check:

const parent = new Command('parent');
const sub = new Command('sub')
    .passThroughOptions();
parent.addCommand(sub); // allowed without enablePositionalOptions for parent

[shadowspawn]

(Introduces a throw so better go in a major version.)

[shadowspawn]

I am adding this review as a confirmation that requested changes will make it likely that merge accepted.

[aweebit]

I am adding this review as a confirmation that requested changes will make it likely that merge accepted.

All the requested changes have been made now.

[shadowspawn]

(Thanks for all the tweaks.)

[abetomo]

+1

[aweebit]

Thanks for the merge!

(Introduces a throw so better go in a major version.)

Should I set base to the next release branch when there is a breaking change? Or always to develop?

[shadowspawn]

Using next release branch for base is appropriate when know PR contains breaking changes.