From f3b5091887ea92169729433c3e397fffa1e385aa Mon Sep 17 00:00:00 2001 From: John Gee Date: Mon, 30 Nov 2020 19:25:35 +1300 Subject: [PATCH] First cut --- index.js | 8 +++----- tests/args.variadic.test.js | 8 ++++---- tests/command.action.test.js | 21 ++++++++++----------- tests/commander.configureCommand.test.js | 8 ++++---- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/index.js b/index.js index 668c0e775..1697101ee 100644 --- a/index.js +++ b/index.js @@ -890,18 +890,16 @@ class Command extends EventEmitter { action(fn) { const listener = (args) => { - // The .action callback takes an extra parameter which is the command or options. const expectedArgsCount = this._args.length; const actionArgs = args.slice(0, expectedArgsCount); + // Add the "options" (which might actually be the command). if (this._passCommandToAction) { actionArgs[expectedArgsCount] = this; } else { actionArgs[expectedArgsCount] = this.opts(); } - // Add the extra arguments so available too. - if (args.length > expectedArgsCount) { - actionArgs.push(args.slice(expectedArgsCount)); - } + // Add the command. + actionArgs.push(this); const actionResult = fn.apply(this, actionArgs); // Remember result in case it is async. Assume parseAsync getting called on root. diff --git a/tests/args.variadic.test.js b/tests/args.variadic.test.js index 750880175..094f72eaa 100644 --- a/tests/args.variadic.test.js +++ b/tests/args.variadic.test.js @@ -12,7 +12,7 @@ describe('variadic argument', () => { program.parse(['node', 'test', 'id']); - expect(actionMock).toHaveBeenCalledWith('id', [], program); + expect(actionMock).toHaveBeenCalledWith('id', [], program, program); }); test('when extra arguments specified for program then variadic arg is array of values', () => { @@ -25,7 +25,7 @@ describe('variadic argument', () => { program.parse(['node', 'test', 'id', ...extraArguments]); - expect(actionMock).toHaveBeenCalledWith('id', extraArguments, program); + expect(actionMock).toHaveBeenCalledWith('id', extraArguments, program, program); }); test('when no extra arguments specified for command then variadic arg is empty array', () => { @@ -37,7 +37,7 @@ describe('variadic argument', () => { program.parse(['node', 'test', 'sub']); - expect(actionMock).toHaveBeenCalledWith([], cmd); + expect(actionMock).toHaveBeenCalledWith([], cmd, cmd); }); test('when extra arguments specified for command then variadic arg is array of values', () => { @@ -50,7 +50,7 @@ describe('variadic argument', () => { program.parse(['node', 'test', 'sub', ...extraArguments]); - expect(actionMock).toHaveBeenCalledWith(extraArguments, cmd); + expect(actionMock).toHaveBeenCalledWith(extraArguments, cmd, cmd); }); test('when program variadic argument not last then error', () => { diff --git a/tests/command.action.test.js b/tests/command.action.test.js index cef18bee8..8ac2ece43 100644 --- a/tests/command.action.test.js +++ b/tests/command.action.test.js @@ -9,7 +9,7 @@ test('when .action called then command passed to action', () => { .command('info') .action(actionMock); program.parse(['node', 'test', 'info']); - expect(actionMock).toHaveBeenCalledWith(cmd); + expect(actionMock).toHaveBeenCalledWith(cmd, cmd); }); test('when .action called then program.args only contains args', () => { @@ -23,16 +23,15 @@ test('when .action called then program.args only contains args', () => { expect(program.args).toEqual(['info', 'my-file']); }); -test('when .action called with extra arguments then extras also passed to action', () => { - // This is a new and undocumented behaviour for now. - // Might make this an error by default in future. +test('when .action called with extra arguments then extras not passed to action', () => { + // In Command v5 and v6 we were passing extraArgs as undocumented parameter. Removed in v7. const actionMock = jest.fn(); const program = new commander.Command(); const cmd = program .command('info ') .action(actionMock); program.parse(['node', 'test', 'info', 'my-file', 'a']); - expect(actionMock).toHaveBeenCalledWith('my-file', cmd, ['a']); + expect(actionMock).toHaveBeenCalledWith('my-file', cmd, cmd); }); test('when .action on program with required argument and argument supplied then action called', () => { @@ -42,7 +41,7 @@ test('when .action on program with required argument and argument supplied then .arguments('') .action(actionMock); program.parse(['node', 'test', 'my-file']); - expect(actionMock).toHaveBeenCalledWith('my-file', program); + expect(actionMock).toHaveBeenCalledWith('my-file', program, program); }); test('when .action on program with required argument and argument not supplied then action not called', () => { @@ -66,7 +65,7 @@ test('when .action on program and no arguments then action called', () => { program .action(actionMock); program.parse(['node', 'test']); - expect(actionMock).toHaveBeenCalledWith(program); + expect(actionMock).toHaveBeenCalledWith(program, program); }); test('when .action on program with optional argument supplied then action called', () => { @@ -76,7 +75,7 @@ test('when .action on program with optional argument supplied then action called .arguments('[file]') .action(actionMock); program.parse(['node', 'test', 'my-file']); - expect(actionMock).toHaveBeenCalledWith('my-file', program); + expect(actionMock).toHaveBeenCalledWith('my-file', program, program); }); test('when .action on program without optional argument supplied then action called', () => { @@ -86,7 +85,7 @@ test('when .action on program without optional argument supplied then action cal .arguments('[file]') .action(actionMock); program.parse(['node', 'test']); - expect(actionMock).toHaveBeenCalledWith(undefined, program); + expect(actionMock).toHaveBeenCalledWith(undefined, program, program); }); test('when .action on program with optional argument and subcommand and program argument then program action called', () => { @@ -100,7 +99,7 @@ test('when .action on program with optional argument and subcommand and program program.parse(['node', 'test', 'a']); - expect(actionMock).toHaveBeenCalledWith('a', program); + expect(actionMock).toHaveBeenCalledWith('a', program, program); }); // Changes made in #1062 to allow this case @@ -115,7 +114,7 @@ test('when .action on program with optional argument and subcommand and no progr program.parse(['node', 'test']); - expect(actionMock).toHaveBeenCalledWith(undefined, program); + expect(actionMock).toHaveBeenCalledWith(undefined, program, program); }); test('when action is async then can await parseAsync', async() => { diff --git a/tests/commander.configureCommand.test.js b/tests/commander.configureCommand.test.js index 69ad7bcb4..065d52271 100644 --- a/tests/commander.configureCommand.test.js +++ b/tests/commander.configureCommand.test.js @@ -19,7 +19,7 @@ test('when default then command passed to action', () => { .arguments('') .action(callback); program.parse(['node', 'test', 'value']); - expect(callback).toHaveBeenCalledWith('value', program); + expect(callback).toHaveBeenCalledWith('value', program, program); }); // storeOptionsAsProperties @@ -61,7 +61,7 @@ test('when passCommandToAction() then command passed to action', () => { .arguments('') .action(callback); program.parse(['node', 'test', 'value']); - expect(callback).toHaveBeenCalledWith('value', program); + expect(callback).toHaveBeenCalledWith('value', program, program); }); test('when passCommandToAction(true) then command passed to action', () => { @@ -72,7 +72,7 @@ test('when passCommandToAction(true) then command passed to action', () => { .arguments('') .action(callback); program.parse(['node', 'test', 'value']); - expect(callback).toHaveBeenCalledWith('value', program); + expect(callback).toHaveBeenCalledWith('value', program, program); }); test('when passCommandToAction(false) then options passed to action', () => { @@ -83,5 +83,5 @@ test('when passCommandToAction(false) then options passed to action', () => { .arguments('') .action(callback); program.parse(['node', 'test', 'value']); - expect(callback).toHaveBeenCalledWith('value', program.opts()); + expect(callback).toHaveBeenCalledWith('value', program.opts(), program); });