-
Notifications
You must be signed in to change notification settings - Fork 971
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 2 commits
e89f79e
ad53abc
07cad96
0bac2e3
5524026
9dd9962
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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,192 @@ | ||
| 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'); | ||
| 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(' _init_completion || return'); | ||
| script.push(''); | ||
|
Comment on lines
+25
to
+28
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. 🧩 Analysis chain🏁 Script executed: # First, let's look at the bash-generator.ts file to understand the complete context
fd -t f bash-generator.ts -x wc -l {}Repository: Fission-AI/OpenSpec Length of output: 118 🏁 Script executed: # Search for _init_completion and bash-completion references
rg -n "_init_completion|bash-completion" --type=ts -A 2 -B 2Repository: Fission-AI/OpenSpec Length of output: 7773 🏁 Script executed: # Check if there are other shell generators for comparison
fd -t f -path "*/generators/*" ".*-generator\.ts$"Repository: Fission-AI/OpenSpec Length of output: 231 🏁 Script executed: # Look for any documentation about dependencies or setup
fd -t f "README|INSTALL|SETUP|package\.json" -x grep -l "bash-completion" {} \;Repository: Fission-AI/OpenSpec Length of output: 45 🏁 Script executed: # Let's look at the full context of bash-generator.ts around lines 25-28
sed -n '1,50p' src/core/completions/generators/bash-generator.tsRepository: Fission-AI/OpenSpec Length of output: 1791 🏁 Script executed: # Check the bash-installer.ts warnings about bash-completion
sed -n '210,275p' src/core/completions/installers/bash-installer.tsRepository: Fission-AI/OpenSpec Length of output: 2577 🏁 Script executed: # Look at the beginning of bash-generator.ts to see if there's any documentation
head -n 30 src/core/completions/generators/bash-generator.tsRepository: Fission-AI/OpenSpec Length of output: 1115 Document bash-completion requirement in generated script header. The Consider adding a comment in the generated script header (e.g., 🤖 Prompt for AI Agents |
||
| 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 => 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 => 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; | ||
| } | ||
| } | ||
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.
🛠️ Refactor suggestion | 🟠 Major
Missing required "Impact" section per coding guidelines.
The proposal is missing the required "Impact" section that should list affected specs and code. Based on learnings,
proposal.mdmust include sections: Why, What Changes (bullet list with breaking change markers), and Impact (affected specs and code).🔎 Suggested addition
🤖 Prompt for AI Agents