From 37b665ab1b592f27f3f8a1627a709054c60ab78f Mon Sep 17 00:00:00 2001 From: John Gee Date: Fri, 29 May 2026 11:12:16 +1200 Subject: [PATCH 1/3] Modify example commander file to match others and what testing expects. Enable cli tests on Commander too. --- examples/demo.commander.ts | 18 ++++++++++ tests/__snapshots__/cli.test.ts.snap | 54 ++++++++++++++++++++++++++++ tests/cli.test.ts | 21 +++++------ 3 files changed, 81 insertions(+), 12 deletions(-) diff --git a/examples/demo.commander.ts b/examples/demo.commander.ts index 7582a39..0a3793e 100644 --- a/examples/demo.commander.ts +++ b/examples/demo.commander.ts @@ -12,6 +12,24 @@ program .option('-v, --verbose', 'enable verbose output'); // Add commands +const devCommand = program + .command('dev') + .description('Start dev server') + .option('-H, --host [host]', `Specify hostname`) + .option('-p, --port ', `Specify port`) + .option('-v, --verbose', `Enable verbose logging`) + .option('--quiet', `Suppress output`) + .action((options) => {}); +// subcommands of dev +devCommand + .command('start') + .description('Start development server') + .action((options) => {}); +devCommand + .command('build') + .description('Build project') + .action((options) => {}); + program .command('serve') .description('Start the server') diff --git a/tests/__snapshots__/cli.test.ts.snap b/tests/__snapshots__/cli.test.ts.snap index dc5cb94..7a55b16 100644 --- a/tests/__snapshots__/cli.test.ts.snap +++ b/tests/__snapshots__/cli.test.ts.snap @@ -626,6 +626,60 @@ my-tool My tool " `; +exports[`cli completion tests for commander > cli option completion tests > should complete option for partial input '{ partial: '--p', expected: '--port' }' 1`] = ` +"--port Specify port +:4 +" +`; + +exports[`cli completion tests for commander > cli option completion tests > should complete option for partial input '{ partial: '-H', expected: '-H' }' 1`] = ` +"-H Specify hostname +:4 +" +`; + +exports[`cli completion tests for commander > cli option completion tests > should complete option for partial input '{ partial: '-p', expected: '-p' }' 1`] = ` +"-p Specify port +:4 +" +`; + +exports[`cli completion tests for commander > cli option exclusion tests > should not suggest already specified option '{ specified: '--config', shouldNotContain: '--config' }' 1`] = ` +":4 +" +`; + +exports[`cli completion tests for commander > cli option value handling > should handle unknown options with no completions 1`] = `":4"`; + +exports[`cli completion tests for commander > cli option value handling > should not show duplicate options 1`] = ` +"--version output the version number +--config specify config file +--debug enable debugging +--verbose enable verbose output +:4 +" +`; + +exports[`cli completion tests for commander > cli option value handling > should resolve config option values correctly 1`] = ` +":4 +" +`; + +exports[`cli completion tests for commander > cli option value handling > should resolve port value correctly 1`] = ` +":4 +" +`; + +exports[`cli completion tests for commander > should complete cli options 1`] = ` +"dev Start dev server +serve Start the server +build Build the project +deploy Deploy the application +lint Lint source files +:4 +" +`; + exports[`cli completion tests for t > --config option tests > should complete --config option values 1`] = ` "vite.config.ts Vite config file vite.config.js Vite config file diff --git a/tests/cli.test.ts b/tests/cli.test.ts index 17222d7..9d83fbe 100644 --- a/tests/cli.test.ts +++ b/tests/cli.test.ts @@ -14,24 +14,21 @@ function runCommand(command: string): Promise { } const cliTools = ['t', 'citty', 'cac', 'commander']; +// const cliTools = ['commander']; // TEMP For quick testing of commander-specific behavior, uncomment this line and comment out the line above. describe.each(cliTools)('cli completion tests for %s', (cliTool) => { // For Commander, we need to skip most of the tests since it handles completion differently + // Legacy comment? Reenabling tests... const shouldSkipTest = cliTool === 'commander'; - // Commander uses a different command structure for completion - // TODO: why commander does that? our convention is the -- part which should be always there. - const commandPrefix = - cliTool === 'commander' - ? `pnpm tsx examples/demo.${cliTool}.ts complete` - : `pnpm tsx examples/demo.${cliTool}.ts complete --`; + const commandPrefix = `pnpm tsx examples/demo.${cliTool}.ts complete --`; - it.runIf(!shouldSkipTest)('should complete cli options', async () => { + it('should complete cli options', async () => { const output = await runCommand(`${commandPrefix}`); expect(output).toMatchSnapshot(); }); - describe.runIf(!shouldSkipTest)('cli option completion tests', () => { + describe('cli option completion tests', () => { const optionTests = [ { partial: '--p', expected: '--port' }, { partial: '-p', expected: '-p' }, // Test short flag completion @@ -48,7 +45,7 @@ describe.each(cliTools)('cli completion tests for %s', (cliTool) => { ); }); - describe.runIf(!shouldSkipTest)('cli option exclusion tests', () => { + describe('cli option exclusion tests', () => { const alreadySpecifiedTests = [ { specified: '--config', shouldNotContain: '--config' }, ]; @@ -63,7 +60,7 @@ describe.each(cliTools)('cli completion tests for %s', (cliTool) => { ); }); - describe.runIf(!shouldSkipTest)('cli option value handling', () => { + describe('cli option value handling', () => { it('should resolve port value correctly', async () => { const command = `${commandPrefix} dev --port=3`; const output = await runCommand(command); @@ -89,7 +86,7 @@ describe.each(cliTools)('cli completion tests for %s', (cliTool) => { }); }); - describe.runIf(!shouldSkipTest)('boolean option handling', () => { + describe('boolean option handling', () => { it('should complete subcommands and arguments after boolean options', async () => { const command = `${commandPrefix} dev --verbose ""`; const output = await runCommand(command); @@ -118,7 +115,7 @@ describe.each(cliTools)('cli completion tests for %s', (cliTool) => { it('should not interfere with option completion after boolean options', async () => { const command = `${commandPrefix} dev --verbose --h`; const output = await runCommand(command); - // Should complete subcommands that start with 's' even after a boolean option + // Should complete options that start with '--h' even after a boolean option expect(output).toContain('--host'); }); }); From 4d50985e8396596d2842a8e42dcbf0f671c60b7c Mon Sep 17 00:00:00 2001 From: John Gee Date: Wed, 3 Jun 2026 19:04:28 +1200 Subject: [PATCH 2/3] Reviewing Commander tests without refactoring anything yet --- examples/demo.commander.ts | 83 ++++++++++++++-------------- tests/__snapshots__/cli.test.ts.snap | 10 ---- tests/cli.test.ts | 33 +++++++---- 3 files changed, 63 insertions(+), 63 deletions(-) diff --git a/examples/demo.commander.ts b/examples/demo.commander.ts index 0a3793e..067b2e6 100644 --- a/examples/demo.commander.ts +++ b/examples/demo.commander.ts @@ -78,48 +78,49 @@ program const completion = tab(program); // Configure custom completions -for (const command of completion.commands.values()) { - if (command.value === 'lint') { - // Note: Direct handler assignment is not supported in the current API - // Custom completion logic would need to be implemented differently - } +// This needs rewriting with config passed into tab(program)? +// for (const command of completion.commands.values()) { +// if (command.value === 'lint') { +// // Note: Direct handler assignment is not supported in the current API +// // Custom completion logic would need to be implemented differently +// } - for (const [option, config] of command.options.entries()) { - if (option === '--port') { - config.handler = () => { - return [ - { value: '3000', description: 'Default port' }, - { value: '8080', description: 'Alternative port' }, - ]; - }; - } - if (option === '--host') { - config.handler = () => { - return [ - { value: 'localhost', description: 'Local development' }, - { value: '0.0.0.0', description: 'All interfaces' }, - ]; - }; - } - if (option === '--mode') { - config.handler = () => { - return [ - { value: 'development', description: 'Development mode' }, - { value: 'production', description: 'Production mode' }, - { value: 'test', description: 'Test mode' }, - ]; - }; - } - if (option === '--config') { - config.handler = () => { - return [ - { value: 'config.json', description: 'JSON config file' }, - { value: 'config.yaml', description: 'YAML config file' }, - ]; - }; - } - } -} +// for (const [option, config] of command.options.entries()) { +// if (option === '--port') { +// config.handler = () => { +// return [ +// { value: '3000', description: 'Default port' }, +// { value: '8080', description: 'Alternative port' }, +// ]; +// }; +// } +// if (option === '--host') { +// config.handler = () => { +// return [ +// { value: 'localhost', description: 'Local development' }, +// { value: '0.0.0.0', description: 'All interfaces' }, +// ]; +// }; +// } +// if (option === '--mode') { +// config.handler = () => { +// return [ +// { value: 'development', description: 'Development mode' }, +// { value: 'production', description: 'Production mode' }, +// { value: 'test', description: 'Test mode' }, +// ]; +// }; +// } +// if (option === '--config') { +// config.handler = () => { +// return [ +// { value: 'config.json', description: 'JSON config file' }, +// { value: 'config.yaml', description: 'YAML config file' }, +// ]; +// }; +// } +// } +// } // Parse command line arguments program.parse(); diff --git a/tests/__snapshots__/cli.test.ts.snap b/tests/__snapshots__/cli.test.ts.snap index 7a55b16..30ed2c7 100644 --- a/tests/__snapshots__/cli.test.ts.snap +++ b/tests/__snapshots__/cli.test.ts.snap @@ -660,16 +660,6 @@ exports[`cli completion tests for commander > cli option value handling > should " `; -exports[`cli completion tests for commander > cli option value handling > should resolve config option values correctly 1`] = ` -":4 -" -`; - -exports[`cli completion tests for commander > cli option value handling > should resolve port value correctly 1`] = ` -":4 -" -`; - exports[`cli completion tests for commander > should complete cli options 1`] = ` "dev Start dev server serve Start the server diff --git a/tests/cli.test.ts b/tests/cli.test.ts index 9d83fbe..c5c9ad4 100644 --- a/tests/cli.test.ts +++ b/tests/cli.test.ts @@ -13,8 +13,8 @@ function runCommand(command: string): Promise { }); } -const cliTools = ['t', 'citty', 'cac', 'commander']; -// const cliTools = ['commander']; // TEMP For quick testing of commander-specific behavior, uncomment this line and comment out the line above. +// const cliTools = ['t', 'citty', 'cac', 'commander']; +const cliTools = ['commander']; // TEMP For quick testing of commander-specific behavior, uncomment this line and comment out the line above. describe.each(cliTools)('cli completion tests for %s', (cliTool) => { // For Commander, we need to skip most of the tests since it handles completion differently @@ -61,23 +61,31 @@ describe.each(cliTools)('cli completion tests for %s', (cliTool) => { }); describe('cli option value handling', () => { - it('should resolve port value correctly', async () => { - const command = `${commandPrefix} dev --port=3`; - const output = await runCommand(command); - expect(output).toMatchSnapshot(); - }); + it.runIf(!shouldSkipTest)( + 'should resolve port value correctly', + async () => { + const command = `${commandPrefix} dev --port=3`; + const output = await runCommand(command); + expect(output).toMatchSnapshot(); + } + ); + // Note: on all frameworks, --config is suggested again, which is inconsistent with test title. + // (Which I think is simple behaviour! Do not know whether option allowed to be specified more than once...) it('should not show duplicate options', async () => { const command = `${commandPrefix} --config vite.config.js --`; const output = await runCommand(command); expect(output).toMatchSnapshot(); }); - it('should resolve config option values correctly', async () => { - const command = `${commandPrefix} --config vite.config`; - const output = await runCommand(command); - expect(output).toMatchSnapshot(); - }); + it.runIf(!shouldSkipTest)( + 'should resolve config option values correctly', + async () => { + const command = `${commandPrefix} --config vite.config`; + const output = await runCommand(command); + expect(output).toMatchSnapshot(); + } + ); it('should handle unknown options with no completions', async () => { const command = `${commandPrefix} --unknownoption`; @@ -225,6 +233,7 @@ describe.each(cliTools)('cli completion tests for %s', (cliTool) => { expect(output).toMatchSnapshot(); }); + // Note: on all frameworks, --config is suggested again, which is inconsistent with test title. it('should not suggest --config after it has been used', async () => { const command = `${commandPrefix} --config vite.config.ts --`; const output = await runCommand(command); From 4b17ba3ed8ed977516b95f604cc4cc8e5a3acf65 Mon Sep 17 00:00:00 2001 From: John Gee Date: Wed, 3 Jun 2026 22:53:33 +1200 Subject: [PATCH 3/3] Clarify comment --- tests/cli.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/cli.test.ts b/tests/cli.test.ts index c5c9ad4..1c86a5b 100644 --- a/tests/cli.test.ts +++ b/tests/cli.test.ts @@ -17,8 +17,7 @@ function runCommand(command: string): Promise { const cliTools = ['commander']; // TEMP For quick testing of commander-specific behavior, uncomment this line and comment out the line above. describe.each(cliTools)('cli completion tests for %s', (cliTool) => { - // For Commander, we need to skip most of the tests since it handles completion differently - // Legacy comment? Reenabling tests... + // Commander has not been refactored yet for new way of passing in custom completion handlers. const shouldSkipTest = cliTool === 'commander'; const commandPrefix = `pnpm tsx examples/demo.${cliTool}.ts complete --`;