diff --git a/Cargo.lock b/Cargo.lock index 9ed454f..be491fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2583,6 +2583,7 @@ dependencies = [ "pmacs-protocol", "pollster", "sys-locale", + "unicode-width", "wgpu", "winit", ] diff --git a/docs/tab-width-parity-framing.md b/docs/tab-width-parity-framing.md index e71e5c9..8b4d1e4 100644 --- a/docs/tab-width-parity-framing.md +++ b/docs/tab-width-parity-framing.md @@ -1,6 +1,7 @@ # Tab-width rendering parity - side quest -**Status:** Revision 2 framing; review findings incorporated; awaiting user approval. +**Status:** Revision 2 implemented on `tab-width-parity`; all fifteen +acceptance criteria pass locally. Awaiting pull-request review. **Base:** `githubsucks/main` at `40111dc` (landed-state documentation for locals-query processing #134); protocol v18. diff --git a/pmacs-gpu/Cargo.toml b/pmacs-gpu/Cargo.toml index bdf2937..c00923d 100644 --- a/pmacs-gpu/Cargo.toml +++ b/pmacs-gpu/Cargo.toml @@ -61,3 +61,4 @@ pmacs-protocol = { version = "1.0.0", path = "../pmacs-protocol" } pollster = "0.4.0" wgpu = "29.0.3" winit = "0.30.13" +unicode-width = "0.2" diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index 4da619e..7f50d52 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -40,10 +40,11 @@ use pmacs_protocol::{ InstanceSignal, Key as ProtocolKey, LineNumberMode, MAX_STATUSLINE_FACE_BYTES, MAX_STATUSLINE_PROVIDERS, MAX_STATUSLINE_SEGMENT_BYTES, MAX_STATUSLINE_TOTAL_TEXT_BYTES, MenuPromptRow, Modifiers, PointerKind, SelectionSnapshot, StatuslineSegment, StyleSegment, - StyleSpan, + StyleSpan, TAB_STOP_COLUMNS, cell::{Color as CellColor, Style as CellStyle}, is_builtin_pair_char, is_modeline_face_name, }; +use unicode_width::UnicodeWidthChar; use wgpu::MultisampleState; use winit::application::ApplicationHandler; use winit::event::{ElementState, WindowEvent}; @@ -679,9 +680,9 @@ struct State { current_line_char_starts: Vec, /// Code-shape data used to give the minimap horizontal structure /// even though `FileStyleSummary` carries only one dominant style - /// per line. Refreshed when a new summary lands, keeping this - /// cache in cadence with the debounced minimap data rather than - /// rebuilding it for every typed byte. + /// per line. Summary replacement rebuilds the table; accepted text + /// edits update the affected line immediately (or rebuild after + /// structural/batched edits). current_line_shapes: Vec, /// Local CRDT replica seeded by `BufferSnapshot`. `None` in /// hello-world mode or before the first snapshot arrives in @@ -2606,6 +2607,7 @@ impl State { if edits.is_empty() { return Ok(edits); } + self.refresh_minimap_shapes_after_edits(&edits, line_count_before); self.translate_cached_anchors(&edits); // A newline edit can cross a gutter digit boundary (9 -> 10, // 99 -> 100). Synchronize the painter-derived code width @@ -2636,6 +2638,36 @@ impl State { } } + /// Keep minimap horizontal geometry in lock-step with accepted text + /// edits instead of waiting for the next debounced style summary. + /// The common one-line edit updates one cached shape; line-structure + /// or batched edits rebuild the table because their intermediate + /// coordinates need not describe the final line partition. + fn refresh_minimap_shapes_after_edits( + &mut self, + edits: &[TextProjectionEdit], + line_count_before: usize, + ) { + if edits.len() == 1 + && self.current_line_starts.len() == line_count_before + && self.current_line_shapes.len() == self.current_line_starts.len() + { + let line = self + .current_line_starts + .partition_point(|&start| start <= edits[0].start) + .saturating_sub(1); + let start = self.current_line_starts[line] as usize; + let end = self + .current_line_starts + .get(line + 1) + .map_or(self.current_text.len(), |next| *next as usize - 1); + self.current_line_shapes[line] = minimap_line_shape(&self.current_text[start..end]); + } else { + self.current_line_shapes = minimap_line_shapes(&self.current_text); + } + self.minimap_cache = None; + } + /// Drop journal entries already reflected in a producer frame /// stamped `generation` — see the `unconfirmed_edits` field docs. fn prune_unconfirmed_edits(&mut self, generation: u64) { @@ -2705,6 +2737,8 @@ impl State { let (line_starts, line_char_starts) = line_offset_tables(text); self.current_line_starts = line_starts; self.current_line_char_starts = line_char_starts; + self.current_line_shapes = minimap_line_shapes(text); + self.minimap_cache = None; let geometry_changed = self.sync_buffer_dimensions(); self.reshape(); if geometry_changed && caret_was_painted { @@ -6070,46 +6104,21 @@ impl State { }) } - /// Map an absolute source `byte` to `(slice line index, projected - /// byte offset within that shaped line)` by inverting the line's - /// `line_chunk_cache` projection (framing Q#F6): source bytes are - /// not projected bytes once inline adornments inject text. An - /// adornment anchor maps to the EARLIEST projected boundary — the - /// current left-gravity caret placement, before the injected - /// text. `None` when the byte's source line is outside the shaped - /// slice. + /// Map an absolute source byte to `(slice line index, projected + /// byte offset within that shaped line)` through the same reusable + /// chunk mapping used by decoration geometry. + /// Adornments retain left gravity, while a source tab's two byte + /// boundaries map to the leading and trailing edges of all of its + /// projected spaces. fn code_byte_to_projected(&self, byte: u64) -> Option<(usize, usize)> { let line_idx = self .current_line_starts .partition_point(|&s| s <= byte) .saturating_sub(1); let slice_i = line_idx.checked_sub(self.shaped_top)?; - if slice_i >= self.line_chunk_cache.len() { - return None; - } + let chunks = self.line_chunk_cache.get(slice_i)?; let rel = byte - self.current_line_starts[line_idx]; - let mut projected = 0usize; - for chunk in &self.line_chunk_cache[slice_i] { - match chunk.source { - ChunkSource::Source { start } => { - let len = chunk.text.len() as u64; - if rel >= start && rel < start + len { - return Some((slice_i, projected + (rel - start) as usize)); - } - } - ChunkSource::Adornment { anchor } => { - // Source chunks tile the line, so reaching an - // adornment chunk unmatched means the byte sits at - // its anchor boundary (or past line end). - if rel <= anchor { - return Some((slice_i, projected)); - } - } - } - projected += chunk.text.len(); - } - // Line end (the `\n` position, or EOF). - Some((slice_i, projected)) + source_to_projected(chunks, rel).map(|projected| (slice_i, projected as usize)) } /// Convert an absolute source byte to a cursor cosmic-text can @@ -6255,11 +6264,11 @@ impl State { } /// Push one rect per visual line whose glyphs overlap the - /// buffer-absolute byte range `[lo, hi)`, spanning the matching - /// glyphs' horizontal extent. A range crossing visual-line - /// boundaries (wrapped or multi-line) fans out into one rect per - /// run. `line_offsets[run.line_i]` rebases the run's line-relative - /// glyph offsets into buffer-absolute space for the comparison. + /// slice-relative source byte range `[lo, hi)`, spanning the + /// matching projected glyphs' horizontal extent. Each source-line + /// intersection is mapped through its cached chunks first, so a + /// source tab covers every expanded space even when a soft wrap + /// divides those spaces between visual runs. fn push_glyph_extent_rects( &self, rects: &mut Vec, @@ -6276,12 +6285,30 @@ impl State { let text_left = self.text_left(); for run in self.buffer.layout_runs() { let line_base = line_offsets.get(run.line_i).copied().unwrap_or(0); + let line_end = line_offsets + .get(run.line_i + 1) + .copied() + .unwrap_or(self.view_range.1 - self.view_range.0); + let source_lo = lo.max(line_base); + let source_hi = hi.min(line_end); + if source_hi <= source_lo { + continue; + } + let Some(chunks) = self.line_chunk_cache.get(run.line_i) else { + continue; + }; + let Some(projected_lo) = source_to_projected(chunks, source_lo - line_base) else { + continue; + }; + let Some(projected_hi) = source_to_projected(chunks, source_hi - line_base) else { + continue; + }; let mut min_x: Option = None; let mut max_x: Option = None; for glyph in run.glyphs { - let g_start = line_base + glyph.start as u64; - let g_end = line_base + glyph.end as u64; - if g_end <= lo || g_start >= hi { + let g_start = glyph.start as u64; + let g_end = glyph.end as u64; + if g_end <= projected_lo || g_start >= projected_hi { continue; } let x0 = glyph.x; @@ -6343,6 +6370,8 @@ struct RichChunk { enum ChunkSource { /// Verbatim source text starting at this slice byte offset. Source { start: u64 }, + /// One source tab byte expanded into one or more projected spaces. + SourceTab { start: u64 }, /// Injected adornment text (inlay hint) anchored at this slice /// byte offset. Hits inside it snap to the anchor. Adornment { anchor: u64 }, @@ -6383,9 +6412,10 @@ fn build_hit_runs(chunks: &[RichChunk]) -> (Vec, Vec) { (runs, line_starts) } -/// Map a projected byte offset back to a slice-relative source byte -/// (Q#M2). Hits inside an adornment run snap to its anchor; offsets -/// past the last run clamp to its end. +/// Map a projected byte offset back to a slice-relative source byte. +/// A source tab's leading boundary maps before the byte; every +/// boundary inside its expanded spaces (including the trailing edge) +/// maps after it. Adornments snap to their left-gravity anchor. fn projected_to_source(runs: &[ProjectedRun], projected: u64) -> Option { if runs.is_empty() { return None; @@ -6397,10 +6427,48 @@ fn projected_to_source(runs: &[ProjectedRun], projected: u64) -> Option { let within = projected.saturating_sub(run.projected_start).min(run.len); match run.source { ChunkSource::Source { start } => Some(start + within), + ChunkSource::SourceTab { start } => Some(start + u64::from(within > 0)), ChunkSource::Adornment { anchor } => Some(anchor), } } +/// Map a slice-relative source boundary into projected byte space. +/// This is the inverse boundary policy shared by caret placement and +/// horizontal decoration geometry. At an adornment anchor the earliest +/// projected boundary wins, preserving left gravity. +fn source_to_projected(chunks: &[RichChunk], source: u64) -> Option { + let mut projected = 0u64; + for chunk in chunks { + let len = chunk.text.len() as u64; + match chunk.source { + ChunkSource::Source { start } => { + if source <= start { + return Some(projected); + } + let end = start + len; + if source <= end { + return Some(projected + source - start); + } + } + ChunkSource::SourceTab { start } => { + if source <= start { + return Some(projected); + } + if source <= start + 1 { + return Some(projected + len); + } + } + ChunkSource::Adornment { anchor } => { + if source <= anchor { + return Some(projected); + } + } + } + projected += len; + } + (!chunks.is_empty()).then_some(projected) +} + fn minimap_left(surface_width: u32) -> Option { if surface_width < MINIMAP_MIN_SURFACE_WIDTH { return None; @@ -6727,9 +6795,10 @@ fn minimap_line_shape(line: &str) -> MinimapLineShape { fn advance_minimap_col(col: usize, ch: char) -> usize { if ch == '\t' { - ((col / 4) + 1) * 4 + let tab_stop = TAB_STOP_COLUMNS as usize; + col + tab_stop - col % tab_stop } else { - col + 1 + col + UnicodeWidthChar::width(ch).unwrap_or(0) } } @@ -7666,7 +7735,89 @@ fn projected_rich_chunks( source: ChunkSource::Source { start: 0 }, }); } - chunks + expand_chunk_tabs(chunks) +} + +/// Expand display tabs after source styling and adornment insertion. +/// Chunks without tabs are moved through unchanged. A chunk containing +/// tabs is split only at those bytes; every emitted space keeps the +/// original color, while source tabs gain explicit provenance. +fn expand_chunk_tabs(chunks: Vec) -> Vec { + let mut expanded = Vec::with_capacity(chunks.len()); + let mut column = 0usize; + for chunk in chunks { + if !chunk.text.contains('\t') { + advance_display_column(&mut column, &chunk.text); + expanded.push(chunk); + continue; + } + + let RichChunk { + text, + color, + source, + } = chunk; + let mut segment_start = 0usize; + for (byte, ch) in text.char_indices() { + if ch != '\t' { + continue; + } + if segment_start < byte { + let segment = &text[segment_start..byte]; + advance_display_column(&mut column, segment); + expanded.push(RichChunk { + text: segment.to_owned(), + color, + source: offset_chunk_source(source, segment_start as u64), + }); + } + let tab_stop = TAB_STOP_COLUMNS as usize; + let tab_width = tab_stop - column % tab_stop; + expanded.push(RichChunk { + text: " ".repeat(tab_width), + color, + source: match source { + ChunkSource::Source { start } => ChunkSource::SourceTab { + start: start + byte as u64, + }, + ChunkSource::Adornment { anchor } => ChunkSource::Adornment { anchor }, + ChunkSource::SourceTab { start } => ChunkSource::SourceTab { start }, + }, + }); + column += tab_width; + segment_start = byte + 1; + } + if segment_start < text.len() { + let segment = &text[segment_start..]; + advance_display_column(&mut column, segment); + expanded.push(RichChunk { + text: segment.to_owned(), + color, + source: offset_chunk_source(source, segment_start as u64), + }); + } + } + expanded +} + +fn offset_chunk_source(source: ChunkSource, byte_offset: u64) -> ChunkSource { + match source { + ChunkSource::Source { start } => ChunkSource::Source { + start: start + byte_offset, + }, + ChunkSource::SourceTab { start } => ChunkSource::SourceTab { start }, + ChunkSource::Adornment { anchor } => ChunkSource::Adornment { anchor }, + } +} + +fn advance_display_column(column: &mut usize, text: &str) { + for ch in text.chars() { + if ch == '\n' { + *column = 0; + } else { + *column += UnicodeWidthChar::width(ch).unwrap_or(0); + } + } } fn renderable_adornment_anchor(adornment: &InlineAdornment, text_len: u64) -> Option { @@ -8564,6 +8715,109 @@ mod tests { assert_eq!(projected_to_source(&[], 0), None); } + #[test] + fn tab_projection_uses_shared_stops_and_unicode_columns() { + let projected = |text: &str| { + projected_rich_chunks(text, &[], &[]) + .into_iter() + .map(|chunk| chunk.text) + .collect::() + }; + + assert_eq!(projected("\t"), " ", "column 0 advances to 8"); + assert_eq!(projected("1234567\t"), "1234567 ", "column 7 advances to 8"); + assert_eq!( + projected("12345678\t"), + "12345678 ", + "column 8 advances to 16" + ); + assert_eq!( + projected("界\t\n\u{301}\t"), + "界 \n\u{301} ", + "wide scalars count as two, zero-width scalars as zero, and newline resets" + ); + } + + #[test] + fn tab_projection_preserves_source_and_adornment_provenance_and_style() { + let red = CellColor::Rgb(255, 0, 0); + let chunks = projected_rich_chunks( + "1234567\tX", + &[span(7, 8, red)], + &[adornment(0, AdornmentPlacement::AtOffset, "\t")], + ); + assert_eq!( + chunks + .iter() + .map(|chunk| chunk.text.as_str()) + .collect::(), + " 1234567 X", + "the adornment tab participates in the same logical column stream" + ); + let source_tab = chunks + .iter() + .find(|chunk| matches!(chunk.source, ChunkSource::SourceTab { start: 7 })) + .expect("source tab has a first-class projected run"); + assert_eq!(source_tab.text, " "); + assert_eq!(source_tab.color, cell_color_to_glyphon(red)); + assert!( + chunks.iter().any( + |chunk| matches!(chunk.source, ChunkSource::Adornment { anchor: 0 }) + && chunk.text == " " + ), + "adornment tabs expand without pretending to be source bytes" + ); + } + + #[test] + fn tab_projection_moves_chunks_without_tabs_unchanged() { + let text = String::from("wide 界 and plain"); + let allocation = text.as_ptr(); + let chunks = expand_chunk_tabs(vec![RichChunk { + text, + color: None, + source: ChunkSource::Source { start: 0 }, + }]); + + assert_eq!(chunks.len(), 1); + assert_eq!(chunks[0].text.as_ptr(), allocation); + } + + #[test] + fn source_tab_projection_boundaries_are_bidirectional() { + let chunks = projected_rich_chunks("\tX", &[], &[]); + let (runs, _) = build_hit_runs(&chunks); + + assert_eq!(source_to_projected(&chunks, 0), Some(0)); + assert_eq!(source_to_projected(&chunks, 1), Some(8)); + assert_eq!(source_to_projected(&chunks, 2), Some(9)); + assert_eq!(projected_to_source(&runs, 0), Some(0)); + for projected in 1..=8 { + assert_eq!( + projected_to_source(&runs, projected), + Some(1), + "projected boundary {projected} inside the tab maps after its source byte" + ); + } + assert_eq!(projected_to_source(&runs, 9), Some(2)); + } + + #[test] + fn adornment_tab_keeps_left_gravity_in_source_mapping() { + let chunks = projected_rich_chunks( + "X", + &[], + &[adornment(0, AdornmentPlacement::AtOffset, "\t")], + ); + let (runs, _) = build_hit_runs(&chunks); + + assert_eq!(source_to_projected(&chunks, 0), Some(0)); + assert_eq!(source_to_projected(&chunks, 1), Some(9)); + for projected in 0..8 { + assert_eq!(projected_to_source(&runs, projected), Some(0)); + } + } + #[test] fn optimistic_delete_range_covers_single_codepoints_only() { let none = Modifiers::NONE; @@ -9125,6 +9379,27 @@ mod tests { ); } + #[test] + fn minimap_columns_match_code_tab_and_unicode_widths() { + assert_eq!( + minimap_line_shapes("\tX\n1234567\tX\n界\u{301}\tX"), + vec![ + MinimapLineShape { + indent_cols: 8, + content_cols: 1, + }, + MinimapLineShape { + indent_cols: 0, + content_cols: 9, + }, + MinimapLineShape { + indent_cols: 0, + content_cols: 9, + }, + ] + ); + } + #[test] fn minimap_rects_encode_six_vertices_per_quad() { let rect = MinimapRect { @@ -11581,6 +11856,141 @@ mod tests { ); } + #[test] + fn source_tab_caret_uses_projected_leading_and_trailing_boundaries() { + let Some(mut state) = headless_or_skip(320, 240, "\tX") else { + return; + }; + let bid = BufferId::next(); + state.current_buffer_id = Some(bid); + state.reshape(); + + state.own_cursor = Some(OwnCursor { + buffer_id: bid, + byte: 0, + }); + let before = state.caret_rect().expect("caret before tab").x; + state.own_cursor = Some(OwnCursor { + buffer_id: bid, + byte: 1, + }); + let after = state.caret_rect().expect("caret after tab").x; + assert!( + after - before > 7.0 * state.mono_advance(), + "one source byte must span all eight projected spaces" + ); + } + + #[test] + fn source_tab_hit_testing_uses_projected_space_boundaries() { + let Some(mut state) = headless_or_skip(320, 240, "\tX") else { + return; + }; + state.current_buffer_id = Some(BufferId::next()); + state.reshape(); + let advance = state.mono_advance(); + let y = f64::from(TEXT_TOP + state.fm.code_line_height() / 2.0); + + assert_eq!( + state.hit_test_source_byte(f64::from(state.text_left()), y), + Some(0), + "the projected leading edge maps before the source tab" + ); + assert_eq!( + state.hit_test_source_byte(f64::from(state.text_left() + advance * 2.5), y,), + Some(1), + "a hit inside the expanded spaces maps after the source tab" + ); + } + + #[test] + fn tab_decoration_geometry_covers_spaces_split_by_soft_wrap() { + let Some(mut state) = headless_or_skip(64, 240, "\tX") else { + return; + }; + let bid = BufferId::next(); + state.current_buffer_id = Some(bid); + state.current_decorations = vec![Decoration { + range: ByteRange { start: 0, end: 1 }, + kind: DecorationKind::Selection, + }]; + state.reshape(); + assert!( + state + .buffer + .layout_runs() + .filter(|run| run.line_i == 0) + .count() + > 1, + "precondition: the eight projected spaces wrap" + ); + + let line_offsets = line_byte_offsets(&state.current_text); + let mut rects = Vec::new(); + state.collect_own_decoration_rects( + &mut rects, + &line_offsets, + state.view_range.0, + state.view_range.1, + ); + assert!( + rects.len() > 1, + "the source tab selection must fan out across wrapped visual runs" + ); + assert!( + rects.iter().all(|rect| rect.w > 0.0), + "every wrapped piece must retain horizontal geometry" + ); + } + + #[test] + fn visible_line_tab_edit_refreshes_cached_projection() { + let Some(mut state) = headless_or_skip(320, 240, "aX") else { + return; + }; + state.minimap_cache = Some(((0, 0, 0, 0), vec![1])); + let edits = state + .apply_loro_text_delta_batches(&[vec![ + loro::TextDelta::Retain { + retain: 1, + attributes: None, + }, + loro::TextDelta::Insert { + insert: "\t".to_owned(), + attributes: None, + }, + ]]) + .expect("visible edit applies"); + + assert_eq!( + edits, + vec![TextProjectionEdit { + start: 1, + old_end: 1, + inserted_len: 1, + }] + ); + assert_eq!(state.buffer.lines[0].text(), "a X"); + assert_eq!( + state.current_line_shapes[0], + MinimapLineShape { + indent_cols: 0, + content_cols: 9, + }, + "the minimap shape must refresh in the same edit transaction" + ); + assert!( + state.minimap_cache.is_none(), + "text geometry changes must invalidate cached minimap vertices" + ); + assert!( + state.line_chunk_cache[0] + .iter() + .any(|chunk| matches!(chunk.source, ChunkSource::SourceTab { start: 1 })), + "the incremental code-line cache must immediately carry tab provenance" + ); + } + /// Acceptance 11 — the `CursorByte` arm follows into a wrapped /// continuation run (the pre-existing source-line-only hole): the /// follow lands as a sub-line residual, normalized to slice-local diff --git a/pmacs-protocol/src/lib.rs b/pmacs-protocol/src/lib.rs index 70377f1..be7e124 100644 --- a/pmacs-protocol/src/lib.rs +++ b/pmacs-protocol/src/lib.rs @@ -17,6 +17,8 @@ //! - The full message envelopes: `InstanceMessage`, `FrontendEvent`, //! `GoodbyeReason`, capability structs, `PresenceUpdate`, etc. //! - The optional `CrdtOp` wire variant (feature-gated on `crdt`). +//! - [`TAB_STOP_COLUMNS`], the shared logical width used when frontends +//! project raw buffer tabs for display. //! //! What does NOT live here: //! - `crate::cell::CellGrid` and `crate::cell::diff()` (rendering @@ -40,6 +42,12 @@ pub mod ids; pub mod message; pub mod transport; +/// Logical display columns between fixed buffer-text tab stops. +/// +/// Semantic frames keep tabs as source bytes; every frontend expands them +/// only in its display projection so protocol byte ranges remain unchanged. +pub const TAB_STOP_COLUMNS: u32 = 8; + pub use cell::{ Attachment, Cell, CellCoord, CellSize, Color, DiffSpan, Glyph, Style, UnderlineStyle, }; diff --git a/src/completion.rs b/src/completion.rs index d7c4492..70bc4ab 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -32,6 +32,7 @@ use unicode_width::UnicodeWidthChar; use crate::buffer::{Buffer, BufferId}; use crate::cell::{CellCoord, CellGrid, Color, Glyph, Style}; +use crate::display_width::byte_to_column; use crate::rope::Position; use crate::view::{View, Viewport}; @@ -589,10 +590,6 @@ pub(crate) const POPUP_MAX_ROWS: u32 = 10; /// Minimum popup width in cells (glyph column + a readable label). const POPUP_MIN_WIDTH: u32 = 12; -/// Tab-stop width in display columns, matching [`crate::diag`] / -/// [`crate::text_view`]. -const TAB_WIDTH: u32 = 8; - /// Style for the currently-selected row (reverse video so it pops on /// any base palette). fn selected_style() -> Style { @@ -663,21 +660,9 @@ impl CompletionView { } } -/// Display column of `byte_end` within `line_bytes` (tab-aware, -/// UTF-8-aware). The completion twin of the diagnostic underline's -/// column resolution. +/// Display column of `byte_end` within `line_bytes`. fn display_col_for_byte(line_bytes: &[u8], byte_end: u32) -> u32 { - let end = (byte_end as usize).min(line_bytes.len()); - let text = String::from_utf8_lossy(&line_bytes[..end]); - let mut col = 0u32; - for ch in text.chars() { - if ch == '\t' { - col += TAB_WIDTH - (col % TAB_WIDTH); - } else { - col += char_display_width(ch); - } - } - col + byte_to_column(line_bytes, byte_end as usize) } /// Resolved popup rectangle, in window-relative cells. @@ -811,7 +796,7 @@ fn paint_popup_row( if col >= width { break; } - let cw = char_display_width(ch); + let cw = UnicodeWidthChar::width(ch).unwrap_or(0) as u32; if cw == 0 { continue; } @@ -879,10 +864,6 @@ impl View for CompletionView { } } -fn char_display_width(ch: char) -> u32 { - UnicodeWidthChar::width(ch).unwrap_or(0) as u32 -} - // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- diff --git a/src/diag.rs b/src/diag.rs index c7ea236..cd2909e 100644 --- a/src/diag.rs +++ b/src/diag.rs @@ -32,10 +32,10 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex}; use serde_json::Value; -use unicode_width::UnicodeWidthChar; use crate::buffer::Buffer; use crate::cell::{CellCoord, CellGrid, Color, Glyph, Style, UnderlineStyle}; +use crate::display_width::byte_range_to_columns; use crate::overlay::merge_styles; use crate::view::{View, Viewport}; @@ -388,10 +388,6 @@ pub fn make_shared_store() -> SharedDiagStore { // View // --------------------------------------------------------------------------- -/// Tab-stop width in display columns, matching -/// [`crate::text_view`] and [`crate::highlight`]. -const TAB_WIDTH: u32 = 8; - /// The RESOLVED severity color (themes arc Q#TH5): the `ui.diag.*` /// face's `fg` when a face is set with a concrete color, else the /// built-in [`DiagnosticSeverity::underline_color`]. The diag family @@ -665,8 +661,7 @@ fn paint_line_markers( } // --------------------------------------------------------------------------- -// Shared helpers (mirror highlight.rs; kept private here to avoid -// cross-module coupling on internal helpers) +// Line lookup helpers shared with the completion overlay. // --------------------------------------------------------------------------- pub(crate) fn compute_line_offsets(source: &[u8]) -> Vec { @@ -699,40 +694,10 @@ pub(crate) fn line_at_offset(line_offsets: &[u32], offset: u32) -> u32 { fn underline_cols_for_line(line_bytes: &[u8], byte_start: u32, byte_end: u32) -> (u32, u32) { if byte_end <= byte_start { let (anchor, _) = - byte_range_to_display_cols(line_bytes, byte_start as usize, byte_start as usize); + byte_range_to_columns(line_bytes, byte_start as usize, byte_start as usize); (anchor, anchor + 1) } else { - byte_range_to_display_cols(line_bytes, byte_start as usize, byte_end as usize) - } -} - -pub(crate) fn byte_range_to_display_cols( - line_bytes: &[u8], - byte_start: usize, - byte_end: usize, -) -> (u32, u32) { - let bs = byte_start.min(line_bytes.len()); - let be = byte_end.min(line_bytes.len()); - let display_to = |upto: usize| -> u32 { - let mut take = upto.min(line_bytes.len()); - while take > 0 && std::str::from_utf8(&line_bytes[..take]).is_err() { - take -= 1; - } - let s = std::str::from_utf8(&line_bytes[..take]).unwrap_or(""); - let mut col: u32 = 0; - for ch in s.chars() { - col += char_display_width(ch, col); - } - col - }; - (display_to(bs), display_to(be)) -} - -fn char_display_width(ch: char, current_col: u32) -> u32 { - if ch == '\t' { - TAB_WIDTH - (current_col % TAB_WIDTH) - } else { - UnicodeWidthChar::width(ch).unwrap_or(0) as u32 + byte_range_to_columns(line_bytes, byte_start as usize, byte_end as usize) } } diff --git a/src/display_width.rs b/src/display_width.rs new file mode 100644 index 0000000..c80670e --- /dev/null +++ b/src/display_width.rs @@ -0,0 +1,106 @@ +// display_width.rs --- Shared byte-to-display-column accounting. + +//! Allocation-free display-column helpers shared by text renderers. +//! +//! Source positions remain byte-addressed. Tabs are expanded only while +//! projecting those bytes into display columns, using the protocol-wide tab +//! 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. +#[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) +} + +/// Display width of the valid UTF-8 prefix of `bytes`. +/// +/// Invalid input is conservatively truncated at the first invalid byte. This +/// also floors a trailing partial code point without allocating or replacing +/// source bytes. +#[must_use] +pub fn valid_prefix_width(bytes: &[u8]) -> u32 { + let valid_len = match std::str::from_utf8(bytes) { + Ok(_) => bytes.len(), + Err(error) => error.valid_up_to(), + }; + let text = std::str::from_utf8(&bytes[..valid_len]).expect("valid_up_to is a UTF-8 boundary"); + text.chars().fold(0, advance_char) +} + +/// Display column at the clamped byte boundary `offset`. +/// +/// If `offset` splits a code point, the result is the column at that code +/// point's leading boundary. +#[must_use] +pub fn byte_to_column(bytes: &[u8], offset: usize) -> u32 { + valid_prefix_width(&bytes[..offset.min(bytes.len())]) +} + +/// Display-column endpoints for the half-open byte range `[start, end)`. +/// +/// Each endpoint is independently clamped and conservatively floored to a +/// complete UTF-8 boundary. +#[must_use] +pub fn byte_range_to_columns(bytes: &[u8], start: usize, end: usize) -> (u32, u32) { + (byte_to_column(bytes, start), byte_to_column(bytes, end)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn tabs_advance_at_zero_before_stop_and_on_stop() { + assert_eq!(advance_char(0, '\t'), 8); + assert_eq!(advance_char(7, '\t'), 8); + assert_eq!(advance_char(8, '\t'), 16); + } + + #[test] + fn unicode_widths_include_wide_and_zero_width_characters() { + assert_eq!(advance_char(3, '中'), 5); + assert_eq!(advance_char(3, '\u{301}'), 3); + assert_eq!(valid_prefix_width("a中\u{301}b".as_bytes()), 4); + } + + #[test] + fn byte_columns_clamp_and_floor_partial_or_invalid_utf8() { + let text = "a中b".as_bytes(); + assert_eq!(byte_to_column(text, 0), 0); + assert_eq!(byte_to_column(text, 1), 1); + assert_eq!(byte_to_column(text, 2), 1); + assert_eq!(byte_to_column(text, 3), 1); + assert_eq!(byte_to_column(text, 4), 3); + assert_eq!(byte_to_column(text, usize::MAX), 4); + + assert_eq!(valid_prefix_width(b"ab\xffcd"), 2); + assert_eq!(byte_to_column(b"ab\xe2\x82", 4), 2); + } + + #[test] + fn byte_ranges_map_half_open_endpoints_with_tab_expansion() { + let text = b"a\tb"; + assert_eq!(byte_range_to_columns(text, 0, 1), (0, 1)); + assert_eq!(byte_range_to_columns(text, 1, 2), (1, 8)); + assert_eq!(byte_range_to_columns(text, 2, 3), (8, 9)); + assert_eq!(byte_range_to_columns(text, 99, 99), (9, 9)); + } + + #[test] + fn range_boundaries_inside_codepoints_are_floored() { + let text = "a中b".as_bytes(); + assert_eq!(byte_range_to_columns(text, 2, 3), (1, 1)); + assert_eq!(byte_range_to_columns(text, 2, 4), (1, 3)); + } +} diff --git a/src/highlight.rs b/src/highlight.rs index 18740ca..30d24e0 100644 --- a/src/highlight.rs +++ b/src/highlight.rs @@ -33,10 +33,9 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex}; -use unicode_width::UnicodeWidthChar; - use crate::buffer::Buffer; use crate::cell::{CellCoord, CellGrid, Color, Style, UnderlineStyle}; +use crate::display_width::byte_range_to_columns; use crate::lsp::SharedLspManager; use crate::overlay::merge_styles; use crate::syntax::{HighlightSpan, ParseTreeBundle, ParseViewHandle, compute_highlight_spans_for}; @@ -316,10 +315,6 @@ impl HighlightCache { } } -/// Tab-stop width in display columns (must match -/// [`crate::text_view`]; both views write into the same cell grid). -const TAB_WIDTH: u32 = 8; - /// View that renders syntax highlighting from a tree-sitter parse /// tree, including its injection layers. Composes over /// [`crate::text_view::TextView`] per the M2.9 view-composition @@ -482,7 +477,7 @@ impl View for SyntaxHighlightView { let byte_col_start = (s_start - line_start) as usize; let byte_col_end = (s_end - line_start) as usize; let (start_col, end_col) = - byte_range_to_display_cols(line_bytes, byte_col_start, byte_col_end); + byte_range_to_columns(line_bytes, byte_col_start, byte_col_end); if end_col <= start_col { continue; } @@ -526,40 +521,6 @@ fn line_at_offset(line_offsets: &[u32], offset: u32) -> u32 { } } -/// Convert a half-open byte-column range `[byte_start, byte_end)` -/// inside `line_bytes` to a display-column range. UTF-8 aware; tabs -/// expand to the next [`TAB_WIDTH`]-aligned column. Bytes that don't -/// form complete codepoints (because the byte range falls inside a -/// multi-byte char) are skipped, matching -/// [`crate::text_view::TextView::pos_to_display`]'s conservative -/// rounding. -fn byte_range_to_display_cols(line_bytes: &[u8], byte_start: usize, byte_end: usize) -> (u32, u32) { - let bs = byte_start.min(line_bytes.len()); - let be = byte_end.min(line_bytes.len()); - let display_to = |upto: usize| -> u32 { - // Drop trailing bytes that don't form complete codepoints. - let mut take = upto.min(line_bytes.len()); - while take > 0 && std::str::from_utf8(&line_bytes[..take]).is_err() { - take -= 1; - } - let s = std::str::from_utf8(&line_bytes[..take]).unwrap_or(""); - let mut col: u32 = 0; - for ch in s.chars() { - col += char_display_width(ch, col); - } - col - }; - (display_to(bs), display_to(be)) -} - -fn char_display_width(ch: char, current_col: u32) -> u32 { - if ch == '\t' { - TAB_WIDTH - (current_col % TAB_WIDTH) - } else { - UnicodeWidthChar::width(ch).unwrap_or(0) as u32 - } -} - // --------------------------------------------------------------------------- // Style equality helper // --------------------------------------------------------------------------- @@ -737,7 +698,7 @@ impl View for LspStyleView { if end_b <= start_b { continue; } - let (start_col, end_col) = byte_range_to_display_cols(line_bytes, start_b, end_b); + let (start_col, end_col) = byte_range_to_columns(line_bytes, start_b, end_b); if end_col <= start_col { continue; } @@ -944,9 +905,9 @@ mod tests { fn byte_range_display_cols_ascii_round_trips() { let line = b"hello world"; // "hello" → cols 0..5 - assert_eq!(byte_range_to_display_cols(line, 0, 5), (0, 5)); + assert_eq!(byte_range_to_columns(line, 0, 5), (0, 5)); // "world" → cols 6..11 - assert_eq!(byte_range_to_display_cols(line, 6, 11), (6, 11)); + assert_eq!(byte_range_to_columns(line, 6, 11), (6, 11)); } #[test] @@ -954,15 +915,15 @@ mod tests { let line = b"\tx"; // The full line: tab (0..8) + 'x' (8..9). Byte cols 0..2 // map to display cols 0..9. - assert_eq!(byte_range_to_display_cols(line, 0, 2), (0, 9)); + assert_eq!(byte_range_to_columns(line, 0, 2), (0, 9)); // Just the tab. - assert_eq!(byte_range_to_display_cols(line, 0, 1), (0, 8)); + assert_eq!(byte_range_to_columns(line, 0, 1), (0, 8)); } #[test] fn byte_range_display_cols_clamps_past_end() { let line = b"hi"; - assert_eq!(byte_range_to_display_cols(line, 0, 999), (0, 2)); + assert_eq!(byte_range_to_columns(line, 0, 999), (0, 2)); } #[test] diff --git a/src/lib.rs b/src/lib.rs index 4f283f5..bad3e6f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -70,6 +70,7 @@ pub mod daemon_attach; pub mod definition; pub mod desktop; pub mod diag; +pub mod display_width; pub mod document_highlight; pub mod editor; pub mod editor_core; diff --git a/src/overlay.rs b/src/overlay.rs index edc3b73..5d41e28 100644 --- a/src/overlay.rs +++ b/src/overlay.rs @@ -43,10 +43,9 @@ use std::sync::{Arc, Mutex}; -use unicode_width::UnicodeWidthChar; - use crate::buffer::Buffer; use crate::cell::{Cell, CellCoord, CellGrid, Style}; +use crate::display_width::byte_range_to_columns; use crate::rope::Edit; use crate::view::{View, Viewport}; @@ -366,30 +365,6 @@ fn line_end(buf: &Buffer, line_offsets: &[u64], line: usize) -> u64 { } } -fn display_col_for_range(buf: &Buffer, start: u64, end: u64) -> u32 { - if end <= start { - return 0; - } - let mut bytes = vec![0u8; (end - start) as usize]; - buf.snapshot_rope().slice(start, end, &mut bytes); - while !bytes.is_empty() && std::str::from_utf8(&bytes).is_err() { - bytes.pop(); - } - let Ok(s) = std::str::from_utf8(&bytes) else { - return 0; - }; - let mut col = 0; - for ch in s.chars() { - let width = if ch == '\t' { - 8 - (col % 8) - } else { - UnicodeWidthChar::width(ch).unwrap_or(0) as u32 - }; - col += width; - } - col -} - fn render_buffer_style_span( buf: &Buffer, line_offsets: &[u64], @@ -418,8 +393,14 @@ fn render_buffer_style_span( if style_start >= style_end { continue; } - let start_col = display_col_for_range(buf, line_start, style_start); - let end_col = display_col_for_range(buf, line_start, style_end); + let mut line_prefix = vec![0; (style_end - line_start) as usize]; + buf.snapshot_rope() + .slice(line_start, style_end, &mut line_prefix); + let (start_col, end_col) = byte_range_to_columns( + &line_prefix, + (style_start - line_start) as usize, + line_prefix.len(), + ); let start_col = start_col.min(viewport.cell_size.cols); let end_col = end_col.min(viewport.cell_size.cols); for col in start_col..end_col { diff --git a/src/search.rs b/src/search.rs index 7ff49c1..0ff1211 100644 --- a/src/search.rs +++ b/src/search.rs @@ -21,6 +21,7 @@ use std::sync::{Arc, Mutex}; use pmacs_protocol::ByteRange; use crate::buffer::BufferId; +use crate::display_width::byte_range_to_columns; /// One buffer's search state: the resolved query, its matches (byte /// ranges, ascending and non-overlapping), and the active index. @@ -516,7 +517,7 @@ impl View for SearchView { let within_start = (paint_start - line_start) as usize; let within_end = (paint_end - line_start) as usize; let (start_col, end_col) = - crate::diag::byte_range_to_display_cols(line_bytes, within_start, within_end); + byte_range_to_columns(line_bytes, within_start, within_end); if end_col <= start_col { continue; } diff --git a/src/text_view.rs b/src/text_view.rs index 7f22daa..b94e03d 100644 --- a/src/text_view.rs +++ b/src/text_view.rs @@ -19,10 +19,9 @@ //! Main thread only. The view is held inside a [`Buffer`], which is itself //! main-only. -use unicode_width::UnicodeWidthChar; - use crate::buffer::{Buffer, BufferError}; use crate::cell::{Cell, CellCoord, CellGrid, Glyph, Style}; +use crate::display_width::{advance_char, valid_prefix_width}; use crate::rope::{Edit, Position}; use crate::view::{DisplayCoord, View, Viewport}; @@ -30,28 +29,10 @@ use crate::view::{DisplayCoord, View, Viewport}; // Tuning // --------------------------------------------------------------------------- -/// Tab stop width in display columns. A `\t` advances to the next column -/// that is a multiple of this value. -const TAB_WIDTH: u32 = 8; - /// Line-prefix lengths up to this many bytes are decoded on the stack in /// [`TextView::pos_to_display`]; longer prefixes fall back to a heap buffer. const STACK_CAP: usize = 256; -/// Display width of `ch` when drawn starting at column `current_col`. -/// -/// Tabs expand to the next [`TAB_WIDTH`]-aligned column, so they need the -/// running column to compute width. Everything else delegates to -/// [`UnicodeWidthChar`]: control characters return 0 (skipped by the -/// caller), printable characters return 1, wide characters return 2. -fn char_display_width(ch: char, current_col: u32) -> u32 { - if ch == '\t' { - TAB_WIDTH - (current_col % TAB_WIDTH) - } else { - UnicodeWidthChar::width(ch).unwrap_or(0) as u32 - } -} - // --------------------------------------------------------------------------- // TextView // --------------------------------------------------------------------------- @@ -196,19 +177,7 @@ impl View for TextView { &mut heap_buf }; buf.snapshot_rope().slice(line_start, pos, bytes); - // If `pos` fell inside a multi-byte codepoint, keep only the bytes up to - // the last complete codepoint. `valid_up_to()` gives that boundary in - // one step, replacing the old pop-one-byte-and-revalidate loop. (Only - // trailing bytes can be invalid here, since the slice is a prefix of - // valid UTF-8 cut at `pos`.) - let s = match std::str::from_utf8(bytes) { - Ok(valid) => valid, - Err(e) => std::str::from_utf8(&bytes[..e.valid_up_to()]).unwrap(), - }; - let mut col: u32 = 0; - for ch in s.chars() { - col += char_display_width(ch, col); - } + let col = valid_prefix_width(bytes); Some(DisplayCoord::new(row_idx as u32, col)) } @@ -228,7 +197,7 @@ impl View for TextView { walked_bytes = byte_idx; return Some(line_start + walked_bytes as u64); } - walked_cols += char_display_width(ch, walked_cols); + walked_cols = advance_char(walked_cols, ch); walked_bytes = byte_idx + ch.len_utf8(); } // Past the line's last codepoint: clamp to the line's visible end. @@ -265,8 +234,8 @@ impl View for TextView { break; } if ch == '\t' { - // Expand to the next TAB_WIDTH-aligned column with spaces. - let pad = char_display_width(ch, col); + // Expand to the next protocol-wide tab stop with spaces. + let pad = advance_char(col, ch) - col; for _ in 0..pad { if col >= max_cols { break; @@ -279,7 +248,7 @@ impl View for TextView { } continue; } - let width = UnicodeWidthChar::width(ch).unwrap_or(0) as u32; + let width = advance_char(col, ch) - col; if width == 0 { // Combining mark or other zero-width control: M1.5 // skips; M2+ will attach to the previous cell as @@ -531,7 +500,7 @@ mod tests { #[test] fn tab_aligned_input_advances_full_width() { - // 8 chars then tab: tab pads from col 8 to col 16 (a full TAB_WIDTH). + // 8 chars then tab: the protocol tab stop advances col 8 to col 16. let (buf, view) = attached(b"01234567\tx"); assert_eq!(view.pos_to_display(&buf, 8), Some(DisplayCoord::new(0, 8))); assert_eq!(view.pos_to_display(&buf, 9), Some(DisplayCoord::new(0, 16))); diff --git a/tests/tab_width_acceptance.rs b/tests/tab_width_acceptance.rs new file mode 100644 index 0000000..ba15f52 --- /dev/null +++ b/tests/tab_width_acceptance.rs @@ -0,0 +1,87 @@ +//! Cross-frontend tab-stop acceptance for core/TUI rendering. + +use std::sync::{Arc, Mutex}; + +use pmacs::buffer::{Buffer, BufferId}; +use pmacs::cell::{Cell, CellCoord, CellGrid, CellSize, Glyph, Style}; +use pmacs::overlay::{BufferStyleOverlay, BufferStyleSpan, SharedBufferStyleSpans}; +use pmacs::text_view::TextView; +use pmacs::view::{DisplayCoord, View, Viewport}; + +fn viewport(rows: u32, cols: u32, buffer_end: u64) -> Viewport { + Viewport { + buffer_start: 0, + buffer_end, + cell_origin: CellCoord::new(0, 0), + cell_size: CellSize::new(rows, cols), + gutter_w: 0, + } +} + +fn render_text(buf: &Buffer, rows: u32, cols: u32) -> Vec { + let mut cells = vec![Cell::default(); (rows * cols) as usize]; + let mut grid = CellGrid { + cells: &mut cells, + stride: cols, + size: CellSize::new(rows, cols), + }; + TextView::new(buf).render(buf, viewport(rows, cols, buf.len()), &mut grid); + cells +} + +#[test] +fn plain_text_projects_tabs_without_changing_source_bytes() { + let source = b"\tx\n1234567\ty\n12345678\tz"; + let buf = Buffer::from_bytes(BufferId::next(), "tabs", source); + let cells = render_text(&buf, 3, 20); + let view = TextView::new(&buf); + assert_eq!(view.pos_to_display(&buf, 1), Some(DisplayCoord::new(0, 8))); + assert_eq!(view.pos_to_display(&buf, 11), Some(DisplayCoord::new(1, 8))); + assert_eq!( + view.pos_to_display(&buf, 22), + Some(DisplayCoord::new(2, 16)) + ); + + for cell in cells.iter().take(8) { + assert_eq!(cell.glyph, Glyph::Char(' ')); + } + assert_eq!(cells[8].glyph, Glyph::Char('x')); + assert_eq!(cells[20 + 8].glyph, Glyph::Char('y')); + assert_eq!(cells[40 + 16].glyph, Glyph::Char('z')); + + let mut retained = vec![0; buf.len() as usize]; + buf.snapshot_rope().slice(0, buf.len(), &mut retained); + assert_eq!(retained, source, "rendering must not replace source tabs"); +} + +#[test] +fn buffer_style_overlay_covers_the_same_expanded_tab_columns_as_plain_text() { + let source = b"a\tb"; + let buf = Buffer::from_bytes(BufferId::next(), "styled-tab", source); + let mut cells = render_text(&buf, 1, 12); + let spans: SharedBufferStyleSpans = Arc::new(Mutex::new(vec![BufferStyleSpan { + start: 1, + end: 2, + style: Style { + bold: true, + ..Style::default() + }, + }])); + let mut overlay = BufferStyleOverlay::new(spans); + let mut grid = CellGrid { + cells: &mut cells, + stride: 12, + size: CellSize::new(1, 12), + }; + overlay.render(&buf, viewport(1, 12, buf.len()), &mut grid); + + assert_eq!(grid.get(CellCoord::new(0, 0)).glyph, Glyph::Char('a')); + assert!(!grid.get(CellCoord::new(0, 0)).style.bold); + for col in 1..8 { + let cell = grid.get(CellCoord::new(0, col)); + assert_eq!(cell.glyph, Glyph::Char(' '), "expanded tab column {col}"); + assert!(cell.style.bold, "overlay missed expanded tab column {col}"); + } + assert_eq!(grid.get(CellCoord::new(0, 8)).glyph, Glyph::Char('b')); + assert!(!grid.get(CellCoord::new(0, 8)).style.bold); +}