feat: add shell tab completion via clap_complete#182
Conversation
Uses clap_complete's CompleteEnv for dynamic runtime completions, plus a `detail completions` install command that auto-detects the user's shell and appends the activation snippet to the appropriate rc file. Supports bash, zsh, fish, elvish, and powershell. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sachiniyer
temporarily deployed
to
integration-tests
GitHub Actions
Inactive
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sachiniyer
temporarily deployed
to
integration-tests
GitHub Actions
Inactive
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| fn detect_shell() -> Result<String> { | ||
| let shell = env::var("SHELL").context("could not detect shell from $SHELL")?; | ||
| // $SHELL is e.g. "/bin/zsh" — take the basename | ||
| let name = shell.rsplit('/').next().unwrap_or(&shell); | ||
| Ok(name.to_lowercase()) | ||
| } |
There was a problem hiding this comment.
🔴 detect_shell() always fails on Windows because $SHELL is not set
The detect_shell() function relies exclusively on the $SHELL environment variable (line 9), which is a Unix convention and is never set on Windows. Since the tool explicitly targets x86_64-pc-windows-msvc (per dist-workspace.toml:15) and ships a PowerShell installer, Windows users running detail completions will always get the error "could not detect shell from $SHELL" instead of detecting PowerShell/pwsh. The function should fall back to checking the parent process name or the PSModulePath/COMSPEC environment variables on Windows to detect PowerShell or cmd.
Prompt for agents
In src/commands/completions.rs, the detect_shell() function at lines 8-13 only checks the $SHELL environment variable, which does not exist on Windows. Since the tool targets Windows (x86_64-pc-windows-msvc) and ships a PowerShell installer, you need to add a Windows fallback. For example, on Windows you could check for the PSModulePath environment variable to detect PowerShell, or check the COMSPEC variable for cmd.exe. A possible approach:
1. Keep the $SHELL check as the first attempt.
2. If $SHELL is not set (especially on Windows), check if PSModulePath is set in the environment — if so, return "powershell".
3. As a last resort on Windows, check COMSPEC.
4. Return a clear error if no shell could be detected on any platform.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0941b2742b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn detect_shell() -> Result<String> { | ||
| let shell = env::var("SHELL").context("could not detect shell from $SHELL")?; | ||
| // $SHELL is e.g. "/bin/zsh" — take the basename | ||
| let name = shell.rsplit('/').next().unwrap_or(&shell); |
There was a problem hiding this comment.
detect_shell currently extracts the shell name with rsplit('/'), which only works for POSIX-style paths; on Windows, values like C:\Program Files\PowerShell\7\pwsh.exe are not split and therefore never match the supported "pwsh"/"powershell" cases. In that environment, detail completions will report an unsupported shell even though PowerShell is intended to be supported.
Useful? React with 👍 / 👎.
| "powershell" | "pwsh" => Ok(env::var("PROFILE").map_or_else( | ||
| |_| home.join(".config/powershell/Microsoft.PowerShell_profile.ps1"), |
There was a problem hiding this comment.
The PowerShell branch in rc_path reads PROFILE from process environment, but $PROFILE is a PowerShell automatic variable and is typically not exported as an environment variable. That means this usually falls back to ~/.config/powershell/..., which is not the profile file PowerShell loads on Windows, so the installer can claim success while writing to a file that is never sourced.
Useful? React with 👍 / 👎.
## Summary - Bumps version from 0.1.10 to 0.1.11 - The release workflow will auto-tag and build artifacts when merged to main ### Changes since v0.1.10 **Features:** - Add shell tab completion via clap_complete (#182) - Add --scan-id flag to bugs list command (#184) - Add scans list command (#163) - Add scans to the skill.md for the detail CLI (#185) - Auto-detect repository in detail-bugs skill (#160) **Fixes:** - Show user-friendly error messages instead of debug output (#129) - Continue list numbering across pages instead of resetting (#130) - Add file locking to prevent concurrent config overwrites (#131) **Chores & Refactoring:** - Replace Makefile with cargo xtask (#159) - Upgrade all dependencies to latest versions (#158) - Enable comprehensive clippy lints (pedantic, nursery, restriction) (#134, #135, #144, #152, #153, #154, #155) - Add GitHub Pages workflow for API docs (#178) - Add Scalar API reference page (#177) - Add integration tests against live API (#181) - Remove alpha warning from README (#156) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Uses clap_complete's CompleteEnv for dynamic runtime completions,
plus a
detail completionsinstall command that auto-detects theuser's shell and appends the activation snippet to the appropriate
rc file. Supports bash, zsh, fish, elvish, and powershell.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com