feat: improve completion command with shell auto-detection, pwsh support, and usage hints - #2552
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review @stalep i can't get the pwsh completion to work ...any idea what doing wrong? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get PR info and diff
gh pr view 2552 --repo jbangdev/jbang --json title,body,files,headRefName,baseRefName 2>/dev/null | jq '{title, headRefName, baseRefName, files: [.files[].path]}'Length of output: 1926 🏁 Script executed: #!/bin/bash
# Get the diff of the PR
gh pr diff 2552 --repo jbangdev/jbang 2>/dev/null | head -500Length of output: 28734 🏁 Script executed: #!/bin/bash
# Look at the completion command implementation
fd -e java . --exec grep -l -i "completion" {} \; | head -20Length of output: 1005 🏁 Script executed: #!/bin/bash
# Look at the aesh version used
cat build.gradle | grep -i "aesh" || true
cat build.gradle.kts | grep -i "aesh" || true
fd -g "*.gradle" --exec grep -l "aesh" {} \;Length of output: 878 🏁 Script executed: #!/bin/bash
# Read the full Completion.java file
cat -n src/main/java/dev/jbang/cli/Completion.javaLength of output: 6617 🏁 Script executed: #!/bin/bash
# Get the rest of the PR diff
gh pr diff 2552 --repo jbangdev/jbang 2>/dev/null | tail -300Length of output: 12515
Review:
|
| # | Severity | File | Issue |
|---|---|---|---|
| 1 | 🐛 Bug | Completion.java |
detectShell() returns BASH from PowerShell on macOS/Linux due to BASH_VERSION leaking from the wrapper; PSModulePath should be checked first |
| 2 | Completion.java |
Inconsistent pwsh hint (header vs. usageHint) | |
| 3 | Completion.java |
Bare jbang completion now throws instead of defaulting to bash |
|
| 4 | 🐛 Bug | nav.adoc |
Duplicate navigation entries |
| 5 | jbang-completion.adoc, jbang.adoc |
Machine-specific clidoc artifact (mac-aarch64) in committed docs |
The PSModulePath-first detection order fix (#1) should resolve the pwsh completion issue you're seeing @maxandersen.
ʕ •ᴥ•ʔ (hope that helps track it down!)
…537) Move the PSModulePath check before BASH_VERSION/ZSH_VERSION/FISH_VERSION in detectShell(). On macOS/Linux, CLI tools are typically launched via a bash wrapper script which sets BASH_VERSION in the environment. When PowerShell calls such a tool, Java inherits BASH_VERSION from the wrapper and detectShell() incorrectly returns BASH instead of PWSH. PSModulePath is reliably set by PowerShell on all platforms (Windows, macOS, Linux) and is never set by bash/zsh/fish, so checking it first correctly identifies the user's actual shell. Fixes: #537 Related: jbangdev/jbang#2552
…539) Remove the cursorAtPositional heuristic from performDynamicCompletion() that aggressively removed all option candidates (anything starting with "-") when the cursor was at a positional argument position. This caused "jbang run <tab>" to show only files instead of the expected options and file arguments. The completion engine (AeshCommandLineCompletionParser) already determines the correct candidate set based on parser state. The cursorAtPositional filter was second-guessing the engine and removing valid candidates for the very common case of pressing tab after a subcommand name. Changes: - Remove the option filtering heuristic (was lines 234-239) - Emit __aesh_file__/__aesh_dir__ sentinel alongside other candidates (not instead of them) when the current positional supports file completion. Shell scripts (bash/fish/pwsh) merge file completion with option/subcommand suggestions. - Update test: options and file sentinel now coexist at positional positions, matching how bash/fish/zsh handle completion Fixes: #539 Related: jbangdev/jbang#2552
7f4f6a1 to
0b250bc
Compare
…ort, and usage hints - Auto-detect shell from FISH_VERSION, ZSH_VERSION, BASH_VERSION, PSModulePath env vars, falling back to $SHELL - Accept shell as positional argument: jbang completion fish - Add PowerShell (pwsh) support via aesh 3.15.1 upgrade - Show quick-start commands in --help output - Append shell-specific setup instructions to generated scripts - Add Tab Completion section to installation.adoc - Add pwsh support to dev-shell.sh and .justfile - Regenerate CLI reference docs - Set rootProject.name = 'jbang' in settings.gradle
0b250bc to
f8ff8f3
Compare
…experimental, nav duplicates - Check PSModulePath before BASH_VERSION in detectShell() to fix pwsh detection on macOS/Linux where bash wrapper leaks BASH_VERSION - Add testable detectShell(Function) overload with 11 unit tests - Mark pwsh completion support as experimental in help, docs, and clidoc - Remove duplicate jdk sub-command entries in cli nav.adoc
When we migrated from picocli to aesh, the
completioncommand lost the helpful usage instructions that told users how to enable tab completions. This PR restores and improves that experience.Changes
Shell auto-detection —
jbang completionnow auto-detects the current shell by checkingFISH_VERSION,ZSH_VERSION,BASH_VERSION, andPSModulePathenvironment variables, then falls back to$SHELL. No--shellflag needed in most cases.Positional argument — The shell can now be passed as a simple argument:
jbang completion fishinstead ofjbang completion --shell fish. The--shelloption is still supported for backwards compatibility.PowerShell support — Added pwsh as a supported shell, enabled by upgrading aesh from 3.14.3 to 3.15.1 which includes
PowerShellCompletionGenerator.Usage hints in
--help—jbang completion --helpnow shows quick-start commands for each shell.Usage hints in generated scripts — Each generated completion script includes shell-specific comments at the end explaining how to enable completions (both "try it now" and "make it permanent" approaches).
Documentation — Added a "Tab Completion" section to
installation.adocwith per-shell setup instructions. Updated CLI reference docs viaclidoc.Dev shell —
just shell pwshnow works for interactive completion testing.Build fixes — Set
rootProject.name = 'jbang'insettings.gradleso the shadow jar is namedjbang.jar(matching what the launcher script expects).Example output