Add live picker for bare qr go - #50
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The incremental change (commit range Files Reviewed (1 file)
Previous Review Summaries (6 snapshots, latest commit 38aac1d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 38aac1d)Status: No Issues Found | Recommendation: Merge The incremental change (commit range Files Reviewed (1 file)
Previous review (commit a4da6f4)Status: No Issues Found | Recommendation: Merge Files Reviewed (6 files)
Previous review (commit fd65294)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit 910cfa9)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 2cec2aa)Status: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Previous review (commit 25c9b35)Status: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Reviewed by glm-5.2-short · Input: 11.6K · Output: 1.8K · Cached: 167.9K |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a live-filter picker for ChangesLive Picker Feature
Test Isolation and npm Handling
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant run_with_config
participant execute_live
participant pick_live_index
participant ProjectCache
User->>run_with_config: qr go
run_with_config->>execute_live: empty query
execute_live->>ProjectCache: load cached projects
execute_live->>pick_live_index: project labels
pick_live_index-->>execute_live: selected project index
execute_live-->>run_with_config: GoResult with path
run_with_config-->>User: formatted result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25c9b35c1a
ℹ️ 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".
|
|
||
| loop { | ||
| render_live_filter(&state, per_page)?; | ||
| if let Event::Key(key) = event::read()? { |
There was a problem hiding this comment.
Ignore release key events in live picker
On Windows, Crossterm sets key event kinds and emits release events as well as presses; because the new bare-qr go live picker handles every Event::Key, a single typed character can be inserted twice (for example api becomes aappii, and arrow/backspace actions can also apply twice). Please skip KeyEventKind::Release events before matching so filtering remains usable on Windows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 910cfa9: live picker now ignores KeyEventKind::Release while still handling press/repeat events.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/commands/go.rs`:
- Around line 19-26: The empty-cache path in `go` is being misreported as a user
cancellation because `picker::pick_live_index` returns `Ok(None)` when `labels`
is empty and `project_at_picker_index` turns that into “Selection cancelled.”
Add an explicit check in the `go` command flow before calling
`picker::pick_live_index`/`project_at_picker_index` so an empty `cache.projects`
returns a clear “no projects found” style error instead of cancellation, and
keep the normal `Selection cancelled` behavior only for real picker aborts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5bd2f6c7-7c57-4477-b673-707c6ab67046
📒 Files selected for processing (5)
CHANGELOG.mdREADME.mdsrc/commands/go.rssrc/main.rssrc/picker.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/picker.rs`:
- Around line 193-234: The live picker in pick_live_index leaves stale terminal
output if render_live_filter or event::read fails because the ? operator exits
before cleanup. Update pick_live_index to ensure
clear_live_filter(rendered_lines) runs on every error path before propagating
the failure, preserving the same cleanup used for Enter/Esc/Ctrl-C and keeping
the UI consistent even when rendering or input reading fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc03ce42-10f3-4b2c-a1b2-ced3e9f1a6e8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
CHANGELOG.mdCargo.tomlREADME.mdsrc/commands/go.rssrc/main.rssrc/picker.rs
✅ Files skipped from review due to trivial changes (2)
- CHANGELOG.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main.rs
- src/commands/go.rs
Summary
qr go/qr gqr go <project>direct lookup behavior unchanged--print-pathstdout remains shell-wrapper safeVerification
cargo fmt --all -- --checkcargo testcargo clippy --all-targets --locked -- -D warningsqr go --print-path, typedapi, selected/tmp/orion-apiqr go quick-runner --print-pathprinted only/tmp/quick-runnerSummary by CodeRabbit
qr go/qr gwhen no project argument is provided, using cached projects.EscorCtrl-Ccancels cleanly.