feat(shell): bash-style multi-column tab completion with custom pager#318
Merged
F16shen merged 2 commits intoJun 29, 2026
Merged
Conversation
Replace rustyline's built-in completion list with a bash-like multi-column pager (Space/Enter/q/Ctrl+C), route local paths through FilenameCompleter, and use double-Tab to show candidates while Repaint keeps ANSI prompts intact.
Contributor
|
Thanks for the pull request. A maintainer will review it when available. Please keep the PR focused, explain the why in the description, and make sure local checks pass before requesting review. Contribution guide: https://github.com/AI-Shell-Team/aish/blob/main/CONTRIBUTING.md |
Contributor
|
This pull request description looks incomplete. Please update the missing sections below before review. Missing items:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds shared shell completion, paged completion-list output, and double-Tab readline handling. It also updates PTY path-token detection, re-exports the local-path helper, and enables ChangesCompletion and readline tab flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/aish-shell/src/readline.rs (1)
109-116: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid querying completions twice on the first Tab.
The handler calls
engine.complete(...)and then returnsNone, so rustyline immediately reachesShellHelper::complete, which callsengine.complete(...)again. For PTY-backed completions, that can duplicate the timeout-boundquery_completionsround trip on every first Tab.Consider making the handler only arm
TAB_STATEon the first Tab, and queryengine.complete(...)only when a pending second Tab is actually going to print the custom list.Also applies to: 268-287
🤖 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/aish-shell/src/readline.rs` around lines 109 - 116, The Tab handling in ShellHelper::complete is triggering engine.complete(...) twice on the first Tab because the pre-check returns None and rustyline then calls the completion path again. Update the first-Tab logic so it only arms TAB_STATE.pending_list and defers calling engine.complete(...) until the second Tab actually needs to print the custom list, using the existing ShellHelper::complete and TAB_STATE flow to avoid the duplicate completion query.
🤖 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 `@crates/aish-pty/src/readline_tab.rs`:
- Around line 61-68: The remote-path heuristic in looks_like_remote_path() is
missing the bare host:/path form, so should_complete_path_locally() still routes
scp-style remote targets to FilenameCompleter. Update looks_like_remote_path()
in readline_tab.rs to also पहचान bare host:/... tokens before local completion
is chosen, while preserving the existing URL and user@host:/... checks. Add a
regression test covering host:/etc/ (and similar) to verify it is treated as
remote and not handled by the local filename completer.
In `@crates/aish-shell/src/completion_list.rs`:
- Around line 100-120: The terminal size selection in completion_list should use
precedence instead of max, so stale cached or COLUMNS/LINES values do not
override the current tty size after a resize. Update the logic around
current_terminal_size, winsize_ioctl, and the env fallbacks to prefer a non-zero
ioctl result first, then the cached rustyline size, then environment/default
values, and treat zero ws_col/ws_row as unavailable rather than converting them
into sentinel dimensions. Also remove the behavior where winsize_ioctl turns
ws_row == 0 into u16::MAX, since the rows path later clamps it and page_size
never sees the intended unknown-size signal.
- Around line 229-234: The pager flow in the `for row in rows` loop is
discarding legitimate input by calling `drain_stdin_typeahead()` right after
rendering `MORE_PROMPT` in the `read_more_action()` path. Remove that
stdin-draining step from the `--More--` prompt handling so keys like Space or q
entered immediately after the prompt are preserved and processed normally.
---
Nitpick comments:
In `@crates/aish-shell/src/readline.rs`:
- Around line 109-116: The Tab handling in ShellHelper::complete is triggering
engine.complete(...) twice on the first Tab because the pre-check returns None
and rustyline then calls the completion path again. Update the first-Tab logic
so it only arms TAB_STATE.pending_list and defers calling engine.complete(...)
until the second Tab actually needs to print the custom list, using the existing
ShellHelper::complete and TAB_STATE flow to avoid the duplicate completion
query.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e8300e2-b114-462e-90fe-ccc378f71c76
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
Cargo.tomlcrates/aish-pty/src/lib.rscrates/aish-pty/src/readline_tab.rscrates/aish-shell/src/completion.rscrates/aish-shell/src/completion_list.rscrates/aish-shell/src/lib.rscrates/aish-shell/src/readline.rs
Route scp-style host:/path tokens to PTY completion instead of FilenameCompleter, and move typeahead draining ahead of the pager prompt so Space/q are not eaten.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CompletionEngineto route local path tokens throughFilenameCompleterand everything else through PTYquery_completions(noforward_readline_tabprobe in the shell layer).--More--pager (Space/y next page, Enter next line, q/n/Backspace/Ctrl+C stop).Cmd::Repaintso ANSI prompts are not corrupted by manual stdout redraw.Test plan
make format-check && make lintcargo test -p aish-shell completioncargo test -p aish-pty readline_tabls /usr/bin→ Tab Tab → multi-column list appearsq/ Ctrl+C to stop without extra blank linels /usrTab → LCP extends to/usr/…(notr/)systemctl restTab → completes without timeoutSummary by CodeRabbit
CompletionEnginethat chooses local filesystem completion or PTY-backed completion, improving behavior around cursor position and word detection.user@host:/path,host:/path).rustylinefeature for directory-aware behavior.