Wrap the patch detail pane, and show dates the way the web does #84

Merged

a73x merged 1 commit from worktree-agent-a2345ee9684496dbd into main

2026-08-13 07:34:53 +00:00

a73x commented

2026-08-13 07:29:37 +00:00

(Migrated from git.a73x.sh)

Fixes 9f9dee31 (wrapping) and 952390ad (timestamps) — both display
defects in the dashboard, both in src/tui/.

`9f9dee31` — wrap by default, and own the wrap

07a78a60 turned wrapping off because Paragraph::wrap wraps after it
is handed lines and exposes no mapping back to the logical line. So
the pane now wraps itself: logical lines are built by the key handler
as before, and the renderer lays them out into display rows that each
name the line they came from.

The cursor became a logical index rather than a row index. That is
what makes the settled decisions fall out rather than be bolted on:

j/k cross a wrapped line in one press.
The highlight covers the whole run of rows the line wrapped to.
c anchors to the same file:line from any part of it — there is
one target, on the line, and the rows point at it. DisplayRow
deliberately does not carry a copy, so rows cannot disagree with
the line they came from.

Cursor movement is per logical line, scrolling stays per display row.
Landing on a line taller than the pane shows its start; Ctrl-E /
Ctrl-Y walk rows without moving the cursor. follow_cursor also
leaves the view alone while the reader is part way through such a
line, instead of snapping back to its first row.

w toggles no-wrap for the diff (prose keeps wrapping — the +/-
alignment argument is not about an issue body) and sticks for the
session, surviving a revision change or a rebuild. With it off, a
clipped line ends in ›.

Two things fell out of doing the layout by hand:

git hands the whole diff --git / index / --- a/f file header
over as one diff row with newlines inside it. A Line cannot hold a
newline, so those arrived mashed onto one screen row — and such a
line's width would have lied to the wrapper. Now one row per
physical line, each keeping its anchor.
w:wrap went at the end of the footer so it does not evict
o:checkout; the tail already clips at 100 columns either way.

`952390ad` — timestamps

The Timestamp helper moves from the server binary into the library
(src/timestamp.rs; http::timestamp is now a re-export, so every
existing path still resolves) and the TUI uses it everywhere a stored
date reaches the screen: issue detail, issue comments, linked commits,
the patch detail's revisions / reviews / thread comments, the event
list and the event detail. Checked every pane, not just the two the
issue named. No hover in a terminal, so the short form only.

Testing

TDD throughout — each test written first and watched fail.

Wrapping is asserted as presence, not as row arithmetic: a 209-column
line is split into the column runs a pane of the recorded width would
show it in, and every run is required to be somewhere in the rendered
TestBackend buffer. Separately the display rows of one line are
required to reassemble into exactly that line with none wider than the
pane. Both ends carry sentinels. There is a pty test too, since the
clipping was a property of the pane as drawn: a 300-column diff line
whose tail is a distinct token, and the token has to reach the screen.

I mutated the implementation to check the tests bite: disabling
wrapping fails 9 of them, and dropping the .min(first) from
follow_cursor fails the over-tall test with the exact off-by-a-line
it is there to catch (scrolled to 36, the line's end, instead of 34,
its start).

Also driven through a real pty at 100x30 and 44x14 and the screens
read back: wrapping, the › after w, the split file header, and
Ctrl-E walking through a line ten rows tall in a nine-row pane.

What I could not verify

Nobody has run this in a human terminal. Everything above is a
TestBackend buffer or a pty replay — real terminal emulators,
resizing by hand, and wide/combining characters in a live diff are
untested. The wrapper is width-aware via unicode-width and has unit
coverage for degenerate widths, but no CJK or emoji diff was rendered.

tests/tui_review_test.rs has a pre-existing flake: at default test
parallelism two editor-suspend tests hang until the harness deadline.
It is not from this change — I A/B'd it interleaved on the
unmodified tree, which failed 3 of 3 runs of the whole file and 2 of 4
runs of just those two tests, against 0 of 4 for this branch. Serially
the file is 14 of 14 green. A wider sleep budget does not fix it, so
it is not a simple timing budget; it wants its own issue.

unicode-width becomes a direct dependency — already in the tree via
ratatui at the same version, so one line of lockfile and no new crate.

Migrated from git-collab patche b3ddfe29bf53c72ea1c243b133aec907f0d715e7.

Original author: a73x dev@a73x.sh.

Fixes #79

