diff --git a/src/commander.ts b/src/commander.ts index 0001de4..09799e7 100644 --- a/src/commander.ts +++ b/src/commander.ts @@ -101,6 +101,55 @@ export default function tab(instance: CommanderCommand): RootCommand { return t; } +/** + * Detect whether a commander option flag expects a value argument. + * Options with `` or `[value]` in their flags are value-taking. + */ +function optionTakesValue(flags: string): boolean { + return flags.includes('<') || flags.includes('['); +} + +/** + * Register a commander option with the tab library, correctly setting + * isBoolean based on whether the option takes a value. + * + * The tab Command.option() method infers isBoolean from the argument types: + * - string arg → alias, isBoolean=true + * - function arg → handler, isBoolean=false + * So for value-taking options with an alias, we pass a no-op handler + * and the alias separately to get isBoolean=false. + */ +function registerOption( + tabCommand: { + option: ( + value: string, + description: string, + handlerOrAlias?: ((...args: unknown[]) => void) | string, + alias?: string + ) => unknown; + }, + flags: string, + longFlag: string, + description: string, + shortFlag?: string +): void { + const takesValue = optionTakesValue(flags); + if (shortFlag) { + if (takesValue) { + // Pass a no-op handler to force isBoolean=false, with alias as 4th arg + tabCommand.option(longFlag, description, () => {}, shortFlag); + } else { + tabCommand.option(longFlag, description, shortFlag); + } + } else { + if (takesValue) { + tabCommand.option(longFlag, description, () => {}); + } else { + tabCommand.option(longFlag, description); + } + } +} + function processRootCommand(command: CommanderCommand): void { // Add root command options to the root t instance for (const option of command.options) { @@ -110,11 +159,7 @@ function processRootCommand(command: CommanderCommand): void { const longFlag = flags.match(/--([a-zA-Z0-9-]+)/)?.[1]; if (longFlag) { - if (shortFlag) { - t.option(longFlag, option.description || '', shortFlag); - } else { - t.option(longFlag, option.description || ''); - } + registerOption(t, flags, longFlag, option.description || '', shortFlag); } } } @@ -141,11 +186,13 @@ function processSubcommands(rootCommand: CommanderCommand): void { const longFlag = flags.match(/--([a-zA-Z0-9-]+)/)?.[1]; if (longFlag) { - if (shortFlag) { - command.option(longFlag, option.description || '', shortFlag); - } else { - command.option(longFlag, option.description || ''); - } + registerOption( + command, + flags, + longFlag, + option.description || '', + shortFlag + ); } } } diff --git a/src/t.ts b/src/t.ts index 8dabcaa..b22cd9c 100644 --- a/src/t.ts +++ b/src/t.ts @@ -349,9 +349,11 @@ export class RootCommand extends Command { // positional argument completion private handlePositionalCompletion(command: Command, previousArgs: string[]) { + // Strip options so flags don't inflate the positional index + const strippedArgs = this.stripOptions(previousArgs); // current argument position (subtract command name) const commandParts = command.value.split(' ').length; - const currentArgIndex = Math.max(0, previousArgs.length - commandParts); + const currentArgIndex = Math.max(0, strippedArgs.length - commandParts); const argumentEntries = Array.from(command.arguments.entries()); if (argumentEntries.length > 0) { @@ -438,20 +440,10 @@ export class RootCommand extends Command { lastPrevArg ); } else { - if (lastPrevArg?.startsWith('-') && toComplete === '' && endsWithSpace) { - let option = this.findOption(this, lastPrevArg); - if (!option) { - for (const [, command] of this.commands) { - option = this.findOption(command, lastPrevArg); - if (option) break; - } - } - - if (option && option.isBoolean) { - this.complete(toComplete); - return; - } - } + // Note: we intentionally do NOT early-return after detecting a boolean + // flag. The previous code called this.complete(toComplete) and returned + // here, which skipped positional argument completion. After a boolean + // flag like -f, the user may still be completing a positional argument. if (this.shouldCompleteCommands(toComplete)) { this.handleCommandCompletion(previousArgs, toComplete); diff --git a/tests/__snapshots__/cli.test.ts.snap b/tests/__snapshots__/cli.test.ts.snap index 4c5bdb9..dc5cb94 100644 --- a/tests/__snapshots__/cli.test.ts.snap +++ b/tests/__snapshots__/cli.test.ts.snap @@ -506,12 +506,18 @@ exports[`cli completion tests for citty > root command argument tests > should c "dev Start dev server copy Copy files lint Lint project +my-app My application +my-lib My library +my-tool My tool :4 " `; exports[`cli completion tests for citty > root command argument tests > should complete root command project argument with options and partial input 1`] = ` -":4 +"my-app My application +my-lib My library +my-tool My tool +:4 " `; @@ -620,23 +626,6 @@ my-tool My tool " `; -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 > short flag handling > should handle global short flags 1`] = ` -"-c specify config file -:4 -" -`; - -exports[`cli completion tests for commander > should complete cli options 1`] = ` -"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 @@ -846,12 +835,18 @@ exports[`cli completion tests for t > root command argument tests > should compl serve Start the server copy Copy files lint Lint project +my-app My application +my-lib My library +my-tool My tool :4 " `; exports[`cli completion tests for t > root command argument tests > should complete root command project argument with options and partial input 1`] = ` -":4 +"my-app My application +my-lib My library +my-tool My tool +:4 " `; diff --git a/tests/cli.test.ts b/tests/cli.test.ts index 0822ee1..17222d7 100644 --- a/tests/cli.test.ts +++ b/tests/cli.test.ts @@ -90,18 +90,22 @@ describe.each(cliTools)('cli completion tests for %s', (cliTool) => { }); describe.runIf(!shouldSkipTest)('boolean option handling', () => { - it('should not provide value completions for boolean options', async () => { + it('should complete subcommands and arguments after boolean options', async () => { const command = `${commandPrefix} dev --verbose ""`; const output = await runCommand(command); - // Boolean options should return just the directive, no completions - expect(output.trim()).toBe(':4'); + // After a boolean option, should show subcommands/arguments (not flag values) + expect(output).toContain('build'); + expect(output).toContain('start'); + expect(output).not.toContain('verbose'); }); - it('should not provide value completions for short boolean options', async () => { + it('should complete subcommands and arguments after short boolean options', async () => { const command = `${commandPrefix} dev -v ""`; const output = await runCommand(command); - // Boolean options should return just the directive, no completions - expect(output.trim()).toBe(':4'); + // After a short boolean option, should show subcommands/arguments (not flag values) + expect(output).toContain('build'); + expect(output).toContain('start'); + expect(output).not.toContain('verbose'); }); it('should not interfere with command completion after boolean options', async () => { @@ -124,24 +128,27 @@ describe.each(cliTools)('cli completion tests for %s', (cliTool) => { // This tests the case: option('quiet', 'Suppress output') const command = `${commandPrefix} dev --quiet ""`; const output = await runCommand(command); - // Should be treated as boolean flag (no value completion) - expect(output.trim()).toBe(':4'); + // Should be treated as boolean flag — shows subcommands, not flag values + expect(output).toContain('build'); + expect(output).not.toContain('quiet'); }); it('should handle option with alias only as boolean flag', async () => { // This tests the case: option('verbose', 'Enable verbose', 'v') const command = `${commandPrefix} dev --verbose ""`; const output = await runCommand(command); - // Should be treated as boolean flag (no value completion) - expect(output.trim()).toBe(':4'); + // Should be treated as boolean flag — shows subcommands, not flag values + expect(output).toContain('build'); + expect(output).not.toContain('verbose'); }); it('should handle option with alias only (short flag) as boolean flag', async () => { // This tests the short flag version: -v instead of --verbose const command = `${commandPrefix} dev -v ""`; const output = await runCommand(command); - // Should be treated as boolean flag (no value completion) - expect(output.trim()).toBe(':4'); + // Should be treated as boolean flag — shows subcommands, not flag values + expect(output).toContain('build'); + expect(output).not.toContain('verbose'); }); it('should handle option with handler only as value option', async () => {