Add copyInheritedSettings (#1557)

* Factor out copySettings so can be used in programs using addCommand.

* Fill out tests for properties for copySettings

* Add TypeScript

* Rename

Author: shadowspawn

lib/command.js

@@ -73,6 +73,35 @@ class Command extends EventEmitter {
     this._helpConfiguration = {};
   }
 
+  /**
+   * Copy settings that are useful to have in common across root command and subcommands.
+   *
+   * (Used internally when adding a command using `.command()` so subcommands inherit parent settings.)
+   *
+   * @param {Command} sourceCommand
+   * @return {Command} returns `this` for executable command
+   */
+  copyInheritedSettings(sourceCommand) {
+    this._outputConfiguration = sourceCommand._outputConfiguration;
+    this._hasHelpOption = sourceCommand._hasHelpOption;
+    this._helpFlags = sourceCommand._helpFlags;
+    this._helpDescription = sourceCommand._helpDescription;
+    this._helpShortFlag = sourceCommand._helpShortFlag;
+    this._helpLongFlag = sourceCommand._helpLongFlag;
+    this._helpCommandName = sourceCommand._helpCommandName;
+    this._helpCommandnameAndArgs = sourceCommand._helpCommandnameAndArgs;
+    this._helpCommandDescription = sourceCommand._helpCommandDescription;
+    this._helpConfiguration = sourceCommand._helpConfiguration;
+    this._exitCallback = sourceCommand._exitCallback;
+    this._storeOptionsAsProperties = sourceCommand._storeOptionsAsProperties;
+    this._combineFlagAndOptionalValue = sourceCommand._combineFlagAndOptionalValue;
+    this._allowExcessArguments = sourceCommand._allowExcessArguments;
+    this._enablePositionalOptions = sourceCommand._enablePositionalOptions;
+    this._showHelpAfterError = sourceCommand._showHelpAfterError;
+
+    return this;
+  }
+
   /**
    * Define a command.
    *
@@ -108,37 +137,19 @@ class Command extends EventEmitter {
     }
     opts = opts || {};
     const [, name, args] = nameAndArgs.match(/([^ ]+) *(.*)/);
-    const cmd = this.createCommand(name);
 
+    const cmd = this.createCommand(name);
     if (desc) {
       cmd.description(desc);
       cmd._executableHandler = true;
     }
     if (opts.isDefault) this._defaultCommandName = cmd._name;
-
-    cmd._outputConfiguration = this._outputConfiguration;
-
     cmd._hidden = !!(opts.noHelp || opts.hidden); // noHelp is deprecated old name for hidden
-    cmd._hasHelpOption = this._hasHelpOption;
-    cmd._helpFlags = this._helpFlags;
-    cmd._helpDescription = this._helpDescription;
-    cmd._helpShortFlag = this._helpShortFlag;
-    cmd._helpLongFlag = this._helpLongFlag;
-    cmd._helpCommandName = this._helpCommandName;
-    cmd._helpCommandnameAndArgs = this._helpCommandnameAndArgs;
-    cmd._helpCommandDescription = this._helpCommandDescription;
-    cmd._helpConfiguration = this._helpConfiguration;
-    cmd._exitCallback = this._exitCallback;
-    cmd._storeOptionsAsProperties = this._storeOptionsAsProperties;
-    cmd._combineFlagAndOptionalValue = this._combineFlagAndOptionalValue;
-    cmd._allowExcessArguments = this._allowExcessArguments;
-    cmd._enablePositionalOptions = this._enablePositionalOptions;
-    cmd._showHelpAfterError = this._showHelpAfterError;
-
     cmd._executableFile = opts.executableFile || null; // Custom name for executable file, set missing to null to match constructor
     if (args) cmd.arguments(args);
     this.commands.push(cmd);
     cmd.parent = this;
+    cmd.copyInheritedSettings(this);
 
     if (desc) return this;
     return cmd;

tests/command.chain.test.js

@@ -183,4 +183,11 @@ describe('Command methods that should return this for chaining', () => {
     const result = program.showHelpAfterError();
     expect(result).toBe(program);
   });
+
+  test('when call .copyInheritedSettings() then returns this', () => {
+    const program = new Command();
+    const cmd = new Command();
+    const result = cmd.copyInheritedSettings(program);
+    expect(result).toBe(cmd);
+  });
 });

tests/command.copySettings.test.js

@@ -0,0 +1,133 @@
+const commander = require('../');
+
+// Tests some private properties as simpler than pure tests of observable behaviours.
+// Testing before and after values in some cases, to ensure value actually changes (when copied).
+
+test('when add subcommand with .command() then calls copyInheritedSettings from parent', () => {
+  const program = new commander.Command();
+
+  // This is a bit intrusive, but check expectation that copyInheritedSettings is called internally.
+  const copySettingMock = jest.fn();
+  program.createCommand = (name) => {
+    const cmd = new commander.Command(name);
+    cmd.copyInheritedSettings = copySettingMock;
+    return cmd;
+  };
+  program.command('sub');
+
+  expect(copySettingMock).toHaveBeenCalledWith(program);
+});
+
+describe('copyInheritedSettings property tests', () => {
+  test('when copyInheritedSettings then copies outputConfiguration(config)', () => {
+    const source = new commander.Command();
+    const cmd = new commander.Command();
+
+    source.configureOutput({ foo: 'bar' });
+    cmd.copyInheritedSettings(source);
+    expect(cmd.configureOutput().foo).toEqual('bar');
+  });
+
+  test('when copyInheritedSettings then copies helpOption(false)', () => {
+    const source = new commander.Command();
+    const cmd = new commander.Command();
+    expect(cmd._hasHelpOption).toBeTruthy();
+
+    source.helpOption(false);
+    cmd.copyInheritedSettings(source);
+    expect(cmd._hasHelpOption).toBeFalsy();
+  });
+
+  test('when copyInheritedSettings then copies helpOption(flags, description)', () => {
+    const source = new commander.Command();
+    const cmd = new commander.Command();
+
+    source.helpOption('-Z, --zz', 'ddd');
+    cmd.copyInheritedSettings(source);
+    expect(cmd._helpFlags).toBe('-Z, --zz');
+    expect(cmd._helpDescription).toBe('ddd');
+    expect(cmd._helpShortFlag).toBe('-Z');
+    expect(cmd._helpLongFlag).toBe('--zz');
+  });
+
+  test('when copyInheritedSettings then copies addHelpCommand(name, description)', () => {
+    const source = new commander.Command();
+    const cmd = new commander.Command();
+
+    source.addHelpCommand('HELP [cmd]', 'ddd');
+    cmd.copyInheritedSettings(source);
+    expect(cmd._helpCommandName).toBe('HELP');
+    expect(cmd._helpCommandnameAndArgs).toBe('HELP [cmd]');
+    expect(cmd._helpCommandDescription).toBe('ddd');
+  });
+
+  test('when copyInheritedSettings then copies configureHelp(config)', () => {
+    const source = new commander.Command();
+    const cmd = new commander.Command();
+
+    const configuration = { foo: 'bar', helpWidth: 123, sortSubcommands: true };
+    source.configureHelp(configuration);
+    cmd.copyInheritedSettings(source);
+    expect(cmd.configureHelp()).toEqual(configuration);
+  });
+
+  test('when copyInheritedSettings then copies exitOverride()', () => {
+    const source = new commander.Command();
+    const cmd = new commander.Command();
+
+    expect(cmd._exitCallback).toBeFalsy();
+    source.exitOverride();
+    cmd.copyInheritedSettings(source);
+    expect(cmd._exitCallback).toBeTruthy(); // actually a function
+  });
+
+  test('when copyInheritedSettings then copies storeOptionsAsProperties()', () => {
+    const source = new commander.Command();
+    const cmd = new commander.Command();
+
+    expect(cmd._storeOptionsAsProperties).toBeFalsy();
+    source.storeOptionsAsProperties();
+    cmd.copyInheritedSettings(source);
+    expect(cmd._storeOptionsAsProperties).toBeTruthy();
+  });
+
+  test('when copyInheritedSettings then copies combineFlagAndOptionalValue()', () => {
+    const source = new commander.Command();
+    const cmd = new commander.Command();
+
+    expect(cmd._combineFlagAndOptionalValue).toBeTruthy();
+    source.combineFlagAndOptionalValue(false);
+    cmd.copyInheritedSettings(source);
+    expect(cmd._combineFlagAndOptionalValue).toBeFalsy();
+  });
+
+  test('when copyInheritedSettings then copies allowExcessArguments()', () => {
+    const source = new commander.Command();
+    const cmd = new commander.Command();
+
+    expect(cmd._allowExcessArguments).toBeTruthy();
+    source.allowExcessArguments(false);
+    cmd.copyInheritedSettings(source);
+    expect(cmd._allowExcessArguments).toBeFalsy();
+  });
+
+  test('when copyInheritedSettings then copies enablePositionalOptions()', () => {
+    const source = new commander.Command();
+    const cmd = new commander.Command();
+
+    expect(cmd._enablePositionalOptions).toBeFalsy();
+    source.enablePositionalOptions();
+    cmd.copyInheritedSettings(source);
+    expect(cmd._enablePositionalOptions).toBeTruthy();
+  });
+
+  test('when copyInheritedSettings then copies showHelpAfterError()', () => {
+    const source = new commander.Command();
+    const cmd = new commander.Command();
+
+    expect(cmd._showHelpAfterError).toBeFalsy();
+    source.showHelpAfterError();
+    cmd.copyInheritedSettings(source);
+    expect(cmd._showHelpAfterError).toBeTruthy();
+  });
+});

typings/index.d.ts

@@ -389,6 +389,13 @@ export class Command {
   /** Get configuration */
   configureOutput(): OutputConfiguration;
 
+  /**
+   * Copy settings that are useful to have in common across root command and subcommands.
+   *
+   * (Used internally when adding a command using `.command()` so subcommands inherit parent settings.)
+   */
+   copyInheritedSettings(sourceCommand: Command): this;
+
   /**
    * Display the help or a custom message after an error occurs.
    */

typings/index.test-d.ts

@@ -285,6 +285,9 @@ expectType<commander.Command>(program.configureHelp({
 }));
 expectType<commander.HelpConfiguration>(program.configureHelp());
 
+// copyInheritedSettings
+expectType<commander.Command>(program.copyInheritedSettings(new commander.Command()));
+
 // showHelpAfterError
 expectType<commander.Command>(program.showHelpAfterError());
 expectType<commander.Command>(program.showHelpAfterError(true));