fix: positional argument completion broken after flags (#111)

* fix: positional argument completion broken after flags

Three interacting bugs prevented positional argument completion when
flags preceded the argument (e.g., 'mycli logs -f <TAB>'):

1. handlePositionalCompletion received raw args including flags, so its
   positional index calculation was inflated. Fix: call stripOptions()
   on previousArgs before counting positions.

2. parse() had an early return after detecting a boolean flag that
   skipped both handleCommandCompletion and handlePositionalCompletion.
   Fix: remove the early return to let control fall through.

3. The commander adapter marked ALL options as isBoolean=true, even
   value-taking ones like '--output <format>'. This caused stripOptions
   to not skip the value argument, further corrupting the positional
   index. Fix: detect '<' or '[' in flag syntax and register with a
   no-op handler to force isBoolean=false.

Fixes #110

* style: revert cosmetic linter changes, keep only meaningful fix

Remove import reordering, trailing comma additions, arrow function
conversion, and test.each reformatting that were introduced by a
linter not used by the project. Retains only the actual bug fix
for positional completion after flags.

* pnpm changeset

---------

Co-authored-by: AmirSa12 <amirhosseinpr184@gmail.com>

Author: murrayju

.changeset/chatty-wings-melt.md

@@ -0,0 +1,5 @@
+---
+'@bomb.sh/tab': patch
+---
+
+fix positional argument completion broken after flags

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 `<value>` 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
+        );
       }
     }
   }

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);

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
 "
 `;
 

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