a73x

65e0e821

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

a73x   2026-08-13 07:30

Commit message
Wrap the patch detail pane, and show dates the way the web does

Two display defects in the dashboard.

**Wrapping (9f9dee31).** 07a78a60 turned wrapping off because
`Paragraph::wrap` wraps *after* it is handed lines and exposes no
mapping back to the logical line, so the cursor — a raw row offset —
could sit on a different line than `c` was about to comment on. Long
lines have been clipped silently since.

The answer is not to stop wrapping but to own it. The pane now builds
logical lines and lays them out itself: N display rows per line, each
naming the line it came from. The cursor became an index into the
logical lines rather than the rows, which is what makes the rest fall
out — `j`/`k` cross a wrapped line in one press (vim's default, which
is why `gj` exists), the highlight covers the whole run, and `c`
anchors to the same `file:line` from any part of it. This pane has no
column — `patch_cursor` is the entire cursor and there is no
horizontal movement in the TUI — so the only cost of wrapping is
`+`/`-` alignment, against text vanishing off the right edge of a
review tool.

Scrolling stays per display row, because a line can wrap taller than
the pane: landing on such a line shows its *start* and `Ctrl-E` /
`Ctrl-Y` walk through the rest, rather than `j` jumping the view past
rows nobody saw. `w` turns wrapping off for the diff — the alignment
case — and sticks for the session; with it off a clipped line ends in
`›`, because silent clipping is a defect in its own right whichever
default wins.

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, which a `Line` cannot hold: those arrived
mashed onto one screen row, and such a line's width would have lied to
the wrapper. It is now one row per physical line, each keeping the
anchor of the row it came from. And `DisplayRow` deliberately does not
carry a copy of the target — one owner means the rows cannot disagree
with the line they were wrapped from.

**Timestamps (952390ad).** The TUI still printed the stored string,
`2026-08-12T16:57:16.325870642+00:00`, after b2a57996 had replaced
exactly that on the web with `2026-08-12 16:57`. That is the
divergence 10bb2d84 says must not happen for a displayed value. The
`Timestamp` helper moves from the server binary into the library so
both surfaces share it, and every pane that shows a date now uses it —
issue detail, its comments and linked commits, the patch detail's
revisions, reviews and comments, and both commit-browser views. A
terminal has no hover, so the short form is all there is; the stored
value is still one `show` or `--json` away.

`unicode-width` becomes a direct dependency. It was already in the
tree via ratatui at the same version, so this adds a name and not a
crate — one line of lockfile.

