Skip to content

Commit ab81a4b

Browse files
leo-arclay-goodclaude
authored
fix(completions): stop Fish completions falling back to filenames (#1199)
* fix(completions): suppress filesystem fallback in Fish completions * fix(completions): force files back on for path positionals in Fish Fish never restores filesystem completion once a matching rule sets --no-files, so a path positional needs an explicit --force-files rule. Without it, `openspec store register <TAB>` lost file completion because the sibling subcommand rules in the same context now carry -f. Also drop retired "context store" vocabulary from the test fixtures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(completions): preserve workset member paths in fish * fix(completions): force Fish path fallback * fix(completions): harden Fish option handling * fix(completions): scope Fish path fallback * fix(completions): match Fish command paths exactly * fix(completions): skip parent options in Fish paths --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 109f81f commit ab81a4b

6 files changed

Lines changed: 479 additions & 101 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@fission-ai/openspec': patch
3+
---
4+
5+
Improve Fish completions so command, subcommand, flag, and indexed positional completions no longer fall back to filesystem suggestions unless the target is a real path.

src/core/completions/command-registry.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [
291291
name: 'path',
292292
description: 'Directory to use for the store',
293293
takesValue: true,
294+
completionType: 'path',
294295
},
295296
{
296297
name: 'init-git',
@@ -383,6 +384,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [
383384
name: 'code-workspace',
384385
description: 'Also write a VS Code workspace file for the set',
385386
takesValue: true,
387+
completionType: 'path',
386388
},
387389
{
388390
name: 'force',
@@ -414,6 +416,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [
414416
description:
415417
'Member folder as <path> or <name>=<path>; repeatable, first is the primary',
416418
takesValue: true,
419+
completionType: 'path',
417420
},
418421
{
419422
name: 'tool',

src/core/completions/generators/fish-generator.ts

Lines changed: 113 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -15,36 +15,31 @@ export class FishGenerator implements CompletionGenerator {
1515
* @returns Fish completion script as a string
1616
*/
1717
generate(commands: CommandDefinition[]): string {
18-
// Build top-level commands using push() for loop clarity
1918
const topLevelLines: string[] = [];
2019
for (const cmd of commands) {
2120
topLevelLines.push(`# ${cmd.name} command`);
2221
topLevelLines.push(
23-
`complete -c openspec -n '__fish_openspec_no_subcommand' -a '${cmd.name}' -d '${this.escapeDescription(cmd.description)}'`
22+
`complete -c openspec -n '__fish_openspec_no_subcommand' -f -a '${cmd.name}' -d '${this.escapeDescription(cmd.description)}'`
2423
);
2524
}
2625
const topLevelCommands = topLevelLines.join('\n');
2726

28-
// Build command-specific completions using push() for loop clarity
2927
const commandCompletionLines: string[] = [];
3028
for (const cmd of commands) {
3129
commandCompletionLines.push(...this.generateCommandCompletions(cmd));
3230
commandCompletionLines.push('');
3331
}
3432
const commandCompletions = commandCompletionLines.join('\n');
3533

36-
// Static helper functions from template
3734
const helperFunctions = FISH_STATIC_HELPERS;
38-
39-
// Dynamic completion helpers from template
4035
const dynamicHelpers = FISH_DYNAMIC_HELPERS;
4136

42-
// Assemble final script with template literal
4337
return `# Fish completion script for OpenSpec CLI
4438
# Auto-generated - do not edit manually
4539
4640
${helperFunctions}
4741
${dynamicHelpers}
42+
complete -c openspec -l no-color -f -d 'Disable color output'
4843
${topLevelCommands}
4944
5045
${commandCompletions}`;
@@ -56,43 +51,67 @@ ${commandCompletions}`;
5651
private generateCommandCompletions(cmd: CommandDefinition): string[] {
5752
const lines: string[] = [];
5853

59-
// If command has subcommands
6054
if (cmd.subcommands && cmd.subcommands.length > 0) {
61-
// Add subcommand completions
55+
const commandCondition = this.commandPathCondition([cmd.name]);
56+
const parentValueFlags = this.collectValueFlags(cmd.flags);
57+
const noSubcommandCondition = cmd.subcommands
58+
.map((subcmd) => `not ${this.commandPathCondition([cmd.name, subcmd.name], parentValueFlags)}`)
59+
.join('; and ');
6260
for (const subcmd of cmd.subcommands) {
6361
lines.push(
64-
`complete -c openspec -n '__fish_openspec_using_subcommand ${cmd.name}; and not __fish_openspec_using_subcommand ${subcmd.name}' -a '${subcmd.name}' -d '${this.escapeDescription(subcmd.description)}'`
62+
`complete -c openspec -n '${commandCondition}; and ${noSubcommandCondition}' -f -a '${subcmd.name}' -d '${this.escapeDescription(subcmd.description)}'`
6563
);
6664
}
6765
lines.push('');
6866

69-
// Add flags for parent command
7067
for (const flag of cmd.flags) {
71-
lines.push(...this.generateFlagCompletion(flag, `__fish_openspec_using_subcommand ${cmd.name}`));
68+
lines.push(...this.generateFlagCompletion(flag, commandCondition));
7269
}
7370

74-
// Add completions for each subcommand
7571
for (const subcmd of cmd.subcommands) {
72+
const subcommandCondition = this.commandPathCondition([cmd.name, subcmd.name], parentValueFlags);
7673
lines.push(`# ${cmd.name} ${subcmd.name} flags`);
74+
lines.push(`complete -c openspec -n '${subcommandCondition}' -f`);
7775
for (const flag of subcmd.flags) {
78-
lines.push(...this.generateFlagCompletion(flag, `__fish_openspec_using_subcommand ${cmd.name}; and __fish_openspec_using_subcommand ${subcmd.name}`));
76+
lines.push(...this.generateFlagCompletion(flag, subcommandCondition));
7977
}
8078

81-
// Add positional completions for subcommand
82-
if (subcmd.acceptsPositional) {
83-
lines.push(...this.generatePositionalCompletion(subcmd.positionalType, `__fish_openspec_using_subcommand ${cmd.name}; and __fish_openspec_using_subcommand ${subcmd.name}`));
79+
if (subcmd.positionals?.length) {
80+
lines.push(
81+
...this.generateIndexedPositionalCompletions(
82+
subcmd.positionals,
83+
subcommandCondition,
84+
this.collectValueFlags(cmd.flags, subcmd.flags),
85+
2
86+
)
87+
);
88+
} else if (subcmd.acceptsPositional) {
89+
lines.push(
90+
...this.generatePositionalCompletion(
91+
subcmd.positionalType,
92+
subcommandCondition
93+
)
94+
);
8495
}
8596
}
8697
} else {
87-
// Command without subcommands
8898
lines.push(`# ${cmd.name} flags`);
99+
lines.push(`complete -c openspec -n '__fish_openspec_using_command_path ${cmd.name}' -f`);
89100
for (const flag of cmd.flags) {
90-
lines.push(...this.generateFlagCompletion(flag, `__fish_openspec_using_subcommand ${cmd.name}`));
101+
lines.push(...this.generateFlagCompletion(flag, `__fish_openspec_using_command_path ${cmd.name}`));
91102
}
92103

93-
// Add positional completions
94-
if (cmd.acceptsPositional) {
95-
lines.push(...this.generatePositionalCompletion(cmd.positionalType, `__fish_openspec_using_subcommand ${cmd.name}`));
104+
if (cmd.positionals?.length) {
105+
lines.push(
106+
...this.generateIndexedPositionalCompletions(
107+
cmd.positionals,
108+
`__fish_openspec_using_command_path ${cmd.name}`,
109+
this.collectValueFlags(cmd.flags),
110+
1
111+
)
112+
);
113+
} else if (cmd.acceptsPositional) {
114+
lines.push(...this.generatePositionalCompletion(cmd.positionalType, `__fish_openspec_using_command_path ${cmd.name}`));
96115
}
97116
}
98117

@@ -104,44 +123,31 @@ ${commandCompletions}`;
104123
*/
105124
private generateFlagCompletion(flag: FlagDefinition, condition: string): string[] {
106125
const lines: string[] = [];
107-
const longFlag = `--${flag.name}`;
108-
const shortFlag = flag.short ? `-${flag.short}` : undefined;
126+
const description = this.escapeDescription(flag.description);
127+
const shortFlag = flag.short ? `-s ${flag.short} ` : '';
128+
const flagOptions = `${shortFlag}-l ${flag.name}`;
109129

110130
if (flag.takesValue && flag.values) {
111-
// Flag with enum values
112131
for (const value of flag.values) {
113-
if (shortFlag) {
114-
lines.push(
115-
`complete -c openspec -n '${condition}' -s ${flag.short} -l ${flag.name} -a '${value}' -d '${this.escapeDescription(flag.description)}'`
116-
);
117-
} else {
118-
lines.push(
119-
`complete -c openspec -n '${condition}' -l ${flag.name} -a '${value}' -d '${this.escapeDescription(flag.description)}'`
120-
);
121-
}
122-
}
123-
} else if (flag.takesValue) {
124-
// Flag that takes a value but no specific values defined
125-
if (shortFlag) {
126-
lines.push(
127-
`complete -c openspec -n '${condition}' -s ${flag.short} -l ${flag.name} -r -d '${this.escapeDescription(flag.description)}'`
128-
);
129-
} else {
130132
lines.push(
131-
`complete -c openspec -n '${condition}' -l ${flag.name} -r -d '${this.escapeDescription(flag.description)}'`
133+
`complete -c openspec -n '${condition}' ${flagOptions} -r -f -a '${value}' -d '${description}'`
132134
);
133135
}
134-
} else {
135-
// Boolean flag
136-
if (shortFlag) {
137-
lines.push(
138-
`complete -c openspec -n '${condition}' -s ${flag.short} -l ${flag.name} -d '${this.escapeDescription(flag.description)}'`
139-
);
140-
} else {
136+
} else if (flag.takesValue) {
137+
lines.push(`complete -c openspec -n '${condition}' ${flagOptions} -r -f -d '${description}'`);
138+
if (flag.completionType === 'path') {
139+
const optionNames = [`--${flag.name}`, ...(flag.short ? [`-${flag.short}`] : [])];
141140
lines.push(
142-
`complete -c openspec -n '${condition}' -l ${flag.name} -d '${this.escapeDescription(flag.description)}'`
141+
`complete -c openspec -n '${condition}; and __fish_openspec_completing_option_value ${optionNames.join(' ')}' ${flagOptions} -r -F -d '${description}'`
143142
);
143+
if (flag.short) {
144+
lines.push(
145+
`complete -c openspec -n '${condition}' ${flagOptions} -r -f -a '(__fish_openspec_complete_attached_short_path -${flag.short})' -d '${description}'`
146+
);
147+
}
144148
}
149+
} else {
150+
lines.push(`complete -c openspec -n '${condition}' ${flagOptions} -f -d '${description}'`);
145151
}
146152

147153
return lines;
@@ -170,22 +176,73 @@ ${commandCompletions}`;
170176
lines.push(`complete -c openspec -n '${condition}' -a 'zsh bash fish powershell' -f`);
171177
break;
172178
case 'path':
173-
// Fish automatically completes files, no need to specify
179+
// -F re-enables filesystem completion: sibling rules in the same
180+
// context carry -f, and Fish never restores files without --force-files.
181+
lines.push(`complete -c openspec -n '${condition}' -F`);
174182
break;
183+
default:
184+
lines.push(`complete -c openspec -n '${condition}' -f`);
185+
break;
186+
}
187+
188+
return lines;
189+
}
190+
191+
/**
192+
* Generate indexed positional completions.
193+
*/
194+
private generateIndexedPositionalCompletions(
195+
positionals: NonNullable<CommandDefinition['positionals']>,
196+
condition: string,
197+
valueFlags: string[],
198+
depth: number
199+
): string[] {
200+
const lines: string[] = [];
201+
202+
for (const [index, positional] of positionals.entries()) {
203+
const indexCondition = `${condition}; and __fish_openspec_positional_index ${index} ${depth}${valueFlags.length ? ` ${valueFlags.join(' ')}` : ''}`;
204+
lines.push(...this.generatePositionalCompletion(positional.type, indexCondition));
175205
}
176206

177207
return lines;
178208
}
179209

210+
/**
211+
* Collect the long and short names for flags that consume the next token.
212+
*/
213+
private collectValueFlags(...flagGroups: FlagDefinition[][]): string[] {
214+
const flags = new Set<string>();
215+
216+
for (const group of flagGroups) {
217+
for (const flag of group) {
218+
if (!flag.takesValue) {
219+
continue;
220+
}
221+
222+
flags.add(`--${flag.name}`);
223+
if (flag.short) {
224+
flags.add(`-${flag.short}`);
225+
}
226+
}
227+
}
228+
229+
return [...flags];
230+
}
231+
232+
/**
233+
* Build a Fish condition that matches command words in their actual slots.
234+
*/
235+
private commandPathCondition(path: string[], valueFlags: string[] = []): string {
236+
const valueFlagArguments = valueFlags.length ? ` -- ${valueFlags.join(' ')}` : '';
237+
return `__fish_openspec_using_command_path ${path.join(' ')}${valueFlagArguments}`;
238+
}
180239

181240
/**
182241
* Escape description text for Fish
183242
*/
184243
private escapeDescription(description: string): string {
185244
return description
186-
.replace(/\\/g, '\\\\') // Backslashes first
187-
.replace(/'/g, "\\'") // Single quotes
188-
.replace(/\$/g, '\\$') // Dollar signs (prevents $())
189-
.replace(/`/g, '\\`'); // Backticks
245+
.replace(/\\/g, '\\\\') // Backslashes first
246+
.replace(/'/g, "\\'"); // Single quotes
190247
}
191248
}

0 commit comments

Comments
 (0)