WIP: Rework action parameters for more stable interface

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.

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', () => {

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 <file>')
     .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('<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 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() => {

tests/commander.configureCommand.test.js

@@ -19,7 +19,7 @@ test('when default then command passed to action', () => {
     .arguments('<value>')
     .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('<value>')
     .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('<value>')
     .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('<value>')
     .action(callback);
   program.parse(['node', 'test', 'value']);
-  expect(callback).toHaveBeenCalledWith('value', program.opts());
+  expect(callback).toHaveBeenCalledWith('value', program.opts(), program);
 });