Revision 1: a573c8141b8a906415c8e44c623c8568b0424925; base 15bc9a3c2f8c1622b6eb5157c6176452ba7569bb

Fixes 9f9dee31 (wrapping) and 952390ad (timestamps) — both display defects in the dashboard, both in src/tui/. ## 9f9dee31 — wrap by default, and own the wrap 07a78a60 turned wrapping off because `Paragraph::wrap` wraps after it is handed lines and exposes no mapping back to the logical line. So the pane now wraps itself: logical lines are built by the key handler as before, and the renderer lays them out into display rows that each name the line they came from. The cursor became a *logical* index rather than a row index. That is what makes the settled decisions fall out rather than be bolted on: - `j`/`k` cross a wrapped line in one press. - The highlight covers the whole run of rows the line wrapped to. - `c` anchors to the same `file:line` from any part of it — there is one target, on the line, and the rows point at it. `DisplayRow` deliberately does *not* carry a copy, so rows cannot disagree with the line they came from. Cursor movement is per logical line, scrolling stays per display row. Landing on a line taller than the pane shows its *start*; `Ctrl-E` / `Ctrl-Y` walk rows without moving the cursor. `follow_cursor` also leaves the view alone while the reader is part way through such a line, instead of snapping back to its first row. `w` toggles no-wrap for the diff (prose keeps wrapping — the `+`/`-` alignment argument is not about an issue body) and sticks for the session, surviving a revision change or a rebuild. With it off, a clipped line ends in `›`. Two things fell out of doing the layout by hand: - git hands the whole `diff --git` / `index` / `--- a/f` file header over as one diff row with newlines inside it. A `Line` cannot hold a newline, so those arrived mashed onto one screen row — and such a line's width would have lied to the wrapper. Now one row per physical line, each keeping its anchor. - `w:wrap` went at the *end* of the footer so it does not evict `o:checkout`; the tail already clips at 100 columns either way. ## 952390ad — timestamps The `Timestamp` helper moves from the server binary into the library (`src/timestamp.rs`; `http::timestamp` is now a re-export, so every existing path still resolves) and the TUI uses it everywhere a stored date reaches the screen: issue detail, issue comments, linked commits, the patch detail's revisions / reviews / thread comments, the event list and the event detail. Checked every pane, not just the two the issue named. No hover in a terminal, so the short form only. ## Testing TDD throughout — each test written first and watched fail. Wrapping is asserted as *presence*, not as row arithmetic: a 209-column line is split into the column runs a pane of the recorded width would show it in, and every run is required to be somewhere in the rendered `TestBackend` buffer. Separately the display rows of one line are required to reassemble into exactly that line with none wider than the pane. Both ends carry sentinels. There is a pty test too, since the clipping was a property of the pane as drawn: a 300-column diff line whose tail is a distinct token, and the token has to reach the screen. I mutated the implementation to check the tests bite: disabling wrapping fails 9 of them, and dropping the `.min(first)` from `follow_cursor` fails the over-tall test with the exact off-by-a-line it is there to catch (scrolled to 36, the line's end, instead of 34, its start). Also driven through a real pty at 100x30 and 44x14 and the screens read back: wrapping, the `›` after `w`, the split file header, and `Ctrl-E` walking through a line ten rows tall in a nine-row pane. ## What I could not verify Nobody has run this in a human terminal. Everything above is a `TestBackend` buffer or a pty replay — real terminal emulators, resizing by hand, and wide/combining characters in a live diff are untested. The wrapper is width-aware via `unicode-width` and has unit coverage for degenerate widths, but no CJK or emoji diff was rendered. `tests/tui_review_test.rs` has a pre-existing flake: at default test parallelism two editor-suspend tests hang until the harness deadline. It is **not** from this change — I A/B'd it interleaved on the unmodified tree, which failed 3 of 3 runs of the whole file and 2 of 4 runs of just those two tests, against 0 of 4 for this branch. Serially the file is 14 of 14 green. A wider sleep budget does not fix it, so it is not a simple timing budget; it wants its own issue. `unicode-width` becomes a direct dependency — already in the tree via ratatui at the same version, so one line of lockfile and no new crate. --- Migrated from git-collab patche `b3ddfe29bf53c72ea1c243b133aec907f0d715e7`. Original author: a73x <dev@a73x.sh>. Fixes #79 Revision 1: `a573c8141b8a906415c8e44c623c8568b0424925`; base `15bc9a3c2f8c1622b6eb5157c6176452ba7569bb`