Lots of small fixes and improvements#23
Merged
JackDerksen merged 18 commits intomainfrom Apr 22, 2026
Merged
Conversation
cursor goes behind the popup)
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 44 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR adds four Vim motions ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant InputHandler as Input Handler
participant Motions as Motion Resolver
participant Buffer as TextBuffer
participant UI as UI Renderer
rect rgb(100, 150, 200, 0.5)
Note over User,Buffer: Delimiter Matching (%)
User->>InputHandler: Press % over delimiter
InputHandler->>Motions: MatchDelimiter motion
Motions->>Buffer: matching_delimiter(cursor_pos)
Buffer->>Buffer: Scan for paired delimiter
Buffer-->>Motions: Matching position
Motions-->>UI: Cursor moves to match
UI-->>User: Jump to paired bracket
end
rect rgb(100, 200, 150, 0.5)
Note over User,UI: Backward Character Find (F/T)
User->>InputHandler: Press F, then char
InputHandler->>Motions: FindCharBefore(char)
Motions->>Buffer: find_char_before_on_line()
Buffer-->>Motions: Previous position
Motions-->>UI: Cursor repositioned
UI-->>User: Jump to last occurrence
end
rect rgb(200, 150, 100, 0.5)
Note over User,UI: Case Toggle (~)
User->>InputHandler: Press ~ with count
InputHandler->>InputHandler: ToggleCase action
InputHandler->>Buffer: toggle_case_under_cursor_or_selection()
Buffer->>Buffer: Replace chars (lower↔upper)
Buffer-->>InputHandler: Update buffer
InputHandler-->>UI: Render changes
UI-->>User: Characters toggled
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/redox-tui/src/app/state/commands.rs (1)
228-265:⚠️ Potential issue | 🟡 MinorTrim-on-save leaves dirty bookkeeping stale if the write fails.
The trim branch records the undo and invalidates caches, but never calls
recompute_active_dirty(). On the happy pathsave_active()clears dirty, so nothing is visibly wrong. But ifsave_active()returnsErr, the buffer has been mutated (whitespace stripped) while the dirty flag still reflects the pre-trim state. For a buffer that was clean before:w(e.g. disk changed underneath, or a read-only mount causing the write to fail), the user sees trimmed content with no dirty indicator until the next edit.🛠 Suggested fix
let before = self.capture_active_undo_snapshot(); let trimmed = self.trim_active_trailing_whitespace(); if trimmed { let (viewport_width_cells, viewport_height_rows) = self.viewport_size(); let text_vh = viewport_height_rows.saturating_sub(STATUS_BAR_HEIGHT_ROWS); let active_id = self.session.active_id(); let view = self.views.entry(active_id).or_default(); let buffer = self.session.active_buffer(); view.cursor .reconcile_after_edit(buffer, viewport_width_cells, text_vh); self.invalidate_active_render_caches(); let _ = self.record_active_undo_if_changed(before); + let _ = self.session.recompute_active_dirty(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/redox-tui/src/app/state/commands.rs` around lines 228 - 265, The trim-on-save branch mutates the buffer (via trim_active_trailing_whitespace) and records an undo but never updates dirty bookkeeping; ensure the buffer's dirty state is recomputed immediately after the mutation/undo-recording so it reflects the trimmed content even if save_active() fails. In write_current_file, after calling self.record_active_undo_if_changed(before) (inside the trimmed branch) call self.recompute_active_dirty() (or equivalent) so recompute_active_dirty() runs before attempting self.session.save_active(); this guarantees the dirty flag is correct whether save succeeds or errors.
🧹 Nitpick comments (1)
crates/redox-tui/src/app/state/commands.rs (1)
139-149: Unbounded command history growth.
push_command_historyonly dedupes consecutive duplicates — there's no cap oncommand_history.entries. For a session-bound history this is unlikely to bite users in practice, but it's a trivial upgrade to bound it (e.g. keep the last N, say 200) and it also avoids pathological growth if the same alternating two commands are run thousands of times.♻️ Sketch
fn push_command_history(&mut self, command: String) { if self .command_history .entries .last() .is_some_and(|previous| previous == &command) { return; } self.command_history.entries.push(command); + const MAX_COMMAND_HISTORY: usize = 200; + if self.command_history.entries.len() > MAX_COMMAND_HISTORY { + let overflow = self.command_history.entries.len() - MAX_COMMAND_HISTORY; + self.command_history.entries.drain(0..overflow); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/redox-tui/src/app/state/commands.rs` around lines 139 - 149, push_command_history currently only dedupes consecutive duplicates and lets command_history.entries grow unbounded; add a fixed cap (e.g. const MAX_HISTORY: usize = 200) and after pushing the new command trim the Vec to keep only the most recent MAX_HISTORY entries. Keep the existing consecutive-duplicate check in push_command_history, push the command as now, then if self.command_history.entries.len() > MAX_HISTORY remove the oldest entries (e.g. drain(0..excess) where excess = len - MAX_HISTORY) so the history never exceeds the cap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/redox-core/src/buffer/text_buffer/search.rs`:
- Around line 46-146: The asymmetric delimiters are not skipping escaped
characters, so escaped opens/closes (e.g., '\(') are treated as structural
matches; update matching_delimiter to consult char_is_escaped_for_pairing for
the current char (use the computed char_idx) and return None when the delimiter
is escaped before dispatching to match_asymmetric_delimiter_forward or
match_asymmetric_delimiter_backward; alternatively add the same escaped check at
the start of match_asymmetric_delimiter_forward and
match_asymmetric_delimiter_backward (using char_idx/end_idx) so escaped
asymmetric delimiters are ignored the same way symmetric ones are.
In `@crates/redox-tui/src/app/state/editing.rs`:
- Around line 495-542: The loop in toggle_case_under_cursor can encounter
multi-character expansions from toggled_case_text (via
char::to_uppercase()/to_lowercase()), which causes the cursor to advance by the
replacement length and skip over the expanded text; this is intentional but
should be documented and covered by tests—add an inline comment in
toggle_case_under_cursor explaining that multi-codepoint replacements (e.g.,
'ß'→"SS") will advance cursor past the expansion and that this matches Vim-like
behavior, and add/plan a unit/integration test targeting toggled_case_text +
replace_selection behavior to assert correct cursor advancement and rendering
after replacements.
In `@crates/redox-tui/src/app/state/search.rs`:
- Around line 276-279: The match arm that constructs a SearchQuery for
Motion::FindCharBefore and Motion::TillCharBefore should preserve the original
`T` semantics by using SearchLanding::AfterMatch for TillCharBefore; update the
pattern so Motion::FindCharBefore continues to map to SearchLanding::OnMatch
while Motion::TillCharBefore maps to SearchLanding::AfterMatch when building the
SearchQuery (refer to SearchQuery and SearchLanding in the same match).
In `@crates/redox-tui/src/lib.rs`:
- Around line 320-329: The status toast's layout isn't considered when deciding
whether to hide the cursor: after calling draw_status_toast(state, style,
window) the code only checks perf_popup_layout via perf_popup_occludes_cursor,
so a cursor under the toast can remain visible. Change draw_status_toast to
expose/return the toast layout (or provide a way to query it), then in the same
cursor handling block use that layout together with perf_popup_layout and
perf_popup_occludes_cursor to decide whether to call hide_cursor(window) or
window.request_cursor(cursor); update references to draw_status_toast,
perf_popup_layout, perf_popup_occludes_cursor, hide_cursor,
window.request_cursor and cursor_spec accordingly.
In `@crates/redox-tui/src/ui/widgets/command_line.rs`:
- Around line 95-103: The branch treats exact-fit inputs as clipped; change the
clip boundary so text_width == input_width is handled by the unclipped path: in
command_line.rs update the condition that currently checks
command_text_width(text) < input_width to use <= instead, so when
command_text_width(text) <= input_width you return (clip_text_to_cells(text,
input_width), command_text_width(&clipped)) and only call
clip_text_right_to_cells(text, visible_width) when text is strictly wider; the
referenced symbols are command_text_width, clip_text_to_cells,
clip_text_right_to_cells, text/input_width.
In `@crates/redox-tui/src/ui/widgets/toast.rs`:
- Around line 29-31: Only apply the toast offset when the perf popup is actually
going to be drawn: change the logic that computes perf_popup_layout so it checks
the real draw condition used in lib.rs (i.e. ensure the same mode/visibility
check that causes the perf popup to be skipped in command/search modes) instead
of unconditionally calling state.perf_popup(). Locate the use of
state.perf_popup() and perf_popup_layout in toast rendering (symbols:
state.perf_popup, perf_popup_layout, the toast positioning code) and guard the
layout/offset code with the same visibility/mode check so toasts are not shifted
by a non-drawn (phantom) popup.
---
Outside diff comments:
In `@crates/redox-tui/src/app/state/commands.rs`:
- Around line 228-265: The trim-on-save branch mutates the buffer (via
trim_active_trailing_whitespace) and records an undo but never updates dirty
bookkeeping; ensure the buffer's dirty state is recomputed immediately after the
mutation/undo-recording so it reflects the trimmed content even if save_active()
fails. In write_current_file, after calling
self.record_active_undo_if_changed(before) (inside the trimmed branch) call
self.recompute_active_dirty() (or equivalent) so recompute_active_dirty() runs
before attempting self.session.save_active(); this guarantees the dirty flag is
correct whether save succeeds or errors.
---
Nitpick comments:
In `@crates/redox-tui/src/app/state/commands.rs`:
- Around line 139-149: push_command_history currently only dedupes consecutive
duplicates and lets command_history.entries grow unbounded; add a fixed cap
(e.g. const MAX_HISTORY: usize = 200) and after pushing the new command trim the
Vec to keep only the most recent MAX_HISTORY entries. Keep the existing
consecutive-duplicate check in push_command_history, push the command as now,
then if self.command_history.entries.len() > MAX_HISTORY remove the oldest
entries (e.g. drain(0..excess) where excess = len - MAX_HISTORY) so the history
never exceeds the cap.
🪄 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: 97945a04-bb74-4da1-9e76-ea8b88a68241
📒 Files selected for processing (20)
README.mdcrates/redox-core/src/buffer/text_buffer/editing.rscrates/redox-core/src/buffer/text_buffer/search.rscrates/redox-core/src/motion.rscrates/redox-tui/src/app/state.rscrates/redox-tui/src/app/state/actions.rscrates/redox-tui/src/app/state/commands.rscrates/redox-tui/src/app/state/editing.rscrates/redox-tui/src/app/state/explorer.rscrates/redox-tui/src/app/state/search.rscrates/redox-tui/src/app/state/surface.rscrates/redox-tui/src/app/state/tests.rscrates/redox-tui/src/input/mod.rscrates/redox-tui/src/lib.rscrates/redox-tui/src/ui/mod.rscrates/redox-tui/src/ui/widgets/command_line.rscrates/redox-tui/src/ui/widgets/mod.rscrates/redox-tui/src/ui/widgets/perf.rscrates/redox-tui/src/ui/widgets/status_bar.rscrates/redox-tui/src/ui/widgets/toast.rs
and add a unit test accordingly
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.
Lots of small miscellaneous fixes and improvements here:
:bnand:ls.~motion to invert uppercase/lowercase lettersF(shift+f) andT(shift+t) motions for backwards search%motion for jumping to the opening/closing delimiter match of the one under the cursorctrl+n/ctrl+p):perfpopup is activeSummary by CodeRabbit
Release Notes
New Features
~in Normal and Visual modes%FandTDocumentation
Tests