feat(installer): setup PATH and shell completions - #711
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe CLI now uses ChangesInstaller shell integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Installer
participant MinBinary
participant ShellFiles
participant ShellRC
Installer->>MinBinary: execute min completions shell
MinBinary->>Installer: return completion content
Installer->>ShellFiles: write init and completion files
Installer->>ShellRC: append marker-fenced source block
Installer->>ShellFiles: remove generated files during uninstall
Installer->>ShellRC: strip marker-fenced block
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/minimal/src/lib.rs (1)
60-85: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStale
minimalcommand in other help-text examples.The completions example was updated to
min(line 101), but thessh-forwardexample at line 63 (minimal ssh-forward dev 18080:127.0.0.1:80) and theloginexamples at lines 80-84 (minimal login,~/.config/minimal/...) still show the old command name. Since the CLI is now invoked asmin, users copy-pasting these examples get a stale/incorrect command.✏️ Suggested fix
- /// minimal ssh-forward dev 18080:127.0.0.1:80 + /// min ssh-forward dev 18080:127.0.0.1:80- /// minimal login + /// min login🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/minimal/src/lib.rs` around lines 60 - 85, Update the help-text examples in the SshForward and Login command documentation to use the current `min` command name instead of `minimal`, including the login command and all referenced configuration paths. Keep the examples consistent with the updated completions example.
🧹 Nitpick comments (1)
scripts/install.sh (1)
509-527: 🚀 Performance & Scalability | 🔵 Trivial
zsh.shrunscompinitunconditionally on every shell startup.
compinitwithout a cache-aware guard (e.g. checking.zcompdumpfreshness,compinit -C) rescans the wholefpathon every new shell, adding noticeable startup latency for zsh users. Since this file is regenerated on every install run anyway, consider emitting a cached-init pattern.♻️ Optional caching pattern
if [ -d "$zsh_comp_dir" ]; then fpath=("$zsh_comp_dir" \$fpath) - autoload -Uz compinit - compinit + autoload -Uz compinit + for dump in "\${ZDOTDIR:-\$HOME}"/.zcompdump(N.mh+24); do + compinit + done + compinit -C fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install.sh` around lines 509 - 527, Update the generated zsh init template in the heredoc that emits zsh.sh so compinit uses a cache-aware approach instead of unconditionally rescanning fpath on every startup; preserve the fpath setup and use an appropriate .zcompdump freshness check or compinit -C guard before invoking compinit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/minimal/src/lib.rs`:
- Around line 60-85: Update the help-text examples in the SshForward and Login
command documentation to use the current `min` command name instead of
`minimal`, including the login command and all referenced configuration paths.
Keep the examples consistent with the updated completions example.
---
Nitpick comments:
In `@scripts/install.sh`:
- Around line 509-527: Update the generated zsh init template in the heredoc
that emits zsh.sh so compinit uses a cache-aware approach instead of
unconditionally rescanning fpath on every startup; preserve the fpath setup and
use an appropriate .zcompdump freshness check or compinit -C guard before
invoking compinit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02ed5f81-c698-433c-ba47-2871f650f2c8
📒 Files selected for processing (5)
crates/minimal/src/lib.rsdocs/specs/07-spec-installer/07-spec-installer.mdscripts/install.shscripts/install_test.shscripts/stage-release.sh
b375fb0 to
ac69403
Compare
Summary by CodeRabbit
New Features
min(including shell completion/help text).Bug Fixes
Documentation