From 9e54cd2c5b53d939d1d5ec351d56fbac16e168f0 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 1 Sep 2026 10:34:00 +0200 Subject: [PATCH] refactor: put the display-column rule where both frontends share it The previous commit CLAIMED the widest-line rule was shared. It was not. The daemon called `src/display_width.rs`; the GPU folded through its own private `advance_display_col`, a second copy of the same tab-stop and Unicode-width arithmetic. The two agreed for ordinary input, so nothing failed --- which is precisely why the claim was worth checking and why asserting structural protection that does not exist is the defect, not the duplication itself. `pmacs_protocol::columns` now owns the rule, for the same reason `scroll::follow_left` lives there: the protocol crate is the one place both frontends already depend on. `advance_char`, `line_columns` and `widest_line_columns` live there with their own rows; `display_width` and the GPU both delegate. The sharing is now demonstrated rather than described. Mutating the tab stop inside `pmacs_protocol::columns` breaks the GPU's `minimap_columns_match_code_tab_and_unicode_widths` --- a row that used to run entirely through the private copy and could not have noticed. Also restores `r4_p1_a_chrome_press_neither_arms_nor_moves_point`'s opening line, "P1 --- a press on the band's MODE LINE begins nothing", which my insertion had left attached to the B2 test. The attribute came back last round; the first paragraph did not. --- pmacs-gpu/src/main.rs | 28 ++++-------- pmacs-protocol/src/columns.rs | 82 +++++++++++++++++++++++++++++++++++ pmacs-protocol/src/lib.rs | 1 + src/daemon.rs | 4 +- src/display_width.rs | 31 ++++--------- 5 files changed, 102 insertions(+), 44 deletions(-) create mode 100644 pmacs-protocol/src/columns.rs diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index d7a27c7..69977b9 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -8389,13 +8389,7 @@ impl State { /// so the witness states which meaning governs rather than /// discovering it. fn widest_display_columns(&self) -> u32 { - let widest = self - .current_text - .split('\n') - .map(|line| line.chars().fold(0usize, advance_display_col)) - .max() - .unwrap_or(0); - u32::try_from(widest).unwrap_or(u32::MAX) + pmacs_protocol::columns::widest_line_columns(&self.current_text) } /// GUI Stage 1b B3/B7: move the horizontal origin by whole columns. @@ -12874,20 +12868,16 @@ fn minimap_line_shape(line: &str) -> MinimapLineShape { } } -/// Advance a display column past one character: tab stops, then -/// Unicode terminal width. +/// Advance a display column past one character. /// -/// Shared by the minimap and by B3's widest-line bound so the two -/// cannot disagree about what a column is. It is the same rule as the -/// TUI's `display_width::advance_char`; that copy lives in the other -/// crate. +/// **Delegates to [`pmacs_protocol::columns::advance_char`]**, which is +/// where the rule lives. It used to be a private copy of the same +/// arithmetic, which is exactly the drift this crate and the daemon +/// must not have: a bound computed one way and a follow computed the +/// other disagree about where the document ends, invisibly until a tab +/// or a wide character reaches the edge. fn advance_display_col(col: usize, ch: char) -> usize { - if ch == '\t' { - let tab_stop = TAB_STOP_COLUMNS as usize; - col + tab_stop - col % tab_stop - } else { - col + UnicodeWidthChar::width(ch).unwrap_or(0) - } + pmacs_protocol::columns::advance_char(u32::try_from(col).unwrap_or(u32::MAX), ch) as usize } fn minimap_style_color(style: CellStyle) -> [f32; 4] { diff --git a/pmacs-protocol/src/columns.rs b/pmacs-protocol/src/columns.rs new file mode 100644 index 0000000..81945e2 --- /dev/null +++ b/pmacs-protocol/src/columns.rs @@ -0,0 +1,82 @@ +// columns.rs --- The display-column rule both frontends reckon in. + +//! One definition of "a display column", shared across the wire. +//! +//! The TUI and the GPU both need to answer "how wide is this line?" — +//! for the caret follow, for the minimap, and for GUI Stage 1b's B7 +//! right bound. **Two copies of that answer is a defect waiting to +//! happen**: a bound computed one way and a follow computed the other +//! disagree about where the document ends, and the disagreement is +//! invisible until a tab or a wide character reaches the edge. +//! +//! It lives here for the same reason [`crate::scroll::follow_left`] +//! does — the protocol crate is the one place both frontends already +//! depend on. +//! +//! **Scope: SOURCE-TEXT columns.** Tab stops and Unicode terminal +//! width. Rendered projections — inline adornments, math substitutions +//! — can occupy a different width on screen and are deliberately not +//! counted here. + +use unicode_width::UnicodeWidthChar; + +/// Advance `column` past one character. +/// +/// A tab reaches the next [`crate::TAB_STOP_COLUMNS`] stop; every other +/// character contributes its Unicode terminal width, so control and +/// zero-width characters do not advance. +#[must_use] +pub fn advance_char(column: u32, ch: char) -> u32 { + let width = if ch == '\t' { + crate::TAB_STOP_COLUMNS - (column % crate::TAB_STOP_COLUMNS) + } else { + UnicodeWidthChar::width(ch).unwrap_or(0) as u32 + }; + column.saturating_add(width) +} + +/// Display width of one line, in columns. +#[must_use] +pub fn line_columns(line: &str) -> u32 { + line.chars().fold(0, advance_char) +} + +/// Widest line in `text`, in display columns — B7's right-bound input. +#[must_use] +pub fn widest_line_columns(text: &str) -> u32 { + text.split('\n').map(line_columns).max().unwrap_or(0) +} + +#[cfg(test)] +mod tests { + use super::{advance_char, line_columns, widest_line_columns}; + + #[test] + fn a_tab_reaches_the_next_stop_rather_than_advancing_one() { + assert_eq!(advance_char(0, '\t'), crate::TAB_STOP_COLUMNS); + assert_eq!(advance_char(1, '\t'), crate::TAB_STOP_COLUMNS); + assert_eq!( + advance_char(crate::TAB_STOP_COLUMNS, '\t'), + crate::TAB_STOP_COLUMNS * 2 + ); + } + + #[test] + fn wide_and_zero_width_characters_are_measured_not_counted() { + assert_eq!(line_columns("ab"), 2); + assert_eq!(line_columns("漢字"), 4, "wide characters take two columns"); + assert_eq!(line_columns("a\u{200b}b"), 2, "zero-width adds nothing"); + } + + /// The widest line, not the last one and not the first. + #[test] + fn widest_line_is_the_maximum_over_all_lines() { + assert_eq!(widest_line_columns("a\nbbbb\ncc"), 4); + assert_eq!(widest_line_columns(""), 0); + assert_eq!( + widest_line_columns("\tx"), + crate::TAB_STOP_COLUMNS + 1, + "tabs count toward the bound" + ); + } +} diff --git a/pmacs-protocol/src/lib.rs b/pmacs-protocol/src/lib.rs index 858359e..d1d4700 100644 --- a/pmacs-protocol/src/lib.rs +++ b/pmacs-protocol/src/lib.rs @@ -37,6 +37,7 @@ //! directly. pub mod cell; +pub mod columns; pub mod crdt; pub mod ids; pub mod message; diff --git a/src/daemon.rs b/src/daemon.rs index 7a1c0f6..549facb 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -7671,8 +7671,6 @@ mod tests { editor.core.borrow().windows[&panel].cursor } - /// P1 — a press on the band's MODE LINE begins nothing. - /// /// GUI Stage 1b B2 — a horizontal panel notch reaches the daemon's /// window-targeted `view_left` path and MOVES it. /// @@ -7737,6 +7735,8 @@ mod tests { ); } + /// P1 — a press on the band's MODE LINE begins nothing. + /// /// The merge made this arm the latch, because `Consumed` and /// `Accepted` were the same `true`. The row reads the cursor as well /// as the latch: a chrome press must not move point either. diff --git a/src/display_width.rs b/src/display_width.rs index 89386de..6b8f7cf 100644 --- a/src/display_width.rs +++ b/src/display_width.rs @@ -7,38 +7,23 @@ //! stop. Offsets are clamped to the supplied slice and offsets inside a UTF-8 //! code point resolve to the preceding complete-code-point boundary. -use unicode_width::UnicodeWidthChar; - /// Advance `column` past one character. /// -/// A tab reaches the next protocol tab stop; all other characters use their -/// Unicode terminal width. Control and zero-width characters do not advance. +/// **Delegates to [`pmacs_protocol::columns::advance_char`]**, which is +/// where the rule lives so both frontends cannot drift apart on what a +/// column is. This re-export keeps the existing call sites. #[must_use] pub fn advance_char(column: u32, ch: char) -> u32 { - let width = if ch == '\t' { - pmacs_protocol::TAB_STOP_COLUMNS - (column % pmacs_protocol::TAB_STOP_COLUMNS) - } else { - UnicodeWidthChar::width(ch).unwrap_or(0) as u32 - }; - column.saturating_add(width) + pmacs_protocol::columns::advance_char(column, ch) } -/// Widest line in `text`, in display columns. +/// Widest line in `text`, in display columns — B7's right bound. /// -/// GUI Stage 1b B7's upper-bound input, shared by both frontends so -/// they cannot disagree about where the right bound is — the same -/// reason `scroll::follow_left` is shared. -/// -/// **This measures SOURCE-TEXT display columns**: tab stops and Unicode -/// width. Rendered projections — inline adornments, math substitutions -/// — can occupy a different width on screen and are deliberately not -/// counted, matching the column rule the rest of this module states. +/// **Delegates to [`pmacs_protocol::columns::widest_line_columns`]**, +/// which both frontends call. #[must_use] pub fn widest_line_columns(text: &str) -> u32 { - text.split('\n') - .map(|line| line.chars().fold(0, advance_char)) - .max() - .unwrap_or(0) + pmacs_protocol::columns::widest_line_columns(text) } /// Display width of the valid UTF-8 prefix of `bytes`.