From 8b01233ba51d64d643e2c3b0a64f8ead38b0e15d Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 19 Jun 2021 19:27:47 +1200 Subject: [PATCH 1/4] Factor out copySettings so can be used in programs using addCommand. --- lib/command.js | 52 ++++++++++++++++++------------ tests/command.chain.test.js | 7 ++++ tests/command.copySettings.test.js | 26 +++++++++++++++ 3 files changed, 65 insertions(+), 20 deletions(-) create mode 100644 tests/command.copySettings.test.js diff --git a/lib/command.js b/lib/command.js index 536d806bf..4c23e984a 100644 --- a/lib/command.js +++ b/lib/command.js @@ -73,6 +73,36 @@ class Command extends EventEmitter { this._helpConfiguration = {}; } + /** + * Copy settings that are useful to have in common across root command and subcommands. + * + * copySettings is used internally when adding a command using `.command()` to copy settings + * from the parent command to the subcommand. + * + * @param {Command} sourceCommand + * @return {Command} returns `this` for executable command + */ + copySettings(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 +138,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.copySettings(this); if (desc) return this; return cmd; diff --git a/tests/command.chain.test.js b/tests/command.chain.test.js index a450afdcd..4e6197080 100644 --- a/tests/command.chain.test.js +++ b/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 .copySettings() then returns this', () => { + const program = new Command(); + const cmd = new Command(); + const result = cmd.copySettings(program); + expect(result).toBe(cmd); + }); }); diff --git a/tests/command.copySettings.test.js b/tests/command.copySettings.test.js new file mode 100644 index 000000000..cfa2a8fb8 --- /dev/null +++ b/tests/command.copySettings.test.js @@ -0,0 +1,26 @@ +const commander = require('../'); + +test('when add subcommand with .command() then calls copySettings from parent', () => { + const program = new commander.Command(); + + // This is a bit intrusive, but check that copySettings is called internally. + const copySettingMock = jest.fn(); + program.createCommand = (name) => { + const cmd = new commander.Command(name); + cmd.copySettings = copySettingMock; + return cmd; + }; + program.command('sub'); + + expect(copySettingMock).toHaveBeenCalledWith(program); +}); + +describe('copySettings property tests', () => { + test('outputConfiguration', () => { + const source = new commander.Command(); + source.configureOutput({ foo: 'bar' }); + const cmd = new commander.Command(); + cmd.copySettings(source); + expect(cmd.configureOutput().foo).toEqual('bar'); + }); +}); From 4fbc1052a4bde49edead09a159e42440d78d34c8 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 3 Jul 2021 16:27:40 +1200 Subject: [PATCH 2/4] Fill out tests for properties for copySettings --- tests/command.copySettings.test.js | 113 ++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 3 deletions(-) diff --git a/tests/command.copySettings.test.js b/tests/command.copySettings.test.js index cfa2a8fb8..0fb84ff1a 100644 --- a/tests/command.copySettings.test.js +++ b/tests/command.copySettings.test.js @@ -1,9 +1,12 @@ 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 copySettings from parent', () => { const program = new commander.Command(); - // This is a bit intrusive, but check that copySettings is called internally. + // This is a bit intrusive, but check expectation that copySettings is called internally. const copySettingMock = jest.fn(); program.createCommand = (name) => { const cmd = new commander.Command(name); @@ -16,11 +19,115 @@ test('when add subcommand with .command() then calls copySettings from parent', }); describe('copySettings property tests', () => { - test('outputConfiguration', () => { + test('when copySettings then copies outputConfiguration(config)', () => { const source = new commander.Command(); - source.configureOutput({ foo: 'bar' }); const cmd = new commander.Command(); + + source.configureOutput({ foo: 'bar' }); cmd.copySettings(source); expect(cmd.configureOutput().foo).toEqual('bar'); }); + + test('when copySettings then copies helpOption(false)', () => { + const source = new commander.Command(); + const cmd = new commander.Command(); + expect(cmd._hasHelpOption).toBeTruthy(); + + source.helpOption(false); + cmd.copySettings(source); + expect(cmd._hasHelpOption).toBeFalsy(); + }); + + test('when copySettings then copies helpOption(flags, description)', () => { + const source = new commander.Command(); + const cmd = new commander.Command(); + + source.helpOption('-Z, --zz', 'ddd'); + cmd.copySettings(source); + expect(cmd._helpFlags).toBe('-Z, --zz'); + expect(cmd._helpDescription).toBe('ddd'); + expect(cmd._helpShortFlag).toBe('-Z'); + expect(cmd._helpLongFlag).toBe('--zz'); + }); + + test('when copySettings then copies addHelpCommand(name, description)', () => { + const source = new commander.Command(); + const cmd = new commander.Command(); + + source.addHelpCommand('HELP [cmd]', 'ddd'); + cmd.copySettings(source); + expect(cmd._helpCommandName).toBe('HELP'); + expect(cmd._helpCommandnameAndArgs).toBe('HELP [cmd]'); + expect(cmd._helpCommandDescription).toBe('ddd'); + }); + + test('when copySettings 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.copySettings(source); + expect(cmd.configureHelp()).toEqual(configuration); + }); + + test('when copySettings then copies exitOverride()', () => { + const source = new commander.Command(); + const cmd = new commander.Command(); + + expect(cmd._exitCallback).toBeFalsy(); + source.exitOverride(); + cmd.copySettings(source); + expect(cmd._exitCallback).toBeTruthy(); // actually a function + }); + + test('when copySettings then copies storeOptionsAsProperties()', () => { + const source = new commander.Command(); + const cmd = new commander.Command(); + + expect(cmd._storeOptionsAsProperties).toBeFalsy(); + source.storeOptionsAsProperties(); + cmd.copySettings(source); + expect(cmd._storeOptionsAsProperties).toBeTruthy(); + }); + + test('when copySettings then copies combineFlagAndOptionalValue()', () => { + const source = new commander.Command(); + const cmd = new commander.Command(); + + expect(cmd._combineFlagAndOptionalValue).toBeTruthy(); + source.combineFlagAndOptionalValue(false); + cmd.copySettings(source); + expect(cmd._combineFlagAndOptionalValue).toBeFalsy(); + }); + + test('when copySettings then copies allowExcessArguments()', () => { + const source = new commander.Command(); + const cmd = new commander.Command(); + + expect(cmd._allowExcessArguments).toBeTruthy(); + source.allowExcessArguments(false); + cmd.copySettings(source); + expect(cmd._allowExcessArguments).toBeFalsy(); + }); + + test('when copySettings then copies enablePositionalOptions()', () => { + const source = new commander.Command(); + const cmd = new commander.Command(); + + expect(cmd._enablePositionalOptions).toBeFalsy(); + source.enablePositionalOptions(); + cmd.copySettings(source); + expect(cmd._enablePositionalOptions).toBeTruthy(); + }); + + test('when copySettings then copies showHelpAfterError()', () => { + const source = new commander.Command(); + const cmd = new commander.Command(); + + expect(cmd._showHelpAfterError).toBeFalsy(); + source.showHelpAfterError(); + cmd.copySettings(source); + expect(cmd._showHelpAfterError).toBeTruthy(); + }); }); From bb497692806e193bb3e4a5f3acb18f9be825b69a Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 3 Jul 2021 16:36:28 +1200 Subject: [PATCH 3/4] Add TypeScript --- lib/command.js | 4 ++-- typings/index.d.ts | 8 ++++++++ typings/index.test-d.ts | 3 +++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/command.js b/lib/command.js index 4c23e984a..7fa636e3a 100644 --- a/lib/command.js +++ b/lib/command.js @@ -76,8 +76,8 @@ class Command extends EventEmitter { /** * Copy settings that are useful to have in common across root command and subcommands. * - * copySettings is used internally when adding a command using `.command()` to copy settings - * from the parent command to the subcommand. + * (Used internally when adding a command using `.command()` to copy settings + * from the parent command to the subcommand.) * * @param {Command} sourceCommand * @return {Command} returns `this` for executable command diff --git a/typings/index.d.ts b/typings/index.d.ts index d39db192a..be6959791 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -389,6 +389,14 @@ 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()` to copy settings + * from the parent command to the subcommand.) + */ + copySettings(sourceCommand: Command): this; + /** * Display the help or a custom message after an error occurs. */ diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index e1f6fc38c..62499c630 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -285,6 +285,9 @@ expectType(program.configureHelp({ })); expectType(program.configureHelp()); +// copySettings +expectType(program.copySettings(new commander.Command())); + // showHelpAfterError expectType(program.showHelpAfterError()); expectType(program.showHelpAfterError(true)); From 3ee80528a77e5080a9b03a3eefa4818569dc1fa5 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 3 Jul 2021 16:43:38 +1200 Subject: [PATCH 4/4] Rename --- lib/command.js | 7 ++-- tests/command.chain.test.js | 4 +-- tests/command.copySettings.test.js | 52 +++++++++++++++--------------- typings/index.d.ts | 5 ++- typings/index.test-d.ts | 4 +-- 5 files changed, 35 insertions(+), 37 deletions(-) diff --git a/lib/command.js b/lib/command.js index 7fa636e3a..997941e91 100644 --- a/lib/command.js +++ b/lib/command.js @@ -76,13 +76,12 @@ class Command extends EventEmitter { /** * Copy settings that are useful to have in common across root command and subcommands. * - * (Used internally when adding a command using `.command()` to copy settings - * from the parent command to the subcommand.) + * (Used internally when adding a command using `.command()` so subcommands inherit parent settings.) * * @param {Command} sourceCommand * @return {Command} returns `this` for executable command */ - copySettings(sourceCommand) { + copyInheritedSettings(sourceCommand) { this._outputConfiguration = sourceCommand._outputConfiguration; this._hasHelpOption = sourceCommand._hasHelpOption; this._helpFlags = sourceCommand._helpFlags; @@ -150,7 +149,7 @@ class Command extends EventEmitter { if (args) cmd.arguments(args); this.commands.push(cmd); cmd.parent = this; - cmd.copySettings(this); + cmd.copyInheritedSettings(this); if (desc) return this; return cmd; diff --git a/tests/command.chain.test.js b/tests/command.chain.test.js index 4e6197080..e80c2b292 100644 --- a/tests/command.chain.test.js +++ b/tests/command.chain.test.js @@ -184,10 +184,10 @@ describe('Command methods that should return this for chaining', () => { expect(result).toBe(program); }); - test('when call .copySettings() then returns this', () => { + test('when call .copyInheritedSettings() then returns this', () => { const program = new Command(); const cmd = new Command(); - const result = cmd.copySettings(program); + const result = cmd.copyInheritedSettings(program); expect(result).toBe(cmd); }); }); diff --git a/tests/command.copySettings.test.js b/tests/command.copySettings.test.js index 0fb84ff1a..79722d78b 100644 --- a/tests/command.copySettings.test.js +++ b/tests/command.copySettings.test.js @@ -3,14 +3,14 @@ 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 copySettings from parent', () => { +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 copySettings is called internally. + // 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.copySettings = copySettingMock; + cmd.copyInheritedSettings = copySettingMock; return cmd; }; program.command('sub'); @@ -18,116 +18,116 @@ test('when add subcommand with .command() then calls copySettings from parent', expect(copySettingMock).toHaveBeenCalledWith(program); }); -describe('copySettings property tests', () => { - test('when copySettings then copies outputConfiguration(config)', () => { +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.copySettings(source); + cmd.copyInheritedSettings(source); expect(cmd.configureOutput().foo).toEqual('bar'); }); - test('when copySettings then copies helpOption(false)', () => { + 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.copySettings(source); + cmd.copyInheritedSettings(source); expect(cmd._hasHelpOption).toBeFalsy(); }); - test('when copySettings then copies helpOption(flags, description)', () => { + test('when copyInheritedSettings then copies helpOption(flags, description)', () => { const source = new commander.Command(); const cmd = new commander.Command(); source.helpOption('-Z, --zz', 'ddd'); - cmd.copySettings(source); + 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 copySettings then copies addHelpCommand(name, description)', () => { + test('when copyInheritedSettings then copies addHelpCommand(name, description)', () => { const source = new commander.Command(); const cmd = new commander.Command(); source.addHelpCommand('HELP [cmd]', 'ddd'); - cmd.copySettings(source); + cmd.copyInheritedSettings(source); expect(cmd._helpCommandName).toBe('HELP'); expect(cmd._helpCommandnameAndArgs).toBe('HELP [cmd]'); expect(cmd._helpCommandDescription).toBe('ddd'); }); - test('when copySettings then copies configureHelp(config)', () => { + 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.copySettings(source); + cmd.copyInheritedSettings(source); expect(cmd.configureHelp()).toEqual(configuration); }); - test('when copySettings then copies exitOverride()', () => { + test('when copyInheritedSettings then copies exitOverride()', () => { const source = new commander.Command(); const cmd = new commander.Command(); expect(cmd._exitCallback).toBeFalsy(); source.exitOverride(); - cmd.copySettings(source); + cmd.copyInheritedSettings(source); expect(cmd._exitCallback).toBeTruthy(); // actually a function }); - test('when copySettings then copies storeOptionsAsProperties()', () => { + test('when copyInheritedSettings then copies storeOptionsAsProperties()', () => { const source = new commander.Command(); const cmd = new commander.Command(); expect(cmd._storeOptionsAsProperties).toBeFalsy(); source.storeOptionsAsProperties(); - cmd.copySettings(source); + cmd.copyInheritedSettings(source); expect(cmd._storeOptionsAsProperties).toBeTruthy(); }); - test('when copySettings then copies combineFlagAndOptionalValue()', () => { + test('when copyInheritedSettings then copies combineFlagAndOptionalValue()', () => { const source = new commander.Command(); const cmd = new commander.Command(); expect(cmd._combineFlagAndOptionalValue).toBeTruthy(); source.combineFlagAndOptionalValue(false); - cmd.copySettings(source); + cmd.copyInheritedSettings(source); expect(cmd._combineFlagAndOptionalValue).toBeFalsy(); }); - test('when copySettings then copies allowExcessArguments()', () => { + test('when copyInheritedSettings then copies allowExcessArguments()', () => { const source = new commander.Command(); const cmd = new commander.Command(); expect(cmd._allowExcessArguments).toBeTruthy(); source.allowExcessArguments(false); - cmd.copySettings(source); + cmd.copyInheritedSettings(source); expect(cmd._allowExcessArguments).toBeFalsy(); }); - test('when copySettings then copies enablePositionalOptions()', () => { + test('when copyInheritedSettings then copies enablePositionalOptions()', () => { const source = new commander.Command(); const cmd = new commander.Command(); expect(cmd._enablePositionalOptions).toBeFalsy(); source.enablePositionalOptions(); - cmd.copySettings(source); + cmd.copyInheritedSettings(source); expect(cmd._enablePositionalOptions).toBeTruthy(); }); - test('when copySettings then copies showHelpAfterError()', () => { + test('when copyInheritedSettings then copies showHelpAfterError()', () => { const source = new commander.Command(); const cmd = new commander.Command(); expect(cmd._showHelpAfterError).toBeFalsy(); source.showHelpAfterError(); - cmd.copySettings(source); + cmd.copyInheritedSettings(source); expect(cmd._showHelpAfterError).toBeTruthy(); }); }); diff --git a/typings/index.d.ts b/typings/index.d.ts index be6959791..3b9ac1521 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -392,10 +392,9 @@ export class Command { /** * Copy settings that are useful to have in common across root command and subcommands. * - * (Used internally when adding a command using `.command()` to copy settings - * from the parent command to the subcommand.) + * (Used internally when adding a command using `.command()` so subcommands inherit parent settings.) */ - copySettings(sourceCommand: Command): this; + copyInheritedSettings(sourceCommand: Command): this; /** * Display the help or a custom message after an error occurs. diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index 62499c630..66432ccd7 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -285,8 +285,8 @@ expectType(program.configureHelp({ })); expectType(program.configureHelp()); -// copySettings -expectType(program.copySettings(new commander.Command())); +// copyInheritedSettings +expectType(program.copyInheritedSettings(new commander.Command())); // showHelpAfterError expectType(program.showHelpAfterError());