TUI detail pane clips long lines; wrap like vim instead #79

Closed
opened 2026-08-12 19:04:27 +00:00 by a73x · 2 comments
a73x commented 2026-08-12 19:04:27 +00:00 (Migrated from git.a73x.sh)

07a78a60 turned wrapping off on the patch detail pane. The comment at widgets.rs:440 explains why:

No wrapping: a wrapped line occupies more rows than it has, which would put the cursor's highlight on a different line than the one c is about to comment on.

That is true of Paragraph::wrap, and not of wrapping in general. Paragraph wraps after it is handed lines, exposes no mapping back to logical lines, and both scroll and 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 own RowTarget (Diff { file, line } or Comment { 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 say Diff { file, line: 305 }; the cursor may land on any of them and c still 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-width is 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; tig makes 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 b3ddfe29

07a78a60 turned wrapping off on the patch detail pane. The comment at widgets.rs:440 explains why: > No wrapping: a wrapped line occupies more rows than it has, which would put the cursor's highlight on a different line than the one `c` is about to comment on. That is true of `Paragraph::wrap`, and not of wrapping in general. `Paragraph` wraps *after* it is handed lines, exposes no mapping back to logical lines, and both `scroll` and 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 own `RowTarget` (`Diff { file, line }` or `Comment { 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 `DetailRow`s that all say `Diff { file, line: 305 }`; the cursor may land on any of them and `c` still 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-width` is 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; `tig` makes 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 b3ddfe29
a73x commented 2026-08-13 04:37:03 +00:00 (Migrated from git.a73x.sh)

Decided: 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 into patch_rows and 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/k should move by logical line, not display row — vim's own default, which is why gj exists as the opposite. With N display rows per logical line all carrying the same RowTarget, that is "advance past the run of rows sharing this target", and it keeps c landing where the reader thinks the cursor is. Without it, crossing one wrapped line takes several presses and the cursor appears to stall.

Revised shape:

  • Wrap on by default, everywhere in the pane, prose and diff alike.
  • A key toggles no-wrap for the diff, for the alignment case; remember it for the session.
  • j/k move by logical line; consider gj/gk for display rows if anyone wants them.
  • When no-wrap is active, mark truncated lines — the silent clipping is a defect in its own right and outlives whichever default is chosen.
**Decided: 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 into `patch_rows` and 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`/`k` should move by logical line, not display row** — vim's own default, which is why `gj` exists as the opposite. With N display rows per logical line all carrying the same `RowTarget`, that is "advance past the run of rows sharing this target", and it keeps `c` landing where the reader thinks the cursor is. Without it, crossing one wrapped line takes several presses and the cursor appears to stall. Revised shape: - Wrap on by default, everywhere in the pane, prose and diff alike. - A key toggles no-wrap for the diff, for the alignment case; remember it for the session. - `j`/`k` move by logical line; consider `gj`/`gk` for display rows if anyone wants them. - When no-wrap **is** active, mark truncated lines — the silent clipping is a defect in its own right and outlives whichever default is chosen.
a73x commented 2026-08-13 05:09:56 +00:00 (Migrated from git.a73x.sh)

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: j jumps the cursor past rows the reader never saw. Vim keeps them separate, which is what Ctrl-E/Ctrl-Y are 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.

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: `j` jumps the cursor past rows the reader never saw. Vim keeps them separate, which is what `Ctrl-E`/`Ctrl-Y` are 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.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
a73x/git-collab#79
No description provided.