-
Notifications
You must be signed in to change notification settings - Fork 990
feature/bash_fish_power_shells_completions #401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e89f79e
ad53abc
07cad96
0bac2e3
5524026
9dd9962
f69feab
d9b4ae6
71f434b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Change Proposal: Extend Shell Completions | ||
|
|
||
| ## Why | ||
|
|
||
| Zsh completions provide an excellent developer experience, but many developers use bash, fish, or PowerShell. Extending completion support to these shells removes friction for the majority of developers who don't use Zsh. | ||
|
|
||
| ## What Changes | ||
|
|
||
| This change adds bash, fish, and PowerShell completion support following the same architectural patterns, documentation methodology, and testing rigor established for Zsh completions. | ||
|
|
||
| ## Deltas | ||
|
|
||
| - **Spec:** `cli-completion` | ||
| - **Operation:** MODIFIED | ||
| - **Description:** Extend completion generation, installation, and testing requirements to support bash, fish, and PowerShell while maintaining the existing Zsh implementation and architectural patterns |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # Implementation Tasks | ||
|
|
||
| ## Phase 1: Foundation and Bash Support | ||
|
|
||
| - [x] Update `SupportedShell` type in `src/utils/shell-detection.ts` to include `'bash' | 'fish' | 'powershell'` | ||
| - [x] Extend shell detection logic to recognize bash, fish, and PowerShell from environment variables | ||
| - [x] Create `src/core/completions/generators/bash-generator.ts` implementing `CompletionGenerator` interface | ||
| - [x] Create `src/core/completions/installers/bash-installer.ts` implementing `CompletionInstaller` interface | ||
| - [x] Update `CompletionFactory.createGenerator()` to support bash | ||
| - [x] Update `CompletionFactory.createInstaller()` to support bash | ||
| - [x] Create test file `test/core/completions/generators/bash-generator.test.ts` mirroring zsh test structure | ||
| - [x] Create test file `test/core/completions/installers/bash-installer.test.ts` mirroring zsh test structure | ||
| - [x] Verify bash completions work manually: `openspec completion install bash && exec bash` | ||
|
|
||
| ## Phase 2: Fish Support | ||
|
|
||
| - [x] Create `src/core/completions/generators/fish-generator.ts` implementing `CompletionGenerator` interface | ||
| - [x] Create `src/core/completions/installers/fish-installer.ts` implementing `CompletionInstaller` interface | ||
| - [x] Update `CompletionFactory.createGenerator()` to support fish | ||
| - [x] Update `CompletionFactory.createInstaller()` to support fish | ||
| - [x] Create test file `test/core/completions/generators/fish-generator.test.ts` | ||
| - [x] Create test file `test/core/completions/installers/fish-installer.test.ts` | ||
| - [x] Verify fish completions work manually: `openspec completion install fish` | ||
|
|
||
| ## Phase 3: PowerShell Support | ||
|
|
||
| - [x] Create `src/core/completions/generators/powershell-generator.ts` implementing `CompletionGenerator` interface | ||
| - [x] Create `src/core/completions/installers/powershell-installer.ts` implementing `CompletionInstaller` interface | ||
| - [x] Update `CompletionFactory.createGenerator()` to support powershell | ||
| - [x] Update `CompletionFactory.createInstaller()` to support powershell | ||
| - [x] Create test file `test/core/completions/generators/powershell-generator.test.ts` | ||
| - [x] Create test file `test/core/completions/installers/powershell-installer.test.ts` | ||
| - [x] Verify PowerShell completions work manually on Windows or macOS PowerShell | ||
|
|
||
| ## Phase 4: Documentation and Testing | ||
|
|
||
| - [x] Update `CLAUDE.md` or relevant documentation to mention all four supported shells | ||
| - [x] Add cross-shell consistency test verifying all shells support same commands | ||
| - [x] Run `pnpm test` to ensure all tests pass | ||
| - [x] Run `pnpm run build` to verify TypeScript compilation | ||
| - [x] Test all shells on different platforms (Linux for bash/fish/zsh, Windows/macOS for PowerShell) | ||
|
|
||
| ## Phase 5: Validation and Cleanup | ||
|
|
||
| - [x] Run `openspec validate extend-shell-completions --strict` and resolve all issues | ||
| - [x] Update error messages to list all four supported shells | ||
| - [x] Verify `openspec completion --help` documentation is current | ||
| - [x] Test auto-detection works for all shells | ||
| - [x] Ensure uninstall works cleanly for all shells |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,214 @@ | ||
| import { CompletionGenerator, CommandDefinition, FlagDefinition } from '../types.js'; | ||
|
|
||
| /** | ||
| * Generates Bash completion scripts for the OpenSpec CLI. | ||
| * Follows Bash completion conventions using complete builtin and COMPREPLY array. | ||
| */ | ||
| export class BashGenerator implements CompletionGenerator { | ||
| readonly shell = 'bash' as const; | ||
|
|
||
| /** | ||
| * Generate a Bash completion script | ||
| * | ||
| * @param commands - Command definitions to generate completions for | ||
| * @returns Bash completion script as a string | ||
| */ | ||
| generate(commands: CommandDefinition[]): string { | ||
| const script: string[] = []; | ||
|
|
||
| // Header comment | ||
| script.push('# Bash completion script for OpenSpec CLI'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I missed the use of ^ something like above (please verify) LMK if there's a strong reasoning behind this approach.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same with |
||
| script.push('# Auto-generated - do not edit manually'); | ||
| script.push(''); | ||
|
|
||
| // Main completion function | ||
| script.push('_openspec_completion() {'); | ||
| script.push(' local cur prev words cword'); | ||
| script.push(''); | ||
| script.push(' # Use _init_completion if available (from bash-completion package)'); | ||
| script.push(' # Otherwise, fall back to manual initialization'); | ||
| script.push(' if declare -F _init_completion >/dev/null 2>&1; then'); | ||
| script.push(' _init_completion || return'); | ||
| script.push(' else'); | ||
| script.push(' # Manual fallback when bash-completion is not installed'); | ||
| script.push(' COMPREPLY=()'); | ||
| script.push(' _get_comp_words_by_ref -n : cur prev words cword 2>/dev/null || {'); | ||
| script.push(' cur="${COMP_WORDS[COMP_CWORD]}"'); | ||
| script.push(' prev="${COMP_WORDS[COMP_CWORD-1]}"'); | ||
| script.push(' words=("${COMP_WORDS[@]}")'); | ||
| script.push(' cword=$COMP_CWORD'); | ||
| script.push(' }'); | ||
| script.push(' fi'); | ||
| script.push(''); | ||
| script.push(' local cmd="${words[1]}"'); | ||
| script.push(' local subcmd="${words[2]}"'); | ||
| script.push(''); | ||
|
|
||
| // Top-level commands | ||
| script.push(' # Top-level commands'); | ||
| script.push(' if [[ $cword -eq 1 ]]; then'); | ||
| script.push(' local commands="' + commands.map(c => this.escapeCommandName(c.name)).join(' ') + '"'); | ||
| script.push(' COMPREPLY=($(compgen -W "$commands" -- "$cur"))'); | ||
| script.push(' return 0'); | ||
| script.push(' fi'); | ||
| script.push(''); | ||
|
|
||
| // Command-specific completion | ||
| script.push(' # Command-specific completion'); | ||
| script.push(' case "$cmd" in'); | ||
|
|
||
| for (const cmd of commands) { | ||
| script.push(` ${cmd.name})`); | ||
| script.push(...this.generateCommandCase(cmd, ' ')); | ||
| script.push(' ;;'); | ||
| } | ||
|
|
||
| script.push(' esac'); | ||
| script.push(''); | ||
| script.push(' return 0'); | ||
| script.push('}'); | ||
| script.push(''); | ||
|
|
||
| // Helper functions for dynamic completions | ||
| script.push(...this.generateDynamicCompletionHelpers()); | ||
|
|
||
| // Register the completion function | ||
| script.push('complete -F _openspec_completion openspec'); | ||
| script.push(''); | ||
|
|
||
| return script.join('\n'); | ||
| } | ||
|
|
||
| /** | ||
| * Generate completion case logic for a command | ||
| */ | ||
| private generateCommandCase(cmd: CommandDefinition, indent: string): string[] { | ||
| const lines: string[] = []; | ||
|
|
||
| // Handle subcommands | ||
| if (cmd.subcommands && cmd.subcommands.length > 0) { | ||
| lines.push(`${indent}if [[ $cword -eq 2 ]]; then`); | ||
| lines.push(`${indent} local subcommands="` + cmd.subcommands.map(s => this.escapeCommandName(s.name)).join(' ') + '"'); | ||
| lines.push(`${indent} COMPREPLY=($(compgen -W "$subcommands" -- "$cur"))`); | ||
| lines.push(`${indent} return 0`); | ||
| lines.push(`${indent}fi`); | ||
| lines.push(''); | ||
| lines.push(`${indent}case "$subcmd" in`); | ||
|
|
||
| for (const subcmd of cmd.subcommands) { | ||
| lines.push(`${indent} ${subcmd.name})`); | ||
| lines.push(...this.generateArgumentCompletion(subcmd, indent + ' ')); | ||
| lines.push(`${indent} ;;`); | ||
| } | ||
|
|
||
| lines.push(`${indent}esac`); | ||
| } else { | ||
| // No subcommands, just complete arguments | ||
| lines.push(...this.generateArgumentCompletion(cmd, indent)); | ||
| } | ||
|
|
||
| return lines; | ||
| } | ||
|
|
||
| /** | ||
| * Generate argument completion (flags and positional arguments) | ||
| */ | ||
| private generateArgumentCompletion(cmd: CommandDefinition, indent: string): string[] { | ||
| const lines: string[] = []; | ||
|
|
||
| // Check for flag completion | ||
| if (cmd.flags.length > 0) { | ||
| lines.push(`${indent}if [[ "$cur" == -* ]]; then`); | ||
| const flags = cmd.flags.map(f => { | ||
| const parts: string[] = []; | ||
| if (f.short) parts.push(`-${f.short}`); | ||
| parts.push(`--${f.name}`); | ||
| return parts.join(' '); | ||
| }).join(' '); | ||
| lines.push(`${indent} local flags="${flags}"`); | ||
| lines.push(`${indent} COMPREPLY=($(compgen -W "$flags" -- "$cur"))`); | ||
| lines.push(`${indent} return 0`); | ||
| lines.push(`${indent}fi`); | ||
| lines.push(''); | ||
| } | ||
|
|
||
| // Handle positional completions | ||
| if (cmd.acceptsPositional) { | ||
| lines.push(...this.generatePositionalCompletion(cmd.positionalType, indent)); | ||
| } | ||
|
|
||
| return lines; | ||
| } | ||
|
|
||
| /** | ||
| * Generate positional argument completion based on type | ||
| */ | ||
| private generatePositionalCompletion(positionalType: string | undefined, indent: string): string[] { | ||
| const lines: string[] = []; | ||
|
|
||
| switch (positionalType) { | ||
| case 'change-id': | ||
| lines.push(`${indent}_openspec_complete_changes`); | ||
| break; | ||
| case 'spec-id': | ||
| lines.push(`${indent}_openspec_complete_specs`); | ||
| break; | ||
| case 'change-or-spec-id': | ||
| lines.push(`${indent}_openspec_complete_items`); | ||
| break; | ||
| case 'shell': | ||
| lines.push(`${indent}local shells="zsh bash fish powershell"`); | ||
| lines.push(`${indent}COMPREPLY=($(compgen -W "$shells" -- "$cur"))`); | ||
| break; | ||
| case 'path': | ||
| lines.push(`${indent}COMPREPLY=($(compgen -f -- "$cur"))`); | ||
| break; | ||
| } | ||
|
|
||
| return lines; | ||
| } | ||
|
|
||
| /** | ||
| * Generate dynamic completion helper functions | ||
| */ | ||
| private generateDynamicCompletionHelpers(): string[] { | ||
| const lines: string[] = []; | ||
|
|
||
| lines.push('# Dynamic completion helpers'); | ||
| lines.push(''); | ||
|
|
||
| // Helper for completing change IDs | ||
| lines.push('_openspec_complete_changes() {'); | ||
| lines.push(' local changes'); | ||
| lines.push(' changes=$(openspec __complete changes 2>/dev/null | cut -f1)'); | ||
| lines.push(' COMPREPLY=($(compgen -W "$changes" -- "$cur"))'); | ||
| lines.push('}'); | ||
| lines.push(''); | ||
|
|
||
| // Helper for completing spec IDs | ||
| lines.push('_openspec_complete_specs() {'); | ||
| lines.push(' local specs'); | ||
| lines.push(' specs=$(openspec __complete specs 2>/dev/null | cut -f1)'); | ||
| lines.push(' COMPREPLY=($(compgen -W "$specs" -- "$cur"))'); | ||
| lines.push('}'); | ||
| lines.push(''); | ||
|
|
||
| // Helper for completing both changes and specs | ||
| lines.push('_openspec_complete_items() {'); | ||
| lines.push(' local items'); | ||
| lines.push(' items=$(openspec __complete changes 2>/dev/null | cut -f1; openspec __complete specs 2>/dev/null | cut -f1)'); | ||
| lines.push(' COMPREPLY=($(compgen -W "$items" -- "$cur"))'); | ||
| lines.push('}'); | ||
| lines.push(''); | ||
|
|
||
| return lines; | ||
| } | ||
|
|
||
| /** | ||
| * Escape command/subcommand names for safe use in Bash scripts | ||
| */ | ||
| private escapeCommandName(name: string): string { | ||
| // Escape shell metacharacters to prevent command injection | ||
| return name.replace(/["\$`\\]/g, '\\$&'); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a bunch of
zshrcConfiguredchecks in this file, do we need to extend for other shells here too?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly I think there's a few places where we focus on
zshheavily. For example there's a restart hint for zsh but not for other shells.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think even when uninstalling we mention zshrc