TUI detail pane clips long lines; wrap like vim instead #79
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
07a78a60turned wrapping off on the patch detail pane. The comment at widgets.rs:440 explains why:That is true of
Paragraph::wrap, and not of wrapping in general.Paragraphwraps after it is handed lines, exposes no mapping back to logical lines, and bothscrolland the cursor are raw row offsets into the post-wrap output. So the mapping breaks.The pane is already built for the vim answer. It renders a flat
Vec<DetailRow>, and each row carries its ownRowTarget(Diff { file, line }orComment { oid }); anchoring reads the target of whichever row the cursor is on. Nothing requires one row per logical line.So: wrap it ourselves and emit N rows per logical line, all sharing one target. A wrapped diff line becomes three
DetailRows that all sayDiff { file, line: 305 }; the cursor may land on any of them andcstill anchors to 305. Highlighting the whole logical line is highlighting the run of rows with the same target — which is what vim does with a wrapped line.unicode-widthis already in the tree via ratatui, so width-aware wrapping needs no new dependency. Re-wrap on resize; the pane is already rebuilt when the view is built.Probably a toggle rather than a mode. Diff tools default to no-wrap because
+/-columns align and wrapping destroys that;tigmakes it a key. Prose — an issue body, a review, a comment — wants wrapping and currently loses text off the right edge. Suggest: wrap on for prose sections, off for the diff, and a key to flip the diff, with the choice remembered for the session.Whatever is chosen, clipping must stop being silent: today a long line simply ends and nothing indicates there is more. Even without wrapping, mark truncation.
Raised by the user on reviewing 07a78a60's tradeoff.
Migrated from git-collab issue
9f9dee3138c64d9d9c92c2030f2910921316061c.Original author: a73x dev@a73x.sh.
Close reason: merged in patch
b3ddfe29Decided: wrap by default. The toggle is the escape hatch for the alignment case, not the other way round.
The reason is that the argument for no-wrap does not apply here. In vim it rests substantially on column navigation —
$,0,w, visual block all need a stable column, and wrapping complicates each. This pane has no column.patch_cursor: usize(state.rs:127) is an index intopatch_rowsand that is the entire cursor; there is no horizontal movement and no column-wise operation anywhere in the TUI.So the only cost of wrapping is visual alignment of
+/-while reading. Against that: text silently vanishing off the right edge, in a tool whose whole job is reading code carefully. A reviewer who cannot see the end of a line cannot review it. Alignment loses.j/kshould move by logical line, not display row — vim's own default, which is whygjexists as the opposite. With N display rows per logical line all carrying the sameRowTarget, that is "advance past the run of rows sharing this target", and it keepsclanding where the reader thinks the cursor is. Without it, crossing one wrapped line takes several presses and the cursor appears to stall.Revised shape:
j/kmove by logical line; considergj/gkfor display rows if anyone wants them.One more detail, and it is where this usually goes wrong: cursor movement is per logical line, scrolling stays per row.
A logical line can wrap to more display rows than the pane is tall — a long diff line in a narrow pane, or a comment body. If cursor movement and scrolling are the same mechanism, the middle of such a line is unreachable:
jjumps the cursor past rows the reader never saw. Vim keeps them separate, which is whatCtrl-E/Ctrl-Yare for.So
follow_cursor(state.rs) must scroll far enough to bring the cursor's logical line into view without assuming the whole line fits, and there should be a way to scroll rows independently of the cursor for the over-tall case.