From 94df44e5719635d433099cad872195ee3cbd3874 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Sat, 28 Sep 2019 22:48:45 -0400 Subject: [PATCH 1/9] Run default command if it's not just * --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 69469f7db..06586abb3 100644 --- a/index.js +++ b/index.js @@ -681,7 +681,7 @@ Command.prototype.parseArgs = function(args, unknown) { if (unknown.length > 0) { this.unknownOption(unknown[0]); } - if (this.commands.length === 0 && + if ((this.commands.length !== 1 || this.commands[0]._name !== "*") && this._args.filter(function(a) { return a.required; }).length === 0) { this.emit('command:*'); } From f5ecadd0dc05f9a822da5bdef61a6a13f2be9a58 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Sat, 28 Sep 2019 22:50:52 -0400 Subject: [PATCH 2/9] Fix lint error --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 06586abb3..b6923ba18 100644 --- a/index.js +++ b/index.js @@ -681,7 +681,7 @@ Command.prototype.parseArgs = function(args, unknown) { if (unknown.length > 0) { this.unknownOption(unknown[0]); } - if ((this.commands.length !== 1 || this.commands[0]._name !== "*") && + if ((this.commands.length !== 1 || this.commands[0]._name !== '*') && this._args.filter(function(a) { return a.required; }).length === 0) { this.emit('command:*'); } From 6a87525ef966b5a2259fddc88df5c09dbf30b5e9 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Sat, 28 Sep 2019 22:48:45 -0400 Subject: [PATCH 3/9] Add prgm action, only emit command:* when it's registered --- index.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/index.js b/index.js index ce38da26b..4c9bc11d9 100644 --- a/index.js +++ b/index.js @@ -360,8 +360,12 @@ Command.prototype.action = function(fn) { fn.apply(self, actionArgs); }; var parent = this.parent || this; - var name = parent === this ? '*' : this._name; - parent.on('command:' + name, listener); + if (parent === this) { + parent.on('program-action', listener); + } else { + parent.on('command:' + this._name, listener); + } + if (this._alias) parent.on('command:' + this._alias, listener); return this; }; @@ -793,25 +797,28 @@ Command.prototype.normalize = function(args) { Command.prototype.parseArgs = function(args, unknown) { var name; + if (this.listeners('program-action').length && this.listeners('command:*').length) { + console.error("Can't have listeners for both command:* and for the main program"); + this._exit(1); + } + if (args.length) { name = args[0]; if (this.listeners('command:' + name).length) { this.emit('command:' + args.shift(), args, unknown); - } else { + } else if (this.listeners('command:*').length) { this.emit('command:*', args, unknown); + } else { + this.emit('program-action', args, unknown); } } else { outputHelpIfNecessary(this, unknown); - // If there were no args and we have unknown options, // then they are extraneous and we need to error. if (unknown.length > 0 && !this.defaultExecutable) { this.unknownOption(unknown[0]); } - if (this.commands.length === 0 && - this._args.filter(function(a) { return a.required; }).length === 0) { - this.emit('command:*'); - } + this.emit('program-action'); } return this; From b44d1a0efccb6b546412f407e7c49007dd5dd19d Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Sat, 7 Dec 2019 01:49:05 -0500 Subject: [PATCH 4/9] Test asterisk handler not called without args --- tests/command.asterisk.test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/command.asterisk.test.js b/tests/command.asterisk.test.js index 44a8690f2..1a5e79ce4 100644 --- a/tests/command.asterisk.test.js +++ b/tests/command.asterisk.test.js @@ -10,6 +10,16 @@ test('when no arguments then asterisk action not called', () => { expect(mockAction).not.toHaveBeenCalled(); }); +test('when no arguments with asterisk handler then asterisk action not called', () => { + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .command('*') + .action(mockAction); + program.parse(['node', 'test']); + expect(mockAction).not.toHaveBeenCalled(); +}); + test('when recognised command then asterisk action not called', () => { const mockAction = jest.fn(); const program = new commander.Command(); From 1e78ec0db63cea9b1bbf50e5aa21fe28cf337b09 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 8 Dec 2019 16:59:33 +1300 Subject: [PATCH 5/9] Restore check to not call program when signature wrong, and expand program tests --- index.js | 5 ++++- tests/command.action.test.js | 29 +++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 4c9bc11d9..af724d456 100644 --- a/index.js +++ b/index.js @@ -818,7 +818,10 @@ Command.prototype.parseArgs = function(args, unknown) { if (unknown.length > 0 && !this.defaultExecutable) { this.unknownOption(unknown[0]); } - this.emit('program-action'); + // Call the program actionm handler, unless it has a (missing) required parameter and signature does not match. + if (this._args.filter(function(a) { return a.required; }).length === 0) { + this.emit('program-action'); + } } return this; diff --git a/tests/command.action.test.js b/tests/command.action.test.js index fa0f85c35..ebe9f3caf 100644 --- a/tests/command.action.test.js +++ b/tests/command.action.test.js @@ -35,7 +35,7 @@ test('when .action called with extra arguments then extras also passed to action expect(actionMock).toHaveBeenCalledWith('my-file', cmd, ['a']); }); -test('when .action on program with argument then action called', () => { +test('when .action on program with required argument and argument supplied then action called', () => { const actionMock = jest.fn(); const program = new commander.Command(); program @@ -45,6 +45,16 @@ test('when .action on program with argument then action called', () => { expect(actionMock).toHaveBeenCalledWith('my-file', program); }); +test('when .action on program with required argument and argument not supplied then action not called', () => { + const actionMock = jest.fn(); + const program = new commander.Command(); + program + .arguments('') + .action(actionMock); + program.parse(['node', 'test']); + expect(actionMock).not.toHaveBeenCalled(); +}); + // Changes made in #729 to call program action handler test('when .action on program and no arguments then action called', () => { const actionMock = jest.fn(); @@ -75,7 +85,7 @@ test('when .action on program without optional argument supplied then action cal expect(actionMock).toHaveBeenCalledWith(undefined, program); }); -test('when .action on program with subcommand and program argument then program action called', () => { +test('when .action on program with optional argument and subcommand and program argument then program action called', () => { const actionMock = jest.fn(); const program = new commander.Command(); program @@ -88,3 +98,18 @@ test('when .action on program with subcommand and program argument then program expect(actionMock).toHaveBeenCalledWith('a', program); }); + +// Changes made in #1062 to allow this case +test('when .action on program with optional argument and subcommand and no program argument then program action called', () => { + const actionMock = jest.fn(); + const program = new commander.Command(); + program + .arguments('[file]') + .action(actionMock); + program + .command('subcommand'); + + program.parse(['node', 'test']); + + expect(actionMock).toHaveBeenCalledWith(undefined, program); +}); From b9e2570342dcdb0b13a10b96f7b1c1a310978f45 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 8 Dec 2019 17:50:19 +1300 Subject: [PATCH 6/9] Document and rework and extend asterisk tests --- tests/command.asterisk.test.js | 132 ++++++++++++++++++++++++--------- 1 file changed, 95 insertions(+), 37 deletions(-) diff --git a/tests/command.asterisk.test.js b/tests/command.asterisk.test.js index 1a5e79ce4..19d0257f9 100644 --- a/tests/command.asterisk.test.js +++ b/tests/command.asterisk.test.js @@ -1,44 +1,102 @@ const commander = require('../'); -test('when no arguments then asterisk action not called', () => { - const mockAction = jest.fn(); - const program = new commander.Command(); - program - .command('install') - .action(mockAction); - program.parse(['node', 'test']); - expect(mockAction).not.toHaveBeenCalled(); -}); +// .command('*') is the old main/default command handler. It adds a listener +// for 'command:*'. It has been somewhat replaced by the program action handler, +// so most uses are probably old code. Current plan is keep the code backwards compatible +// and put work in elsewhere for new code (e.g. evolving behaviour for program action handler). +// +// The event 'command:*' is also listened for directly for testing for unknown commands +// due to an example in the README, although this is not robust (e.g. sent for git-style commands). +// +// Historical: the event 'command:*' used to also be shared by the action handler on the program. -test('when no arguments with asterisk handler then asterisk action not called', () => { - const mockAction = jest.fn(); - const program = new commander.Command(); - program - .command('*') - .action(mockAction); - program.parse(['node', 'test']); - expect(mockAction).not.toHaveBeenCalled(); -}); +describe(".command('*')", () => { + test('when no arguments then asterisk action not called', () => { + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .command('*') + .action(mockAction); + program.parse(['node', 'test']); + expect(mockAction).not.toHaveBeenCalled(); + }); + + test('when unrecognised argument then asterisk action called', () => { + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .command('*') + .action(mockAction); + program.parse(['node', 'test', 'unrecognised-command']); + expect(mockAction).toHaveBeenCalled(); + }); + + test('when recognised command then asterisk action not called', () => { + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .command('install') + .action(() => { }); + program + .command('*') + .action(mockAction); + program.parse(['node', 'test', 'install']); + expect(mockAction).not.toHaveBeenCalled(); + }); -test('when recognised command then asterisk action not called', () => { - const mockAction = jest.fn(); - const program = new commander.Command(); - program - .command('install') - .action(() => { }); - program - .action(mockAction); - program.parse(['node', 'test', 'install']); - expect(mockAction).not.toHaveBeenCalled(); + test('when unrecognised command/argument then asterisk action called', () => { + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .command('install'); + program + .command('*') + .action(mockAction); + program.parse(['node', 'test', 'unrecognised-command']); + expect(mockAction).toHaveBeenCalled(); + }); }); -test('when unrecognised command/argument then asterisk action called', () => { - const mockAction = jest.fn(); - const program = new commander.Command(); - program - .command('install'); - program - .action(mockAction); - program.parse(['node', 'test', 'unrecognised-command']); - expect(mockAction).toHaveBeenCalled(); +// Test .on explicitly rather than assuming covered by .command +describe(".on('command:*')", () => { + test('when no arguments then listener not called', () => { + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .on('command:*', mockAction); + program.parse(['node', 'test']); + expect(mockAction).not.toHaveBeenCalled(); + }); + + test('when unrecognised argument then listener called', () => { + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .on('command:*', mockAction); + program.parse(['node', 'test', 'unrecognised-command']); + expect(mockAction).toHaveBeenCalled(); + }); + + test('when recognised command then listener not called', () => { + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .command('install') + .action(() => { }); + program + .on('command:*', mockAction); + program.parse(['node', 'test', 'install']); + expect(mockAction).not.toHaveBeenCalled(); + }); + + test('when unrecognised command/argument then listener called', () => { + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .command('install'); + program + .on('command:*', mockAction); + program.parse(['node', 'test', 'unrecognised-command']); + expect(mockAction).toHaveBeenCalled(); + }); }); From ec66ad576331ae8e012e81b899825f5f4da81b7b Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 9 Dec 2019 18:46:01 -0500 Subject: [PATCH 7/9] Update * and program-action logic based on PR feedback --- index.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/index.js b/index.js index e74d5b043..fb13697af 100644 --- a/index.js +++ b/index.js @@ -362,6 +362,7 @@ Command.prototype.action = function(fn) { var parent = this.parent || this; if (parent === this) { parent.on('program-action', listener); + parent.on('command:*', listener) } else { parent.on('command:' + this._name, listener); } @@ -797,19 +798,13 @@ Command.prototype.normalize = function(args) { Command.prototype.parseArgs = function(args, unknown) { var name; - if (this.listeners('program-action').length && this.listeners('command:*').length) { - console.error("Can't have listeners for both command:* and for the main program"); - this._exit(1); - } - if (args.length) { name = args[0]; if (this.listeners('command:' + name).length) { this.emit('command:' + args.shift(), args, unknown); - } else if (this.listeners('command:*').length) { - this.emit('command:*', args, unknown); } else { this.emit('program-action', args, unknown); + this.emit('command:*', args, unknown); } } else { outputHelpIfNecessary(this, unknown); From 676f23d7573809da8c0d3e3a1fce2634faac6ea1 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 9 Dec 2019 18:49:19 -0500 Subject: [PATCH 8/9] Rename program-action to program-command --- index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index fb13697af..c5fdbfcf5 100644 --- a/index.js +++ b/index.js @@ -361,8 +361,8 @@ Command.prototype.action = function(fn) { }; var parent = this.parent || this; if (parent === this) { - parent.on('program-action', listener); - parent.on('command:*', listener) + parent.on('program-command', listener); + parent.on('command:*', listener); } else { parent.on('command:' + this._name, listener); } @@ -803,7 +803,7 @@ Command.prototype.parseArgs = function(args, unknown) { if (this.listeners('command:' + name).length) { this.emit('command:' + args.shift(), args, unknown); } else { - this.emit('program-action', args, unknown); + this.emit('program-command', args, unknown); this.emit('command:*', args, unknown); } } else { @@ -815,7 +815,7 @@ Command.prototype.parseArgs = function(args, unknown) { } // Call the program action handler, unless it has a (missing) required parameter and signature does not match. if (this._args.filter(function(a) { return a.required; }).length === 0) { - this.emit('program-action'); + this.emit('program-command'); } } From 8b71d8d4cad18fbd900412593fa663ec8cce97c3 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 10 Dec 2019 01:57:55 -0500 Subject: [PATCH 9/9] Remove unnecessary command listener --- index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/index.js b/index.js index c5fdbfcf5..42caa05e6 100644 --- a/index.js +++ b/index.js @@ -362,7 +362,6 @@ Command.prototype.action = function(fn) { var parent = this.parent || this; if (parent === this) { parent.on('program-command', listener); - parent.on('command:*', listener); } else { parent.on('command:' + this._name, listener); }