fix: expand copy-mode word-end window past a final wide glyph - #2145
fix: expand copy-mode word-end window past a final wide glyph#2145kiakiraki wants to merge 1 commit into
Conversation
point_is_final_atom compared an atom's terminating column against a word-motion target's starting column. The two are equal for narrow cells but never for wide ones, so the read window stopped expanding and copy-mode `e` halted at the soft-wrap boundary on CJK text.
📝 WalkthroughWalkthroughThe terminal pane now detects the final atom by its start point. A regression test covers ChangesWide Glyph Word Motion
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThe PR corrects copy-mode next-word-end motion when a long soft-wrapped read window ends on a wide glyph.
Confidence Score: 5/5The PR appears safe to merge, with the corrected comparison matching the start-point representation used by word-motion targets. The sole final-atom check receives atom start points, and the new equality correctly handles both narrow and wide atoms while preserving behavior for spacer heads and excluding point-less line-break atoms.
|
| Filename | Overview |
|---|---|
| src/pane/terminal.rs | Corrects final-atom detection for wide glyphs and adds a focused regression test without changing narrow-cell behavior. |
Reviews (1): Last reviewed commit: "fix: expand copy-mode word-end window pa..." | Re-trigger Greptile
What happens
Copy mode
e(next word end) stops at the read window boundary instead of the real end of the word. It triggers on a line that soft-wraps over more than 64 rows, with a wide (CJK) glyph as the last cell in the read window. The same case works fine with narrow characters.In practice you hit this with a long CJK log line or a JSON blob that wraps as one logical line. A minimal repro is a 2-column pane with
界written 66 times: pressingefrom the top should land on row 65, but it stops at row 63.Why
RetainedTextBuffer::point_is_final_atomcompared an atom's end column against the word-motion target's start column. Narrow cells haveend_col == start.col, so it matched by accident. Wide cells haveend_col == start.col + 1, so it never matched.The only caller is
needs_more_futureinword_motion_target, which decides "the target landed on the last atom in the window, so double the window and look further". With a wide glyph that never fired, and the window was returned unexpanded.clippy::suspicious_operation_groupingsflags this line on its own too. Its suggestedpoint.end_coldoes not compile, though —TerminalTextPointonly hasrowandcol.The fix
Word-motion targets are always atom start points, given how
previous_pointandnext_pointare written, so compare against the final atom's start point directly.Narrow cells behave exactly as before.
end_col == start.colmakes the new expression equivalent to the old one, the SpacerHead atom (end_col: col) is the same, and the hard-line-break atom is dropped byfindbecause itspointisNone. There is also no case where only the old expression matched: a wide cell'sstart.col + 1is the spacer tail position, and no atom starts there.Termination is unchanged.
window_rowssaturates attotal_rows, and at that point eitherends_in_continuationgoes false orreached_edgegoes true.Testing
Added
live_terminal_word_end_expands_through_a_long_wide_soft_wrap, the wide-glyph twin of the adjacentlive_terminal_word_end_expands_through_a_long_soft_wrap. Before the fix the motion returns row 63 and the test fails; after the fix it passes.just checkpasses.Docs
The only user-visible change is that something broken starts working, and the copy-mode section in
keyboard.mdxsays nothing about wide characters, so I don't think this needs a docs update.Related
#1000 is also copy mode and full-width characters, but it is a different bug (single-cell
h/lmovement stopping mid-glyph) and this PR does not fix it.