Fixes 9f9dee31
Fixes 952390ad

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cargo.lock
Old New
@@ -1630,6 +1630,7 @@ dependencies = [
1630 "toml_edit", 1630 "toml_edit",
1631 "tracing", 1631 "tracing",
1632 "tracing-subscriber", 1632 "tracing-subscriber",
1633 "unicode-width",
1633 "zeroize", 1634 "zeroize",
1634 ] 1635 ]
1635 1636
Cargo.toml
Old New
@@ -47,6 +47,12 @@ sha2 = "0.10"
47 # can push the config, so the engine must be one with a linear-time guarantee 47 # can push the config, so the engine must be one with a linear-time guarantee
48 # rather than a backtracker. Already in the tree via tracing-subscriber. 48 # rather than a backtracker. Already in the tree via tracing-subscriber.
49 regex = "1" 49 regex = "1"
50 # The dashboard wraps the patch detail pane itself rather than letting
51 # `Paragraph` do it, because it needs the mapping from display rows back to
52 # logical lines that `Paragraph` does not expose. Wrapping by column means
53 # knowing how many columns a character occupies. Already in the tree via
54 # ratatui, at the same version, so this adds a name and not a crate.
55 unicode-width = "0.2"
50 tempfile = "3" 56 tempfile = "3"
51 tokio-util = { version = "0.7", features = ["io"] } 57 tokio-util = { version = "0.7", features = ["io"] }
52 58
src/lib.rs
Old New
@@ -21,6 +21,7 @@ pub mod status;
21 pub mod sync; 21 pub mod sync;
22 pub mod sync_lock; 22 pub mod sync_lock;
23 pub mod timeline; 23 pub mod timeline;
24 pub mod timestamp;
24 pub mod trailer; 25 pub mod trailer;
25 pub mod trust; 26 pub mod trust;
26 pub mod tui; 27 pub mod tui;
src/server/http/mod.rs
Old New
@@ -1,7 +1,11 @@
1 pub mod git_http; 1 pub mod git_http;
2 pub mod repo; 2 pub mod repo;
3 pub mod repo_list; 3 pub mod repo_list;
4 pub mod timestamp; 4 // The same helper the dashboard renders with. It lives in the library because
5 // two surfaces showing one stored field two ways is the defect `952390ad`
6 // reported: a reader moves between them, and a screenshot from one gets pasted
7 // next to output from the other.
8 pub use git_collab::timestamp;
5 9
6 use axum::extract::{DefaultBodyLimit, Request, State}; 10 use axum::extract::{DefaultBodyLimit, Request, State};
7 use axum::http::uri::{PathAndQuery, Uri}; 11 use axum::http::uri::{PathAndQuery, Uri};
src/server/http/timestamp.rs
Old New
@@ -1,117 +0,0 @@
1 //! How a stored timestamp goes onto a page.
2 //!
3 //! Events store `chrono::Utc::now().to_rfc3339()`, which is 35 characters of
4 //! which nine are nanoseconds:
5 //!
6 //! ```text
7 //! 2026-08-11T16:08:31.458124482+00:00
8 //! ```
9 //!
10 //! That is the right thing to *store* — it is exact, sortable and unambiguous
11 //! about its offset — and the wrong thing to put in a table cell. At 35
12 //! characters it was the widest column after the title, it wrapped every row
13 //! onto two lines, and the nine digits of precision are not a number any
14 //! reader has ever needed. See issue `b2a57996`.
15 //!
16 //! So a timestamp reaching a template is split in two: a short form for the
17 //! cell and the stored form for `title=`, which browsers surface on hover.
18 //! Nothing is discarded — the exact value is still on the page, one hover
19 //! away, and `--json` is untouched and still carries it in full.
20 //!
21 //! # Why minutes, and why absolute
22 //!
23 //! Minute precision is the coarsest rendering that still orders two events on
24 //! the same day, which is the common case for a review and the comment
25 //! answering it. Seconds add three characters and settle nothing a reader
26 //! cares about.
27 //!
28 //! Absolute rather than relative ("3 days ago") because a relative rendering
29 //! is a function of when the page was built, so the same URL says different
30 //! things at different times: it cannot be cached, cannot be compared between
31 //! two rows rendered from different requests, and cannot be asserted by a test
32 //! without freezing a clock. The exact instant is what was recorded; the page
33 //! should say it.
34 //!
35 //! # Why UTC
36 //!
37 //! The server cannot know the reader's timezone, and a page rendered in the
38 //! *server's* zone would silently disagree with the same page rendered
39 //! elsewhere. Every timestamp this project writes is already `+00:00`, so
40 //! normalizing to UTC changes nothing in practice and fixes the display for a
41 //! value that arrived with some other offset from a repository synced in.
42
43 /// A stored timestamp, split into what a cell shows and what it stands for.
44 #[derive(Debug, Clone)]
45 pub struct Timestamp {
46 /// Exactly as stored — the `title` attribute, so nothing is lost.
47 pub full: String,
48 /// `YYYY-MM-DD HH:MM` in UTC.
49 pub short: String,
50 }
51
52 impl Timestamp {
53 /// Split a stored RFC3339 timestamp for display.
54 ///
55 /// An unparseable value renders as itself. A timestamp this project did
56 /// not write is still a fact about the object, and a page that dropped it
57 /// or printed a placeholder would be hiding the one thing that could
58 /// explain the row. Ugly beats absent.
59 pub fn new(stored: impl Into<String>) -> Self {
60 let full = stored.into();
61 let short = chrono::DateTime::parse_from_rfc3339(&full)
62 .map(|dt| {
63 dt.with_timezone(&chrono::Utc)
64 .format("%Y-%m-%d %H:%M")
65 .to_string()
66 })
67 .unwrap_or_else(|_| full.clone());
68 Timestamp { full, short }
69 }
70 }
71
72 #[cfg(test)]
73 mod tests {
74 use super::*;
75
76 #[test]
77 fn nanoseconds_and_the_offset_are_dropped_from_the_short_form() {
78 let t = Timestamp::new("2026-08-11T16:08:31.458124482+00:00");
79 assert_eq!(t.short, "2026-08-11 16:08");
80 assert_eq!(t.short.len(), 16);
81 }
82
83 #[test]
84 fn the_stored_value_survives_for_the_title_attribute() {
85 let stored = "2026-08-11T16:08:31.458124482+00:00";
86 assert_eq!(Timestamp::new(stored).full, stored);
87 }
88
89 /// A value that arrived with a non-UTC offset must not read as though its
90 /// wall-clock digits were UTC — two rows in one table have to be
91 /// comparable by eye.
92 #[test]
93 fn a_foreign_offset_is_normalized_to_utc() {
94 let t = Timestamp::new("2026-08-11T18:08:31+02:00");
95 assert_eq!(t.short, "2026-08-11 16:08");
96 }
97
98 #[test]
99 fn a_second_precision_timestamp_shortens_too() {
100 assert_eq!(
101 Timestamp::new("2026-08-11T16:08:31Z").short,
102 "2026-08-11 16:08"
103 );
104 }
105
106 #[test]
107 fn an_unparseable_timestamp_renders_as_itself_rather_than_vanishing() {
108 let t = Timestamp::new("not a date");
109 assert_eq!(t.short, "not a date");
110 assert_eq!(t.full, "not a date");
111 }
112
113 #[test]
114 fn an_empty_timestamp_stays_empty() {
115 assert_eq!(Timestamp::new("").short, "");
116 }
117 }
src/timestamp.rs
Old New
@@ -0,0 +1,126 @@
1 //! How a stored timestamp goes onto a screen.
2 //!
3 //! Events store `chrono::Utc::now().to_rfc3339()`, which is 35 characters of
4 //! which nine are nanoseconds:
5 //!
6 //! ```text
7 //! 2026-08-11T16:08:31.458124482+00:00
8 //! ```
9 //!
10 //! That is the right thing to *store* — it is exact, sortable and unambiguous
11 //! about its offset — and the wrong thing to put in a table cell. At 35
12 //! characters it was the widest column after the title, it wrapped every row
13 //! onto two lines, and the nine digits of precision are not a number any
14 //! reader has ever needed. See issue `b2a57996`.
15 //!
16 //! So a timestamp reaching a template is split in two: a short form for the
17 //! cell and the stored form for `title=`, which browsers surface on hover.
18 //! Nothing is discarded — the exact value is still on the page, one hover
19 //! away, and `--json` is untouched and still carries it in full.
20 //!
21 //! # Why the dashboard uses it too
22 //!
23 //! Issue `952390ad`: the TUI kept printing the stored string after the web had
24 //! stopped, which is the divergence `10bb2d84` says must not happen for a
25 //! displayed value — two surfaces rendering one field two ways teach two
26 //! different habits and make screenshots and pasted output disagree. A
27 //! terminal has no hover, so the dashboard shows [`Timestamp::short`] and
28 //! nothing else; the full value is still one `show` or `--json` away.
29 //!
30 //! # Why minutes, and why absolute
31 //!
32 //! Minute precision is the coarsest rendering that still orders two events on
33 //! the same day, which is the common case for a review and the comment
34 //! answering it. Seconds add three characters and settle nothing a reader
35 //! cares about.
36 //!
37 //! Absolute rather than relative ("3 days ago") because a relative rendering
38 //! is a function of when the page was built, so the same URL says different
39 //! things at different times: it cannot be cached, cannot be compared between
40 //! two rows rendered from different requests, and cannot be asserted by a test
41 //! without freezing a clock. The exact instant is what was recorded; the page
42 //! should say it.
43 //!
44 //! # Why UTC
45 //!
46 //! The server cannot know the reader's timezone, and a page rendered in the
47 //! *server's* zone would silently disagree with the same page rendered
48 //! elsewhere. Every timestamp this project writes is already `+00:00`, so
49 //! normalizing to UTC changes nothing in practice and fixes the display for a
50 //! value that arrived with some other offset from a repository synced in.
51
52 /// A stored timestamp, split into what a cell shows and what it stands for.
53 #[derive(Debug, Clone)]
54 pub struct Timestamp {
55 /// Exactly as stored — the `title` attribute, so nothing is lost.
56 pub full: String,
57 /// `YYYY-MM-DD HH:MM` in UTC.
58 pub short: String,
59 }
60
61 impl Timestamp {
62 /// Split a stored RFC3339 timestamp for display.
63 ///
64 /// An unparseable value renders as itself. A timestamp this project did
65 /// not write is still a fact about the object, and a page that dropped it
66 /// or printed a placeholder would be hiding the one thing that could
67 /// explain the row. Ugly beats absent.
68 pub fn new(stored: impl Into<String>) -> Self {
69 let full = stored.into();
70 let short = chrono::DateTime::parse_from_rfc3339(&full)
71 .map(|dt| {
72 dt.with_timezone(&chrono::Utc)
73 .format("%Y-%m-%d %H:%M")
74 .to_string()
75 })
76 .unwrap_or_else(|_| full.clone());
77 Timestamp { full, short }
78 }
79 }
80
81 #[cfg(test)]
82 mod tests {
83 use super::*;
84
85 #[test]
86 fn nanoseconds_and_the_offset_are_dropped_from_the_short_form() {
87 let t = Timestamp::new("2026-08-11T16:08:31.458124482+00:00");
88 assert_eq!(t.short, "2026-08-11 16:08");
89 assert_eq!(t.short.len(), 16);
90 }
91
92 #[test]
93 fn the_stored_value_survives_for_the_title_attribute() {
94 let stored = "2026-08-11T16:08:31.458124482+00:00";
95 assert_eq!(Timestamp::new(stored).full, stored);
96 }
97
98 /// A value that arrived with a non-UTC offset must not read as though its
99 /// wall-clock digits were UTC — two rows in one table have to be
100 /// comparable by eye.
101 #[test]
102 fn a_foreign_offset_is_normalized_to_utc() {
103 let t = Timestamp::new("2026-08-11T18:08:31+02:00");
104 assert_eq!(t.short, "2026-08-11 16:08");
105 }
106
107 #[test]
108 fn a_second_precision_timestamp_shortens_too() {
109 assert_eq!(
110 Timestamp::new("2026-08-11T16:08:31Z").short,
111 "2026-08-11 16:08"
112 );
113 }
114
115 #[test]
116 fn an_unparseable_timestamp_renders_as_itself_rather_than_vanishing() {
117 let t = Timestamp::new("not a date");
118 assert_eq!(t.short, "not a date");
119 assert_eq!(t.full, "not a date");
120 }
121
122 #[test]
123 fn an_empty_timestamp_stays_empty() {
124 assert_eq!(Timestamp::new("").short, "");
125 }
126 }
src/tui/events.rs
Old New
@@ -288,11 +288,15 @@ fn comment_seed(headline: &str, context: Vec<String>) -> String {
288 seed 288 seed
289 } 289 }
290 290
291 /// The rows around the cursor, as `#` context for the editor buffer. 291 /// The lines around the cursor, as `#` context for the editor buffer.
292 ///
293 /// Logical lines, not screen rows: the reviewer is being shown what they are
294 /// writing about, and a line broken across three rows by the width of a pane
295 /// they are no longer looking at is not that.
292 fn cursor_context(app: &App) -> Vec<String> { 296 fn cursor_context(app: &App) -> Vec<String> {
293 let start = app.patch_cursor.saturating_sub(5); 297 let start = app.patch_cursor.saturating_sub(5);
294 let end = (app.patch_cursor + 4).min(app.patch_rows.len()); 298 let end = (app.patch_cursor + 4).min(app.patch_lines.len());
295 app.patch_rows[start..end] 299 app.patch_lines[start..end]
296 .iter() 300 .iter()
297 .enumerate() 301 .enumerate()
298 .map(|(i, row)| { 302 .map(|(i, row)| {
src/tui/mod.rs
Old New
@@ -293,7 +293,11 @@ mod tests {
293 } 293 }
294 294
295 fn render_app(app: &mut App) -> Buffer { 295 fn render_app(app: &mut App) -> Buffer {
296 let backend = TestBackend::new(80, 24); 296 render_app_sized(app, 80, 24)
297 }
298
299 fn render_app_sized(app: &mut App, cols: u16, rows: u16) -> Buffer {
300 let backend = TestBackend::new(cols, rows);
297 let mut terminal = Terminal::new(backend).unwrap(); 301 let mut terminal = Terminal::new(backend).unwrap();
298 terminal.draw(|frame| ui(frame, app, None)).unwrap(); 302 terminal.draw(|frame| ui(frame, app, None)).unwrap();
299 terminal.backend().buffer().clone() 303 terminal.backend().buffer().clone()
@@ -461,7 +465,8 @@ mod tests {
461 let detail = format_event_detail(&oid, &event, &Abbrev::minimal()); 465 let detail = format_event_detail(&oid, &event, &Abbrev::minimal());
462 assert!(detail.contains("aaaaaaa")); 466 assert!(detail.contains("aaaaaaa"));
463 assert!(detail.contains("Test User <test@example.com>")); 467 assert!(detail.contains("Test User <test@example.com>"));
464 assert!(detail.contains("2026-01-01T00:00:00Z")); 468 // The short form, not the stored one: see `an_issues_created_date...`.
469 assert!(detail.contains("2026-01-01 00:00"));
465 assert!(detail.contains("Issue Open")); 470 assert!(detail.contains("Issue Open"));
466 assert!(detail.contains("Title: My Issue")); 471 assert!(detail.contains("Title: My Issue"));
467 assert!(detail.contains("Description here")); 472 assert!(detail.contains("Description here"));
@@ -1272,7 +1277,9 @@ mod tests {
1272 app.mode = ViewMode::PatchDetail; 1277 app.mode = ViewMode::PatchDetail;
1273 app.current_patch = Some(make_patch_with_revisions()); 1278 app.current_patch = Some(make_patch_with_revisions());
1274 app.rebuild_patch_rows(); 1279 app.rebuild_patch_rows();
1275 let last = app.patch_rows.len() - 1; 1280 // Logical lines: the cursor indexes those, not the screen rows they
1281 // wrap to.
1282 let last = app.patch_lines.len() - 1;
1276 1283
1277 for _ in 0..500 { 1284 for _ in 0..500 {
1278 app.handle_key( 1285 app.handle_key(
@@ -1540,7 +1547,7 @@ mod tests {
1540 app.rebuild_patch_rows(); 1547 app.rebuild_patch_rows();
1541 1548
1542 let anchored: Vec<&RowTarget> = app 1549 let anchored: Vec<&RowTarget> = app
1543 .patch_rows 1550 .patch_lines
1544 .iter() 1551 .iter()
1545 .map(|r| &r.target) 1552 .map(|r| &r.target)
1546 .filter(|t| matches!(t, RowTarget::Diff { .. })) 1553 .filter(|t| matches!(t, RowTarget::Diff { .. }))
@@ -1571,7 +1578,7 @@ mod tests {
1571 app.rebuild_patch_rows(); 1578 app.rebuild_patch_rows();
1572 1579
1573 assert!(app 1580 assert!(app
1574 .patch_rows 1581 .patch_lines
1575 .iter() 1582 .iter()
1576 .all(|r| !matches!(r.target, RowTarget::Diff { .. }))); 1583 .all(|r| !matches!(r.target, RowTarget::Diff { .. })));
1577 } 1584 }
@@ -1589,7 +1596,7 @@ mod tests {
1589 app.rebuild_patch_rows(); 1596 app.rebuild_patch_rows();
1590 1597
1591 let found = app 1598 let found = app
1592 .patch_rows 1599 .patch_lines
1593 .iter() 1600 .iter()
1594 .find_map(|r| match &r.target { 1601 .find_map(|r| match &r.target {
1595 RowTarget::Comment { oid, resolved } => Some((oid.clone(), *resolved)), 1602 RowTarget::Comment { oid, resolved } => Some((oid.clone(), *resolved)),
@@ -1671,6 +1678,618 @@ mod tests {
1671 assert_buffer_contains(&buf, "1 new event "); 1678 assert_buffer_contains(&buf, "1 new event ");
1672 } 1679 }
1673 1680
1681 // ── Wrapping the patch detail pane (9f9dee31) ────────────────────────
1682 //
1683 // `07a78a60` turned wrapping off because `Paragraph::wrap` wraps *after* it
1684 // is handed lines and exposes no mapping back to the logical line, so the
1685 // cursor — a raw row offset — landed on a different line than `c` would
1686 // comment on. The answer is not to stop wrapping but to own it: emit N
1687 // display rows per logical line, all carrying that line's target, and move
1688 // the cursor by logical line the way vim does.
1689
1690 /// A patch detail pane whose diff is `n` anchored lines, each far wider
1691 /// than any pane, with the two ends marked so a test can see both.
1692 fn app_with_long_diff_lines(n: u32) -> App {
1693 let mut app = make_app(0, 0);
1694 app.mode = ViewMode::PatchDetail;
1695 app.current_patch = Some(make_patch_with_revisions());
1696 app.patch_diff = (0..n)
1697 .map(|i| crate::patch::DiffRow {
1698 text: format!("+HEAD{}{}TAIL{}\n", i, "x".repeat(200), i),
1699 body: true,
1700 anchor: Some(("src/main.rs".to_string(), 305 + i)),
1701 })
1702 .collect();
1703 app
1704 }
1705
1706 /// The text of one logical line as it was handed to the pane.
1707 fn logical_text(app: &App, logical: usize) -> String {
1708 app.patch_lines[logical].line.to_string()
1709 }
1710
1711 fn display_rows_of(app: &App, logical: usize) -> Vec<&DisplayRow> {
1712 app.patch_rows
1713 .iter()
1714 .filter(|r| r.logical == logical)
1715 .collect()
1716 }
1717
1718 /// The index of the logical line holding the diff line anchored at `line`.
1719 fn logical_index_of_diff_line(app: &App, line: u32) -> usize {
1720 app.patch_lines
1721 .iter()
1722 .position(|r| {
1723 matches!(&r.target, RowTarget::Diff { line: l, .. } if *l == line)
1724 })
1725 .expect("a diff line with that anchor")
1726 }
1727
1728 /// Split `s` into the runs of characters a pane `width` columns wide would
1729 /// show it in, so a test can look for every one of them on screen.
1730 fn column_chunks(s: &str, width: usize) -> Vec<String> {
1731 let mut out = vec![String::new()];
1732 for c in s.chars() {
1733 if out.last().unwrap().chars().count() == width {
1734 out.push(String::new());
1735 }
1736 out.last_mut().unwrap().push(c);
1737 }
1738 out
1739 }
1740
1741 /// The property the issue is about: a line longer than the pane must be
1742 /// **fully present**, not clipped at the right edge with no sign that
1743 /// anything is missing. Asserted against the rendered cells — every column
1744 /// run of the logical line has to be somewhere on the screen — rather than
1745 /// against the row list, so that it is the reader's view being checked.
1746 #[test]
1747 fn a_line_longer_than_the_pane_is_fully_on_screen() {
1748 let mut app = app_with_long_diff_lines(1);
1749 app.rebuild_patch_rows();
1750 let buf = render_app_sized(&mut app, 80, 60);
1751
1752 let width = app.patch_width as usize;
1753 assert!(width > 0 && width < 100, "implausible pane width {}", width);
1754
1755 let logical = logical_index_of_diff_line(&app, 305);
1756 let text = logical_text(&app, logical);
1757 let chunks = column_chunks(&text, width);
1758 assert!(
1759 chunks.len() > 3,
1760 "the fixture must be long enough to wrap several times, got {} rows",
1761 chunks.len()
1762 );
1763 for chunk in &chunks {
1764 assert_buffer_contains(&buf, chunk);
1765 }
1766 // Both ends specifically, since those are what clipping loses.
1767 assert_buffer_contains(&buf, "HEAD0");
1768 assert_buffer_contains(&buf, "TAIL0");
1769 }
1770
1771 /// ...and the display rows really are a partition of the logical line: put
1772 /// back together they are it exactly, with nothing dropped at a break and
1773 /// nothing duplicated, and none of them overruns the pane.
1774 #[test]
1775 fn the_display_rows_of_a_line_reassemble_into_it() {
1776 let mut app = app_with_long_diff_lines(1);
1777 app.patch_width = 37;
1778 app.rebuild_patch_rows();
1779
1780 let logical = logical_index_of_diff_line(&app, 305);
1781 let rows = display_rows_of(&app, logical);
1782 assert!(rows.len() > 3, "expected several rows, got {}", rows.len());
1783 let rejoined: String = rows.iter().map(|r| r.line.to_string()).collect();
1784 assert_eq!(rejoined, logical_text(&app, logical));
1785 for r in &rows {
1786 assert!(
1787 r.line.width() <= 37,
1788 "a display row overran the pane: {:?}",
1789 r.line.to_string()
1790 );
1791 }
1792 }
1793
1794 /// The rule that makes wrapping safe: every display row of a wrapped line
1795 /// carries that line's target, so `c` anchors to the same `file:line`
1796 /// whichever of them the reader is looking at.
1797 #[test]
1798 fn every_display_row_of_a_wrapped_diff_line_carries_the_same_anchor() {
1799 let mut app = app_with_long_diff_lines(1);
1800 app.patch_width = 37;
1801 app.rebuild_patch_rows();
1802
1803 let logical = logical_index_of_diff_line(&app, 305);
1804 let rows = display_rows_of(&app, logical);
1805 assert!(rows.len() > 3);
1806 let expected = RowTarget::Diff {
1807 file: "src/main.rs".to_string(),
1808 line: 305,
1809 };
1810 // Read the way the program reads it: a screen row names a logical
1811 // line, and the anchor belongs to that line. A wrong `logical` on any
1812 // row would show up here as a different anchor — or as a row of this
1813 // line missing from the run above.
1814 for r in &rows {
1815 assert_eq!(app.patch_lines[r.logical].target, expected);
1816 }
1817
1818 // And what `c` actually reads agrees with them.
1819 app.patch_cursor = logical;
1820 assert_eq!(app.cursor_target(), expected);
1821 }
1822
1823 /// The cursor covers the whole run, the way vim highlights a wrapped line
1824 /// — otherwise the reader sees a highlight on one third of a line and has
1825 /// to guess what `c` is about to act on.
1826 #[test]
1827 fn the_cursor_highlights_every_row_of_the_line_it_is_on() {
1828 let mut app = app_with_long_diff_lines(1);
1829 app.rebuild_patch_rows();
1830 // Draw once so the pane records its width, then put the cursor on the
1831 // wrapped line and draw again.
1832 render_app_sized(&mut app, 80, 60);
1833 app.patch_cursor = logical_index_of_diff_line(&app, 305);
1834 app.patch_scroll = 0;
1835 let expected = display_rows_of(&app, app.patch_cursor).len();
1836 assert!(
1837 expected > 1,
1838 "the line under test has to wrap for this to mean anything"
1839 );
1840 let buf = render_app_sized(&mut app, 80, 60);
1841
1842 let highlighted = (1..buf.area.height - 1)
1843 .filter(|y| {
1844 (0..buf.area.width)
1845 .any(|x| buf.cell((x, *y)).unwrap().bg == Color::DarkGray)
1846 })
1847 .count();
1848 assert_eq!(
1849 highlighted, expected,
1850 "expected the highlight to cover all {} rows of the wrapped line",
1851 expected
1852 );
1853 }
1854
1855 /// `j` moves by logical line, not by display row — vim's own default,
1856 /// which is why `gj` exists. Without it, crossing one wrapped line takes
1857 /// several presses and the cursor appears to stall.
1858 #[test]
1859 fn j_crosses_a_wrapped_line_in_one_press() {
1860 let mut app = app_with_long_diff_lines(3);
1861 app.patch_width = 37;
1862 app.rebuild_patch_rows();
1863
1864 app.patch_cursor = logical_index_of_diff_line(&app, 305);
1865 assert!(
1866 display_rows_of(&app, app.patch_cursor).len() > 3,
1867 "the line under test has to wrap for this to mean anything"
1868 );
1869
1870 app.handle_key(
1871 crossterm::event::KeyCode::Char('j'),
1872 crossterm::event::KeyModifiers::empty(),
1873 );
1874 assert_eq!(
1875 app.cursor_target(),
1876 RowTarget::Diff {
1877 file: "src/main.rs".to_string(),
1878 line: 306
1879 }
1880 );
1881
1882 app.handle_key(
1883 crossterm::event::KeyCode::Char('k'),
1884 crossterm::event::KeyModifiers::empty(),
1885 );
1886 assert_eq!(
1887 app.cursor_target(),
1888 RowTarget::Diff {
1889 file: "src/main.rs".to_string(),
1890 line: 305
1891 }
1892 );
1893 }
1894
1895 /// A logical line can wrap taller than the pane. Cursor movement is per
1896 /// logical line and scrolling is per display row precisely so that this
1897 /// case stays readable: landing on such a line shows its *start*, rather
1898 /// than jumping the view past rows the reader never saw.
1899 #[test]
1900 fn landing_on_an_over_tall_line_shows_its_start() {
1901 let mut app = app_with_long_diff_lines(2);
1902 app.patch_width = 37;
1903 app.rebuild_patch_rows();
1904 app.patch_viewport = 4;
1905
1906 let target = logical_index_of_diff_line(&app, 306);
1907 let (first, last) = app.line_span(target);
1908 assert!(
1909 last - first > 4,
1910 "the line has to be taller than the pane: {} rows",
1911 last - first
1912 );
1913
1914 app.patch_cursor = target - 1;
1915 app.patch_scroll = 0;
1916 app.handle_key(
1917 crossterm::event::KeyCode::Char('j'),
1918 crossterm::event::KeyModifiers::empty(),
1919 );
1920 assert_eq!(app.patch_cursor, target);
1921 assert_eq!(
1922 app.patch_scroll as usize, first,
1923 "the view skipped the start of the line it moved onto"
1924 );
1925 }
1926
1927 /// ...and the rest of such a line is reachable, because rows scroll
1928 /// independently of the cursor. That is what `Ctrl-E`/`Ctrl-Y` are for in
1929 /// vim, and without them the middle of an over-tall line cannot be seen.
1930 #[test]
1931 fn ctrl_e_scrolls_rows_without_moving_the_cursor() {
1932 let mut app = app_with_long_diff_lines(2);
1933 app.patch_width = 37;
1934 app.rebuild_patch_rows();
1935 app.patch_viewport = 4;
1936
1937 let target = logical_index_of_diff_line(&app, 306);
1938 app.patch_cursor = target;
1939 let (first, last) = app.line_span(target);
1940 app.patch_scroll = first as u16;
1941
1942 for _ in 0..3 {
1943 app.handle_key(
1944 crossterm::event::KeyCode::Char('e'),
1945 crossterm::event::KeyModifiers::CONTROL,
1946 );
1947 }
1948 assert_eq!(app.patch_cursor, target, "Ctrl-E moved the cursor");
1949 assert_eq!(app.patch_scroll as usize, first + 3);
1950 assert!(
1951 (app.patch_scroll as usize) < last,
1952 "scrolled past the line it was reading"
1953 );
1954
1955 app.handle_key(
1956 crossterm::event::KeyCode::Char('y'),
1957 crossterm::event::KeyModifiers::CONTROL,
1958 );
1959 assert_eq!(app.patch_cursor, target);
1960 assert_eq!(app.patch_scroll as usize, first + 2);
1961 }
1962
1963 /// `w` is the escape hatch for the alignment case: `+`/`-` line up again
1964 /// because a diff line takes one row. Prose is not what that argument is
1965 /// about, so prose keeps wrapping.
1966 #[test]
1967 fn w_turns_off_wrapping_for_the_diff_but_not_for_prose() {
1968 let mut app = app_with_long_diff_lines(1);
1969 app.current_patch.as_mut().unwrap().comments[0].body = "p".repeat(200);
1970 app.patch_width = 37;
1971 app.rebuild_patch_rows();
1972
1973 let diff_line = logical_index_of_diff_line(&app, 305);
1974 let prose_line = app
1975 .patch_lines
1976 .iter()
1977 .position(|r| r.line.to_string().contains(&"p".repeat(50)))
1978 .expect("the long comment body");
1979 assert!(display_rows_of(&app, diff_line).len() > 3);
1980 assert!(display_rows_of(&app, prose_line).len() > 3);
1981
1982 app.handle_key(
1983 crossterm::event::KeyCode::Char('w'),
1984 crossterm::event::KeyModifiers::empty(),
1985 );
1986
1987 assert_eq!(
1988 display_rows_of(&app, diff_line).len(),
1989 1,
1990 "the diff line still wrapped after `w`"
1991 );
1992 assert!(
1993 display_rows_of(&app, prose_line).len() > 3,
1994 "`w` stopped prose wrapping too"
1995 );
1996 }
1997
1998 /// The silent clipping is a defect in its own right and outlives whichever
1999 /// default wins: with wrapping off, a line that does not fit has to say so.
2000 #[test]
2001 fn a_clipped_line_is_marked_as_clipped() {
2002 let mut app = app_with_long_diff_lines(1);
2003 app.patch_width = 37;
2004 app.patch_wrap = false;
2005 app.rebuild_patch_rows();
2006
2007 let diff_line = logical_index_of_diff_line(&app, 305);
2008 let rows = display_rows_of(&app, diff_line);
2009 assert_eq!(rows.len(), 1);
2010 let text = rows[0].line.to_string();
2011 assert!(
2012 text.ends_with(TRUNCATION_MARK),
2013 "a clipped line said nothing about being clipped: {:?}",
2014 text
2015 );
2016 assert!(text.chars().count() <= 37);
2017
2018 // A line that fits is left alone — the mark means something.
2019 let short = app
2020 .patch_lines
2021 .iter()
2022 .position(|r| r.line.to_string() == "Base: main")
2023 .expect("the Base: header row");
2024 assert!(!display_rows_of(&app, short)[0]
2025 .line
2026 .to_string()
2027 .ends_with(TRUNCATION_MARK));
2028 }
2029
2030 /// The choice is remembered for the session: rebuilding the rows for a new
2031 /// revision, a new diff mode or a fresh comment must not quietly put it
2032 /// back.
2033 #[test]
2034 fn the_wrap_choice_survives_a_rebuild() {
2035 let mut app = app_with_long_diff_lines(1);
2036 app.patch_width = 37;
2037 app.rebuild_patch_rows();
2038
2039 app.handle_key(
2040 crossterm::event::KeyCode::Char('w'),
2041 crossterm::event::KeyModifiers::empty(),
2042 );
2043 assert!(!app.patch_wrap);
2044
2045 app.handle_key(
2046 crossterm::event::KeyCode::Char('d'),
2047 crossterm::event::KeyModifiers::empty(),
2048 );
2049 app.rebuild_patch_rows();
2050 assert!(!app.patch_wrap, "the wrap choice was reset by a rebuild");
2051 assert_eq!(
2052 display_rows_of(&app, logical_index_of_diff_line(&app, 305)).len(),
2053 1
2054 );
2055 }
2056
2057 /// git hands the whole `diff --git ... / index ... / --- a/f` file header
2058 /// over as *one* diff row with newlines inside it. A `Line` cannot hold a
2059 /// newline — ratatui has no row to put the rest on — so those three lines
2060 /// used to arrive mashed into one, and a line whose width counts a `\n` as
2061 /// nothing is a line this pane cannot lay out. One physical line per row.
2062 #[test]
2063 fn a_multi_line_diff_header_becomes_one_row_per_line() {
2064 let mut app = make_app(0, 0);
2065 app.mode = ViewMode::PatchDetail;
2066 app.current_patch = Some(make_patch_with_revisions());
2067 app.patch_diff = vec![crate::patch::DiffRow {
2068 text: "diff --git a/f.rs b/f.rs\nindex fbbee86..789bcdf 100644\n--- a/f.rs\n".into(),
2069 body: false,
2070 anchor: None,
2071 }];
2072 app.patch_width = 200;
2073 app.rebuild_patch_rows();
2074
2075 let start = app
2076 .patch_lines
2077 .iter()
2078 .position(|r| r.line.to_string().starts_with("diff --git"))
2079 .expect("the file header");
2080 let texts: Vec<String> = app.patch_lines[start..start + 3]
2081 .iter()
2082 .map(|r| r.line.to_string())
2083 .collect();
2084 assert_eq!(
2085 texts,
2086 vec![
2087 "diff --git a/f.rs b/f.rs".to_string(),
2088 "index fbbee86..789bcdf 100644".to_string(),
2089 "--- a/f.rs".to_string(),
2090 ]
2091 );
2092 assert!(
2093 app.patch_lines.iter().all(|r| !r.line.to_string().contains('\n')),
2094 "a rendered line still holds a newline"
2095 );
2096 }
2097
2098 /// A body line that carries an anchor keeps it however it is split, so the
2099 /// splitting above cannot cost a comment its target.
2100 #[test]
2101 fn splitting_a_diff_row_keeps_its_anchor_on_every_piece() {
2102 let mut app = make_app(0, 0);
2103 app.mode = ViewMode::PatchDetail;
2104 app.current_patch = Some(make_patch_with_revisions());
2105 app.patch_diff = vec![crate::patch::DiffRow {
2106 text: "+first\n+second\n".into(),
2107 body: true,
2108 anchor: Some(("src/main.rs".to_string(), 12)),
2109 }];
2110 app.patch_width = 200;
2111 app.rebuild_patch_rows();
2112
2113 let anchored: Vec<&RowTarget> = app
2114 .patch_lines
2115 .iter()
2116 .map(|r| &r.target)
2117 .filter(|t| matches!(t, RowTarget::Diff { .. }))
2118 .collect();
2119 let expected = RowTarget::Diff {
2120 file: "src/main.rs".to_string(),
2121 line: 12,
2122 };
2123 assert_eq!(anchored, vec![&expected, &expected]);
2124 }
2125
2126 /// The pane is re-wrapped when it is resized, because the rows were built
2127 /// for a width that no longer exists.
2128 #[test]
2129 fn a_resize_re_wraps_the_pane() {
2130 let mut app = app_with_long_diff_lines(1);
2131 app.rebuild_patch_rows();
2132
2133 render_app_sized(&mut app, 80, 60);
2134 let narrow = app.patch_rows.len();
2135 render_app_sized(&mut app, 200, 60);
2136 let wide = app.patch_rows.len();
2137
2138 assert!(
2139 wide < narrow,
2140 "a wider pane needed as many rows ({} vs {})",
2141 wide,
2142 narrow
2143 );
2144 }
2145
2146 /// A pane too narrow to hold anything must not loop forever or lose the
2147 /// line; the same for a line that is empty.
2148 #[test]
2149 fn a_degenerate_width_still_terminates() {
2150 let mut app = app_with_long_diff_lines(1);
2151 app.patch_width = 1;
2152 app.rebuild_patch_rows();
2153 assert!(!app.patch_rows.is_empty());
2154
2155 app.patch_width = 0;
2156 app.rebuild_patch_rows();
2157 assert_eq!(app.patch_rows.len(), app.patch_lines.len());
2158
2159 // Blank lines are rows too — dropping them would close the gaps the
2160 // pane uses to separate its sections.
2161 app.patch_width = 20;
2162 app.rebuild_patch_rows();
2163 let blanks = app
2164 .patch_lines
2165 .iter()
2166 .filter(|r| r.line.to_string().is_empty())
2167 .count();
2168 assert!(blanks > 0);
2169 assert_eq!(
2170 app.patch_rows
2171 .iter()
2172 .filter(|r| r.line.to_string().is_empty())
2173 .count(),
2174 blanks
2175 );
2176 }
2177
2178 // ── Timestamps (952390ad) ────────────────────────────────────────────
2179 //
2180 // The stored form is `2026-01-01T00:00:00Z` — up to 35 characters, nine of
2181 // them nanoseconds. `b2a57996` settled on `YYYY-MM-DD HH:MM` for the web,
2182 // and the two surfaces have to agree: a reader moves between them and a
2183 // screenshot from one is pasted next to output from the other. The TUI has
2184 // no hover, so the short form is all there is; nothing is lost, because
2185 // `--json` and `show` still carry the stored value in full.
2186
2187 fn assert_buffer_lacks(buf: &Buffer, unwanted: &str) {
2188 let text = buffer_to_string(buf);
2189 assert!(
2190 !text.contains(unwanted),
2191 "expected buffer NOT to contain {:?}, but it is in:\n{}",
2192 unwanted,
2193 text
2194 );
2195 }
2196
2197 #[test]
2198 fn an_issues_created_date_is_shown_to_the_minute() {
2199 let mut app = make_app(3, 0);
2200 app.pane = Pane::Detail;
2201 app.list_state.select(Some(0));
2202
2203 let buf = render_app_sized(&mut app, 120, 24);
2204 assert_buffer_contains(&buf, "Created: 2026-01-01 00:00");
2205 assert_buffer_lacks(&buf, "2026-01-01T00:00:00Z");
2206 }
2207
2208 #[test]
2209 fn an_issue_comments_date_is_shown_to_the_minute() {
2210 let mut app = make_app(1, 0);
2211 app.issues[0].comments = vec![crate::state::Comment {
2212 author: test_author(),
2213 body: "a remark".into(),
2214 timestamp: "2026-02-03T04:05:06.789012345+00:00".into(),
2215 commit_id: Oid::from_str("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap(),
2216 edited: false,
2217 deleted: false,
2218 }];
2219 app.pane = Pane::Detail;
2220 app.list_state.select(Some(0));
2221
2222 let buf = render_app_sized(&mut app, 120, 24);
2223 assert_buffer_contains(&buf, "(2026-02-03 04:05)");
2224 assert_buffer_lacks(&buf, "789012345");
2225 }
2226
2227 #[test]
2228 fn a_linked_commits_date_is_shown_to_the_minute() {
2229 let mut app = make_app(1, 0);
2230 app.issues[0].linked_commits = vec![crate::state::LinkedCommit {
2231 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".into(),
2232 event_author: test_author(),
2233 event_timestamp: "2026-02-03T04:05:06.789012345+00:00".into(),
2234 }];
2235 app.pane = Pane::Detail;
2236 app.list_state.select(Some(0));
2237
2238 let buf = render_app_sized(&mut app, 200, 24);
2239 assert_buffer_contains(&buf, "2026-02-03 04:05");
2240 assert_buffer_lacks(&buf, "789012345");
2241 }
2242
2243 /// The revision list, the reviews and the thread comments all carry a
2244 /// stored timestamp, and all three are on one pane — so all three have to
2245 /// be shortened or the pane shows one field two ways.
2246 #[test]
2247 fn every_date_on_the_patch_detail_pane_is_shown_to_the_minute() {
2248 let mut app = make_app(0, 0);
2249 app.mode = ViewMode::PatchDetail;
2250 app.current_patch = Some(make_patch_with_revisions());
2251 app.patch_diff.clear();
2252 app.rebuild_patch_rows();
2253
2254 let buf = render_app_sized(&mut app, 200, 60);
2255 assert_buffer_contains(&buf, "2026-01-01 00:00"); // revision r1
2256 assert_buffer_contains(&buf, "2026-01-02 00:00"); // revision r2
2257 assert_buffer_contains(&buf, "2026-01-04 00:00"); // the review
2258 assert_buffer_contains(&buf, "(2026-01-05 00:00)"); // the thread comment
2259 assert_buffer_lacks(&buf, "T00:00:00Z");
2260 }
2261
2262 #[test]
2263 fn the_event_list_and_event_detail_show_dates_to_the_minute() {
2264 let mut app = make_app(3, 0);
2265 app.event_history = make_test_event_history();
2266 app.event_list_state.select(Some(0));
2267
2268 app.mode = ViewMode::CommitList;
2269 let buf = render_app_sized(&mut app, 120, 24);
2270 assert_buffer_contains(&buf, "2026-01-01 00:00");
2271 assert_buffer_lacks(&buf, "2026-01-01T00:00:00Z");
2272
2273 app.mode = ViewMode::CommitDetail;
2274 let buf = render_app_sized(&mut app, 120, 24);
2275 assert_buffer_contains(&buf, "2026-01-01 00:00");
2276 assert_buffer_lacks(&buf, "2026-01-01T00:00:00Z");
2277 }
2278
2279 /// A value this project did not write is still a fact about the object.
2280 /// Printing it as it stands beats printing a placeholder that hides the one
2281 /// thing which could explain the row — the same rule the web took.
2282 #[test]
2283 fn an_unparseable_stored_date_is_shown_as_it_stands() {
2284 let mut app = make_app(1, 0);
2285 app.issues[0].created_at = "whenever".into();
2286 app.pane = Pane::Detail;
2287 app.list_state.select(Some(0));
2288
2289 let buf = render_app_sized(&mut app, 120, 24);
2290 assert_buffer_contains(&buf, "Created: whenever");
2291 }
2292
1674 #[test] 2293 #[test]
1675 fn test_linked_patch_for_selected() { 2294 fn test_linked_patch_for_selected() {
1676 let mut app = test_app(); 2295 let mut app = test_app();
src/tui/state.rs
Old New
@@ -7,7 +7,7 @@ use crate::abbrev::Abbrev;
7 use crate::state::{self, IssueState, IssueStatus, PatchState, PatchStatus}; 7 use crate::state::{self, IssueState, IssueStatus, PatchState, PatchStatus};
8 8
9 use super::tips::Tips; 9 use super::tips::Tips;
10 use super::widgets::{DetailRow, RowTarget}; 10 use super::widgets::{DetailRow, DisplayRow, RowTarget};
11 11
12 #[derive(Debug, PartialEq)] 12 #[derive(Debug, PartialEq)]
13 pub(crate) enum Pane { 13 pub(crate) enum Pane {
@@ -130,15 +130,35 @@ pub(crate) struct App {
130 /// comment on it would use. Rows rather than a string because the cursor 130 /// comment on it would use. Rows rather than a string because the cursor
131 /// has to be able to say which line of which file it is sitting on. 131 /// has to be able to say which line of which file it is sitting on.
132 pub(crate) patch_diff: Vec<crate::patch::DiffRow>, 132 pub(crate) patch_diff: Vec<crate::patch::DiffRow>,
133 /// Every line the patch detail pane renders, with what a write key would 133 /// Every logical line the patch detail pane renders, with what a write key
134 /// act on. Built when the view is built rather than during a draw: the key 134 /// would act on. Built when the view is built rather than during a draw:
135 /// handler needs it, and the key handler never renders. 135 /// the key handler needs it, and the key handler never renders.
136 pub(crate) patch_rows: Vec<DetailRow>, 136 pub(crate) patch_lines: Vec<DetailRow>,
137 /// Which of `patch_rows` the cursor is on. 137 /// Those lines laid out for the pane's current width — one entry per screen
138 /// row, each pointing back at the line it came from.
139 pub(crate) patch_rows: Vec<DisplayRow>,
140 /// For each logical line, the half-open range of `patch_rows` it occupies.
141 /// Kept beside the rows so the scroll can ask where a line is without a
142 /// scan on every keypress.
143 patch_spans: Vec<(usize, usize)>,
144 /// Which of `patch_lines` the cursor is on.
145 ///
146 /// A logical line, not a screen row. `j` therefore crosses a wrapped line
147 /// in one press — vim's default, which is why `gj` exists — and `c` anchors
148 /// to the line the reader believes they are on however it happened to wrap.
138 pub(crate) patch_cursor: usize, 149 pub(crate) patch_cursor: usize,
139 /// Rows the detail pane last had room for, so the scroll can follow the 150 /// Rows the detail pane last had room for, so the scroll can follow the
140 /// cursor. Only the renderer knows this; it is recorded on the way past. 151 /// cursor. Only the renderer knows this; it is recorded on the way past.
141 pub(crate) patch_viewport: u16, 152 pub(crate) patch_viewport: u16,
153 /// Columns the detail pane last had room for, and so the width its rows
154 /// were laid out for. Recorded by the renderer, like the height above.
155 pub(crate) patch_width: u16,
156 /// Whether the diff wraps. On by default: this pane has no column — the
157 /// cursor is a single index and there is no horizontal movement anywhere in
158 /// the TUI — so the only cost of wrapping is `+`/`-` alignment, against
159 /// text disappearing off the right edge of a review tool. `w` is the escape
160 /// hatch for the alignment case, and the choice sticks for the session.
161 pub(crate) patch_wrap: bool,
142 /// When set, the diff on screen is the change that answered this comment 162 /// When set, the diff on screen is the change that answered this comment
143 /// rather than the revision's own diff. 163 /// rather than the revision's own diff.
144 pub(crate) patch_answers: Option<String>, 164 pub(crate) patch_answers: Option<String>,
@@ -192,9 +212,13 @@ impl App {
192 event_list_state: ListState::default(), 212 event_list_state: ListState::default(),
193 current_patch: None, 213 current_patch: None,
194 patch_diff: Vec::new(), 214 patch_diff: Vec::new(),
215 patch_lines: Vec::new(),
195 patch_rows: Vec::new(), 216 patch_rows: Vec::new(),
217 patch_spans: Vec::new(),
196 patch_cursor: 0, 218 patch_cursor: 0,
197 patch_viewport: 20, 219 patch_viewport: 20,
220 patch_width: 0,
221 patch_wrap: true,
198 patch_answers: None, 222 patch_answers: None,
199 patch_scroll: 0, 223 patch_scroll: 0,
200 patch_revision_idx: 0, 224 patch_revision_idx: 0,
@@ -228,23 +252,49 @@ impl App {
228 /// have to be the rows the reader was looking at when they pressed the 252 /// have to be the rows the reader was looking at when they pressed the
229 /// key, not the rows a subsequent redraw would have produced. 253 /// key, not the rows a subsequent redraw would have produced.
230 pub(crate) fn rebuild_patch_rows(&mut self) { 254 pub(crate) fn rebuild_patch_rows(&mut self) {
231 self.patch_rows = super::widgets::build_patch_detail_rows(self); 255 self.patch_lines = super::widgets::build_patch_detail_rows(self);
232 if self.patch_cursor >= self.patch_rows.len() { 256 if self.patch_cursor >= self.patch_lines.len() {
233 self.patch_cursor = self.patch_rows.len().saturating_sub(1); 257 self.patch_cursor = self.patch_lines.len().saturating_sub(1);
258 }
259 self.rewrap();
260 }
261
262 /// Lay the logical lines out for the pane's current width.
263 ///
264 /// Called from the renderer when the width changes and from
265 /// [`App::rebuild_patch_rows`] when the lines themselves do. It touches no
266 /// repository state and cannot move the cursor — only where the cursor's
267 /// line happens to sit on screen.
268 pub(crate) fn rewrap(&mut self) {
269 self.patch_rows =
270 super::widgets::wrap_detail_rows(&self.patch_lines, self.patch_width, self.patch_wrap);
271 self.patch_spans = vec![(0, 0); self.patch_lines.len()];
272 for (i, row) in self.patch_rows.iter().enumerate() {
273 let span = &mut self.patch_spans[row.logical];
274 if span.1 == 0 {
275 *span = (i, i + 1);
276 } else {
277 span.1 = i + 1;
278 }
234 } 279 }
235 self.follow_cursor(); 280 self.follow_cursor();
236 } 281 }
237 282
238 /// The row the cursor is on in the patch detail pane. 283 /// The half-open range of display rows a logical line occupies.
284 pub(crate) fn line_span(&self, logical: usize) -> (usize, usize) {
285 self.patch_spans.get(logical).copied().unwrap_or((0, 0))
286 }
287
288 /// The line the cursor is on in the patch detail pane.
239 pub(crate) fn cursor_target(&self) -> RowTarget { 289 pub(crate) fn cursor_target(&self) -> RowTarget {
240 self.patch_rows 290 self.patch_lines
241 .get(self.patch_cursor) 291 .get(self.patch_cursor)
242 .map(|r| r.target.clone()) 292 .map(|r| r.target.clone())
243 .unwrap_or_default() 293 .unwrap_or_default()
244 } 294 }
245 295
246 fn move_cursor(&mut self, delta: i32) { 296 fn move_cursor(&mut self, delta: i32) {
247 let len = self.patch_rows.len(); 297 let len = self.patch_lines.len();
248 if len == 0 { 298 if len == 0 {
249 self.patch_cursor = 0; 299 self.patch_cursor = 0;
250 return; 300 return;
@@ -257,16 +307,57 @@ impl App {
257 self.follow_cursor(); 307 self.follow_cursor();
258 } 308 }
259 309
260 /// Keep the cursor on screen without moving the view any further than it 310 /// Scroll display rows without touching the cursor.
261 /// has to: a reader walking a diff should not have the text jump. 311 ///
312 /// The counterpart to moving by logical line, and the reason a line taller
313 /// than the pane is still readable: `j` would jump the view straight past
314 /// its middle. Vim keeps the two separate for the same reason, and calls
315 /// them `Ctrl-E` and `Ctrl-Y`.
316 fn scroll_rows(&mut self, delta: i32) {
317 let max = self.patch_rows.len().saturating_sub(1);
318 let top = self.patch_scroll as usize;
319 let new = if delta > 0 {
320 (top + delta as usize).min(max)
321 } else {
322 top.saturating_sub((-delta) as usize)
323 };
324 self.patch_scroll = new as u16;
325 }
326
327 /// Bring the cursor's line into view, without moving further than it has to
328 /// and without assuming the line fits.
329 ///
330 /// A logical line can wrap to more rows than the pane is tall. Scrolling to
331 /// put such a line "on screen" cannot mean showing all of it, so it means
332 /// showing its start and leaving the rest to `Ctrl-E`; and once the reader
333 /// is part way through one, nothing here may drag them back to the top.
262 fn follow_cursor(&mut self) { 334 fn follow_cursor(&mut self) {
263 let height = self.patch_viewport.max(1) as usize; 335 let height = self.patch_viewport.max(1) as usize;
336 let (first, end) = self.line_span(self.patch_cursor);
337 if end == 0 {
338 self.patch_scroll = 0;
339 return;
340 }
341 let last = end - 1;
264 let top = self.patch_scroll as usize; 342 let top = self.patch_scroll as usize;
265 if self.patch_cursor < top { 343
266 self.patch_scroll = self.patch_cursor as u16; 344 // Already wholly in view.
267 } else if self.patch_cursor >= top + height { 345 if first >= top && last < top + height {
268 self.patch_scroll = (self.patch_cursor + 1 - height) as u16; 346 return;
347 }
348 // Taller than the pane, and some of it is on screen: the reader is
349 // walking through it a row at a time, so leave the view where they put
350 // it rather than snapping back to the line's first row.
351 if end - first >= height && first < top + height && last >= top {
352 return;
269 } 353 }
354 // Otherwise scroll the least that helps. `.min(first)` is what keeps an
355 // over-tall line's start visible instead of its end.
356 self.patch_scroll = if first < top {
357 first as u16
358 } else {
359 ((last + 1 - height).min(first)) as u16
360 };
270 } 361 }
271 362
272 /// Reset what a change of revision or diff mode invalidates. 363 /// Reset what a change of revision or diff mode invalidates.
@@ -357,6 +448,7 @@ impl App {
357 self.pane = Pane::ItemList; 448 self.pane = Pane::ItemList;
358 self.current_patch = None; 449 self.current_patch = None;
359 self.patch_diff.clear(); 450 self.patch_diff.clear();
451 self.patch_lines.clear();
360 self.patch_rows.clear(); 452 self.patch_rows.clear();
361 self.patch_revision_idx = 0; 453 self.patch_revision_idx = 0;
362 self.patch_interdiff_mode = false; 454 self.patch_interdiff_mode = false;
@@ -367,6 +459,16 @@ impl App {
367 KeyCode::Char('c') if modifiers.contains(KeyModifiers::CONTROL) => { 459 KeyCode::Char('c') if modifiers.contains(KeyModifiers::CONTROL) => {
368 return KeyAction::Quit; 460 return KeyAction::Quit;
369 } 461 }
462 // Rows, not lines, and the cursor stays where it is — the only
463 // way through a line that wrapped taller than the pane.
464 KeyCode::Char('e') if modifiers.contains(KeyModifiers::CONTROL) => {
465 self.scroll_rows(1);
466 return KeyAction::Continue;
467 }
468 KeyCode::Char('y') if modifiers.contains(KeyModifiers::CONTROL) => {
469 self.scroll_rows(-1);
470 return KeyAction::Continue;
471 }
370 KeyCode::Char('j') | KeyCode::Down => { 472 KeyCode::Char('j') | KeyCode::Down => {
371 self.move_cursor(1); 473 self.move_cursor(1);
372 return KeyAction::Continue; 474 return KeyAction::Continue;
@@ -416,6 +518,14 @@ impl App {
416 self.reset_patch_view(); 518 self.reset_patch_view();
417 return KeyAction::Reload; // signal to regenerate diff 519 return KeyAction::Reload; // signal to regenerate diff
418 } 520 }
521 // The escape hatch for reading `+`/`-` in column. Deliberately
522 // not reset by `reset_patch_view`: a reader who turned wrapping
523 // off did not mean "until the next revision".
524 KeyCode::Char('w') => {
525 self.patch_wrap = !self.patch_wrap;
526 self.rewrap();
527 return KeyAction::Continue;
528 }
419 _ => return KeyAction::Continue, 529 _ => return KeyAction::Continue,
420 } 530 }
421 } 531 }
src/tui/widgets.rs
Old New
@@ -8,6 +8,16 @@ use crate::state::{IssueState, IssueStatus, PatchState, PatchStatus};
8 8
9 use super::state::{App, InputMode, ListMode, Pane, StatusFilter, ViewMode}; 9 use super::state::{App, InputMode, ListMode, Pane, StatusFilter, ViewMode};
10 10
11 /// A stored timestamp as the dashboard shows it: `YYYY-MM-DD HH:MM`, the same
12 /// rendering the web settled on in `b2a57996`.
13 ///
14 /// A terminal has no hover, so unlike the web there is nowhere to hang the
15 /// stored value and the short form is all that goes on screen. Nothing is
16 /// lost: `issue show`, `patch show` and `--json` still carry it in full.
17 fn short_time(stored: &str) -> String {
18 crate::timestamp::Timestamp::new(stored).short
19 }
20
11 /// The lines to render for a comment body: the text as it stands, or the 21 /// The lines to render for a comment body: the text as it stands, or the
12 /// tombstone when it was deleted. 22 /// tombstone when it was deleted.
13 /// 23 ///
@@ -86,7 +96,11 @@ pub(crate) fn format_event_detail(
86 96
87 let mut detail = format!( 97 let mut detail = format!(
88 "Commit: {}\nAuthor: {} <{}>\nDate: {}\nType: {}\n", 98 "Commit: {}\nAuthor: {} <{}>\nDate: {}\nType: {}\n",
89 short_oid, event.author.name, event.author.email, event.timestamp, action_label, 99 short_oid,
100 event.author.name,
101 event.author.email,
102 short_time(&event.timestamp),
103 action_label,
90 ); 104 );
91 105
92 // Action-specific payload 106 // Action-specific payload
@@ -218,14 +232,29 @@ pub(crate) enum RowTarget {
218 Comment { oid: String, resolved: bool }, 232 Comment { oid: String, resolved: bool },
219 } 233 }
220 234
221 /// A rendered line of the patch detail pane, and what it stands for. 235 /// Which of the pane's two kinds of content a line is.
236 ///
237 /// The distinction exists for one key. Wrapping is on for everything by
238 /// default, but the case against it — that `+`/`-` stop lining up — is an
239 /// argument about a diff and not about an issue body or a review, so `w` turns
240 /// it off for [`RowKind::Diff`] alone.
241 #[derive(Debug, Clone, Copy, PartialEq)]
242 pub(crate) enum RowKind {
243 Prose,
244 Diff,
245 }
246
247 /// A logical line of the patch detail pane, and what it stands for.
222 /// 248 ///
223 /// Built once when the view is built rather than during a draw: the key 249 /// Built once when the view is built rather than during a draw: the key
224 /// handler has to know what the cursor is on, and the key handler never draws. 250 /// handler has to know what the cursor is on, and the key handler never draws.
251 /// The cursor indexes these, not the display rows below — so what `c` acts on
252 /// does not depend on how wide the pane happened to be.
225 #[derive(Debug, Clone)] 253 #[derive(Debug, Clone)]
226 pub(crate) struct DetailRow { 254 pub(crate) struct DetailRow {
227 pub(crate) line: Line<'static>, 255 pub(crate) line: Line<'static>,
228 pub(crate) target: RowTarget, 256 pub(crate) target: RowTarget,
257 pub(crate) kind: RowKind,
229 } 258 }
230 259
231 impl DetailRow { 260 impl DetailRow {
@@ -233,15 +262,38 @@ impl DetailRow {
233 DetailRow { 262 DetailRow {
234 line, 263 line,
235 target: RowTarget::None, 264 target: RowTarget::None,
265 kind: RowKind::Prose,
236 } 266 }
237 } 267 }
238 } 268 }
239 269
270 /// One screen row of the patch detail pane: a slice of a logical line, wide
271 /// enough to fit and no wider.
272 ///
273 /// `logical` is the whole trick. A diff line that wraps to three rows becomes
274 /// three `DisplayRow`s all pointing at the one line that says
275 /// `Diff { line: 305 }` — so the highlight covers the run, and `c` means 305
276 /// from any part of it. The target is deliberately *not* copied down here:
277 /// one owner means the rows cannot disagree with the line they came from.
278 #[derive(Debug, Clone)]
279 pub(crate) struct DisplayRow {
280 pub(crate) line: Line<'static>,
281 /// Index into the logical lines this was wrapped from.
282 pub(crate) logical: usize,
283 }
284
285 /// What a line that had to be cut short ends with.
286 ///
287 /// Wrapping is the default, so this is only reached with `w` pressed — but a
288 /// line that simply stops, with nothing to say more was there, is the defect
289 /// that started this and it must not survive in the one mode that still clips.
290 pub(crate) const TRUNCATION_MARK: &str = "›";
291
240 /// Accumulator for the detail pane. 292 /// Accumulator for the detail pane.
241 /// 293 ///
242 /// Most of the pane is prose with nothing to act on, so `push` takes a bare 294 /// Most of the pane is prose with nothing to act on, so `push` takes a bare
243 /// line; the two places the cursor can do something — an inline comment and a 295 /// line; the two places the cursor can do something — an inline comment and a
244 /// new-side line of the diff — say so with `push_target`. 296 /// new-side line of the diff — say so with `push_target` and `push_diff`.
245 struct Rows(Vec<DetailRow>); 297 struct Rows(Vec<DetailRow>);
246 298
247 impl Rows { 299 impl Rows {
@@ -254,10 +306,131 @@ impl Rows {
254 } 306 }
255 307
256 fn push_target(&mut self, line: Line<'static>, target: RowTarget) { 308 fn push_target(&mut self, line: Line<'static>, target: RowTarget) {
257 self.0.push(DetailRow { line, target }); 309 self.0.push(DetailRow {
310 line,
311 target,
312 kind: RowKind::Prose,
313 });
314 }
315
316 fn push_diff(&mut self, line: Line<'static>, target: RowTarget) {
317 self.0.push(DetailRow {
318 line,
319 target,
320 kind: RowKind::Diff,
321 });
258 } 322 }
259 } 323 }
260 324
325 /// Lay the pane's logical lines out for a pane `width` columns wide.
326 ///
327 /// This is the mapping `Paragraph::wrap` does not expose: every row knows which
328 /// logical line it came from, so the cursor can stay a logical index while the
329 /// scroll stays a row offset. A width of zero means the pane has not been drawn
330 /// yet and nothing is known about how much room there is, so the lines pass
331 /// through one for one.
332 pub(crate) fn wrap_detail_rows(lines: &[DetailRow], width: u16, wrap_diff: bool) -> Vec<DisplayRow> {
333 let width = width as usize;
334 let mut out = Vec::with_capacity(lines.len());
335 for (logical, row) in lines.iter().enumerate() {
336 let wrapped = if width == 0 {
337 vec![row.line.clone()]
338 } else if row.kind == RowKind::Diff && !wrap_diff {
339 vec![truncate_line(&row.line, width)]
340 } else {
341 wrap_line(&row.line, width)
342 };
343 for line in wrapped {
344 out.push(DisplayRow { line, logical });
345 }
346 }
347 out
348 }
349
350 /// Break one line into rows no wider than `width`, keeping every character and
351 /// the style it was written in.
352 ///
353 /// The break is at the column, not at a word: this pane shows code as often as
354 /// prose, and a reader following an anchor to `file:line` needs the row to
355 /// correspond to the columns of the line. An empty line still yields one row —
356 /// the pane uses blanks to separate its sections.
357 fn wrap_line(line: &Line<'static>, width: usize) -> Vec<Line<'static>> {
358 use unicode_width::UnicodeWidthChar;
359
360 let mut rows: Vec<Vec<Span<'static>>> = vec![Vec::new()];
361 let mut used = 0usize;
362 for span in &line.spans {
363 let style = span.style;
364 let mut buf = String::new();
365 for ch in span.content.chars() {
366 let cw = ch.width().unwrap_or(0);
367 // `used > 0` keeps a character wider than the whole pane on a row
368 // of its own rather than looping forever trying to fit it.
369 if used + cw > width && used > 0 {
370 if !buf.is_empty() {
371 rows.last_mut()
372 .unwrap()
373 .push(Span::styled(std::mem::take(&mut buf), style));
374 }
375 rows.push(Vec::new());
376 used = 0;
377 }
378 buf.push(ch);
379 used += cw;
380 }
381 if !buf.is_empty() {
382 rows.last_mut().unwrap().push(Span::styled(buf, style));
383 }
384 }
385 rows.into_iter()
386 .map(|spans| Line::from(spans).style(line.style))
387 .collect()
388 }
389
390 /// Cut a line down to `width`, ending it with [`TRUNCATION_MARK`] so that what
391 /// was dropped is visible as having been dropped.
392 fn truncate_line(line: &Line<'static>, width: usize) -> Line<'static> {
393 use unicode_width::UnicodeWidthChar;
394
395 if line.width() <= width {
396 return line.clone();
397 }
398 // No room for both a character and the mark; say only that there is more.
399 if width == 1 {
400 return Line::from(vec![Span::styled(
401 TRUNCATION_MARK,
402 Style::default().fg(Color::DarkGray),
403 )])
404 .style(line.style);
405 }
406
407 let keep = width - 1;
408 let mut used = 0usize;
409 let mut spans: Vec<Span<'static>> = Vec::new();
410 'outer: for span in &line.spans {
411 let mut buf = String::new();
412 for ch in span.content.chars() {
413 let cw = ch.width().unwrap_or(0);
414 if used + cw > keep {
415 if !buf.is_empty() {
416 spans.push(Span::styled(buf, span.style));
417 }
418 break 'outer;
419 }
420 buf.push(ch);
421 used += cw;
422 }
423 if !buf.is_empty() {
424 spans.push(Span::styled(buf, span.style));
425 }
426 }
427 spans.push(Span::styled(
428 TRUNCATION_MARK,
429 Style::default().fg(Color::DarkGray),
430 ));
431 Line::from(spans).style(line.style)
432 }
433
261 pub(crate) fn ui(frame: &mut Frame, app: &mut App, repo: Option<&Repository>) { 434 pub(crate) fn ui(frame: &mut Frame, app: &mut App, repo: Option<&Repository>) {
262 let chunks = Layout::default() 435 let chunks = Layout::default()
263 .direction(Direction::Vertical) 436 .direction(Direction::Vertical)
@@ -429,21 +602,32 @@ fn render_detail(frame: &mut Frame, app: &mut App, area: Rect, repo: Option<&Rep
429 602
430 // Handle patch detail mode 603 // Handle patch detail mode
431 if app.mode == ViewMode::PatchDetail { 604 if app.mode == ViewMode::PatchDetail {
432 // The rows are built by the key handler, not here, so that `c` and `x` 605 // The logical lines are built by the key handler, not here, so that `c`
433 // act on the same row the reader is looking at. A draw that rebuilt 606 // and `x` act on the same line the reader is looking at. A draw that
434 // them could disagree with the last keypress. 607 // rebuilt them could disagree with the last keypress.
608 //
609 // Laying those lines out into display rows *is* done here, because how
610 // many rows a line needs is a fact about this pane at this size and
611 // nothing else knows it. That is safe only because the cursor indexes
612 // the logical lines: re-wrapping cannot move what a key would act on.
435 // 613 //
436 // Recording the height is the one thing only the renderer knows, and 614 // Recording the height and width is the one thing only the renderer
437 // the scroll needs it to follow the cursor. It moves no ref and reads 615 // knows. It moves no ref and reads no object: a draw stays a draw.
438 // no object: a draw stays a draw.
439 app.patch_viewport = area.height.saturating_sub(2); 616 app.patch_viewport = area.height.saturating_sub(2);
617 let width = area.width.saturating_sub(2);
618 if width != app.patch_width {
619 app.patch_width = width;
620 app.rewrap();
621 }
440 let cursor = app.patch_cursor; 622 let cursor = app.patch_cursor;
441 let lines: Vec<Line> = app 623 let lines: Vec<Line> = app
442 .patch_rows 624 .patch_rows
443 .iter() 625 .iter()
444 .enumerate() 626 .map(|row| {
445 .map(|(i, row)| { 627 // The highlight covers the whole run of rows the cursor's line
446 if i == cursor { 628 // wrapped to, which is what vim does and what keeps the reader
629 // from guessing which line `c` is about to act on.
630 if row.logical == cursor {
447 row.line.clone().style( 631 row.line.clone().style(
448 Style::default() 632 Style::default()
449 .bg(Color::DarkGray) 633 .bg(Color::DarkGray)
@@ -458,9 +642,10 @@ fn render_detail(frame: &mut Frame, app: &mut App, area: Rect, repo: Option<&Rep
458 .borders(Borders::ALL) 642 .borders(Borders::ALL)
459 .title("Patch Detail") 643 .title("Patch Detail")
460 .border_style(border_style); 644 .border_style(border_style);
461 // No wrapping: a wrapped line occupies more rows than it has, which 645 // No `Paragraph::wrap`: the rows above are already no wider than the
462 // would put the cursor's highlight on a different line than the one 646 // pane, and `Paragraph` would wrap them a second time with no way to
463 // `c` is about to comment on. 647 // tell which logical line the result came from — which is exactly what
648 // `07a78a60` had to turn off.
464 let para = Paragraph::new(Text::from(lines)) 649 let para = Paragraph::new(Text::from(lines))
465 .block(block) 650 .block(block)
466 .scroll((app.patch_scroll, 0)); 651 .scroll((app.patch_scroll, 0));
@@ -477,7 +662,9 @@ fn render_detail(frame: &mut Frame, app: &mut App, area: Rect, repo: Option<&Rep
477 let label = action_type_label(&evt.action); 662 let label = action_type_label(&evt.action);
478 ListItem::new(format!( 663 ListItem::new(format!(
479 "{} | {} | {}", 664 "{} | {} | {}",
480 label, evt.author.name, evt.timestamp 665 label,
666 evt.author.name,
667 short_time(&evt.timestamp)
481 )) 668 ))
482 }) 669 })
483 .collect(); 670 .collect();
@@ -605,7 +792,7 @@ fn build_issue_detail(
605 ]), 792 ]),
606 Line::from(vec![ 793 Line::from(vec![
607 Span::styled("Created: ", Style::default().fg(Color::DarkGray)), 794 Span::styled("Created: ", Style::default().fg(Color::DarkGray)),
608 Span::raw(issue.created_at.clone()), 795 Span::raw(short_time(&issue.created_at)),
609 ]), 796 ]),
610 ]; 797 ];
611 798
@@ -691,7 +878,7 @@ fn build_issue_detail(
691 Style::default().add_modifier(Modifier::BOLD), 878 Style::default().add_modifier(Modifier::BOLD),
692 ), 879 ),
693 Span::styled( 880 Span::styled(
694 format!(" ({})", c.timestamp), 881 format!(" ({})", short_time(&c.timestamp)),
695 Style::default().fg(Color::DarkGray), 882 Style::default().fg(Color::DarkGray),
696 ), 883 ),
697 edited_span(c.edited), 884 edited_span(c.edited),
@@ -730,12 +917,19 @@ fn build_issue_detail(
730 let line_text = if commit_author.is_empty() { 917 let line_text = if commit_author.is_empty() {
731 format!( 918 format!(
732 "· linked {} (commit {} not in local repo) (linked by {}, {})", 919 "· linked {} (commit {} not in local repo) (linked by {}, {})",
733 short_sha, short_sha, lc.event_author.name, lc.event_timestamp 920 short_sha,
921 short_sha,
922 lc.event_author.name,
923 short_time(&lc.event_timestamp)
734 ) 924 )
735 } else { 925 } else {
736 format!( 926 format!(
737 "· linked {} \"{}\" by {} (linked by {}, {})", 927 "· linked {} \"{}\" by {} (linked by {}, {})",
738 short_sha, subject, commit_author, lc.event_author.name, lc.event_timestamp 928 short_sha,
929 subject,
930 commit_author,
931 lc.event_author.name,
932 short_time(&lc.event_timestamp)
739 ) 933 )
740 }; 934 };
741 lines.push(Line::raw(line_text)); 935 lines.push(Line::raw(line_text));
@@ -911,7 +1105,11 @@ pub(crate) fn build_patch_detail_rows(app: &App) -> Vec<DetailRow> {
911 let label = if i == 0 { " (initial)" } else { "" }; 1105 let label = if i == 0 { " (initial)" } else { "" };
912 rows.push(Line::from(vec![Span::raw(format!( 1106 rows.push(Line::from(vec![Span::raw(format!(
913 "{}r{} {} {}{}", 1107 "{}r{} {} {}{}",
914 marker, rev.number, short, rev.timestamp, label 1108 marker,
1109 rev.number,
1110 short,
1111 short_time(&rev.timestamp),
1112 label
915 ))])); 1113 ))]));
916 } 1114 }
917 } 1115 }
@@ -942,7 +1140,7 @@ pub(crate) fn build_patch_detail_rows(app: &App) -> Vec<DetailRow> {
942 review.author.name.clone(), 1140 review.author.name.clone(),
943 Style::default().add_modifier(Modifier::BOLD), 1141 Style::default().add_modifier(Modifier::BOLD),
944 ), 1142 ),
945 Span::raw(format!(" {} ", review.timestamp)), 1143 Span::raw(format!(" {} ", short_time(&review.timestamp))),
946 Span::styled(verdict_str, Style::default().fg(verdict_color)), 1144 Span::styled(verdict_str, Style::default().fg(verdict_color)),
947 Span::raw(rev_label), 1145 Span::raw(rev_label),
948 edited_span(review.edited), 1146 edited_span(review.edited),
@@ -1021,7 +1219,7 @@ pub(crate) fn build_patch_detail_rows(app: &App) -> Vec<DetailRow> {
1021 Style::default().add_modifier(Modifier::BOLD), 1219 Style::default().add_modifier(Modifier::BOLD),
1022 ), 1220 ),
1023 Span::styled( 1221 Span::styled(
1024 format!(" ({})", c.timestamp), 1222 format!(" ({})", short_time(&c.timestamp)),
1025 Style::default().fg(Color::DarkGray), 1223 Style::default().fg(Color::DarkGray),
1026 ), 1224 ),
1027 edited_span(c.edited), 1225 edited_span(c.edited),
@@ -1083,31 +1281,39 @@ fn push_diff_rows(rows: &mut Rows, app: &App) {
1083 rows.push(Line::raw("(no diff available)")); 1281 rows.push(Line::raw("(no diff available)"));
1084 return; 1282 return;
1085 } 1283 }
1284 // Every line from here down is a line of a file, so `w` governs it.
1086 for row in &app.patch_diff { 1285 for row in &app.patch_diff {
1087 let text = row.text.trim_end_matches(['\n', '\r']).to_string(); 1286 let target = match &row.anchor {
1088 let style = if text.starts_with("diff ") { 1287 Some((file, number)) => RowTarget::Diff {
1089 Style::default() 1288 file: file.clone(),
1090 .fg(Color::Yellow) 1289 line: *number,
1091 .add_modifier(Modifier::BOLD) 1290 },
1092 } else if text.starts_with("@@") { 1291 None => RowTarget::None,
1093 Style::default().fg(Color::Cyan)
1094 } else if text.starts_with('+') {
1095 Style::default().fg(Color::Green)
1096 } else if text.starts_with('-') {
1097 Style::default().fg(Color::Red)
1098 } else {
1099 Style::default()
1100 }; 1292 };
1101 let line = Line::styled(text, style); 1293 // One row per *physical* line. git hands the whole file header —
1102 match &row.anchor { 1294 // `diff --git`, `index`, `--- a/f` — over as a single diff row with
1103 Some((file, number)) => rows.push_target( 1295 // newlines inside it, and a `Line` has nowhere to put a newline: it
1104 line, 1296 // used to arrive as one unreadable row, and its width would have lied
1105 RowTarget::Diff { 1297 // to the wrapper. Each piece keeps the anchor of the row it came from.
1106 file: file.clone(), 1298 // The stored text ends with its newline; that one is a terminator and
1107 line: *number, 1299 // not an empty line after it.
1108 }, 1300 let body = row.text.strip_suffix('\n').unwrap_or(&row.text);
1109 ), 1301 for text in body.split('\n') {
1110 None => rows.push(line), 1302 let text = text.trim_end_matches('\r').to_string();
1303 let style = if text.starts_with("diff ") {
1304 Style::default()
1305 .fg(Color::Yellow)
1306 .add_modifier(Modifier::BOLD)
1307 } else if text.starts_with("@@") {
1308 Style::default().fg(Color::Cyan)
1309 } else if text.starts_with('+') {
1310 Style::default().fg(Color::Green)
1311 } else if text.starts_with('-') {
1312 Style::default().fg(Color::Red)
1313 } else {
1314 Style::default()
1315 };
1316 rows.push_diff(Line::styled(text, style), target.clone());
1111 } 1317 }
1112 } 1318 }
1113 } 1319 }
@@ -1167,7 +1373,7 @@ fn render_footer(frame: &mut Frame, app: &App, area: Rect) {
1167 // way out, then the things you can also get to by other means. 1373 // way out, then the things you can also get to by other means.
1168 ViewMode::PatchDetail => { 1374 ViewMode::PatchDetail => {
1169 " j/k:line c:comment R:review x:resolve a:answers Esc:back q:quit \ 1375 " j/k:line c:comment R:review x:resolve a:answers Esc:back q:quit \
1170 [/]:revision d:interdiff o:checkout" 1376 [/]:revision d:interdiff o:checkout w:wrap"
1171 .to_string() 1377 .to_string()
1172 } 1378 }
1173 ViewMode::Details => { 1379 ViewMode::Details => {
tests/tui_review_test.rs
Old New
@@ -138,6 +138,100 @@ fn the_dashboards_own_write_does_not_raise_the_banner() {
138 ); 138 );
139 } 139 }
140 140
141 // ── Wrapping the patch detail pane (9f9dee31) ───────────────────────────────
142
143 /// The defect in a real terminal: a diff line wider than the pane used to end
144 /// at the right edge with nothing to say the rest existed. The end of the line
145 /// is a distinct token, so its presence on screen is the whole property — and
146 /// it is checked through a pty because the clipping was a property of the pane
147 /// as drawn, not of the row list.
148 #[test]
149 fn a_diff_line_wider_than_the_pane_is_not_clipped_away() {
150 let repo = TestRepo::new("Reviewer", "reviewer@example.com");
151 repo.commit_file("src/lib.rs", "one\ntwo\n", "seed");
152 repo.git(&["checkout", "-b", "feature/wide"]);
153 // Far wider than the ~63 columns the detail pane gets at 100 across, with
154 // the far end named so the test can look for it rather than for a length.
155 let wide = format!("let x = \"{}\"; // ENDOFTHEWIDELINE", "w".repeat(300));
156 repo.commit_file("src/lib.rs", &format!("one\ntwo\n{}\n", wide), "a wide line");
157 repo.run_ok(&["patch", "create", "-t", "A wide patch", "-B", "feature/wide"]);
158 repo.git(&["checkout", "main"]);
159
160 let out = repo.run_dashboard_driven(100, 45, &[], |stdin| {
161 let _ = stdin.write_all(b"P");
162 thread::sleep(Duration::from_millis(400));
163 let _ = stdin.write_all(b"\r");
164 thread::sleep(Duration::from_millis(1200));
165 let _ = stdin.write_all(b"q");
166 });
167 let text = screen_text(&out.stdout);
168 assert!(
169 text.contains("ENDOFTHEWIDELINE"),
170 "the end of a 300-column diff line never reached the screen:\n{}",
171 text
172 );
173 }
174
175 // ── Timestamps (952390ad) ───────────────────────────────────────────────────
176
177 /// The defect as it was captured: a real pty, a real repository, and
178 /// `Created: 2026-08-12T16:57:16.325870642+00:00` on the issue pane.
179 ///
180 /// Both halves are asserted from one screen. Stored timestamps are RFC3339
181 /// with a `+00:00` offset and nothing else the dashboard prints ends that way,
182 /// so the offset's absence covers every pane the walk visits at once; and the
183 /// short form has to be positively there, or dropping the field entirely would
184 /// satisfy it. The expected value comes from the stored one the CLI prints
185 /// rather than from the clock, so nothing here races midnight.
186 #[test]
187 fn no_pane_prints_a_stored_timestamp_in_full() {
188 let (repo, id) = repo_with_patch();
189 repo.issue_open("something with a date on it");
190 repo.run_ok(&["patch", "comment", &id, "-b", "a remark with a date on it"]);
191
192 let listed = repo.run_ok(&["issue", "list"]);
193 let issue = listed.split_whitespace().next().expect("one issue");
194 let shown = repo.run_ok(&["issue", "show", issue]);
195 let stored = shown
196 .lines()
197 .find_map(|l| l.strip_prefix("Created: "))
198 .expect("issue show prints the stored timestamp")
199 .trim()
200 .to_string();
201 // `2026-08-13T07:33:57.151572101+00:00` -> `2026-08-1307:33`. The space
202 // between the date and the time is left out on purpose: ratatui writes only
203 // the cells that differ from what is already there, and a cell holding a
204 // space starts out holding a space — so an interior blank never reaches the
205 // stream this reads. Everything either side of it does.
206 let expected = format!("{}{}", &stored[..10], &stored[11..16]);
207
208 // Walk the panes a reader walks: the issue detail, then the patch list,
209 // then the patch detail with its revisions, comments and diff.
210 let out = repo.run_dashboard_driven(120, 45, &[], |stdin| {
211 let _ = stdin.write_all(b"\t");
212 thread::sleep(Duration::from_millis(400));
213 let _ = stdin.write_all(b"P");
214 thread::sleep(Duration::from_millis(400));
215 let _ = stdin.write_all(b"\r");
216 thread::sleep(Duration::from_millis(800));
217 let _ = stdin.write_all(b"q");
218 });
219 let text = screen_text(&out.stdout);
220
221 assert!(
222 text.contains(&expected),
223 "expected the date rendered as `{} {}`:\n{}",
224 &stored[..10],
225 &stored[11..16],
226 text
227 );
228 assert!(
229 !text.contains("+00:00"),
230 "a stored timestamp reached the screen in full:\n{}",
231 text
232 );
233 }
234
141 // ── Writing from the review surface (094b055b) ────────────────────────────── 235 // ── Writing from the review surface (094b055b) ──────────────────────────────
142 236
143 /// The whole point of the issue: read a patch and say something about it 237 /// The whole point of the issue: read a patch and say something about it