From a79ab857f9c0f47813890ddd24632ea7f9c2990c Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 22:18:07 -0700 Subject: [PATCH 1/3] feat(text-editing): BiDi split caret secondary indicator (E3 deferral) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At a bidirectional direction boundary the caret now paints TWO marks — the primary full-height bar plus a shorter SECONDARY indicator at the other run's edge — so the user can tell which direction the next typed character flows (editing-and-ime §§4.1/5). E3 shipped only the primary. cosmic-text 0.19's cursor_glyph is affinity-blind, so a single cursor_position cannot surface the second edge. New secondary_caret_rect_for walks layout_runs: at the caret's line it finds the before glyph (end == index) and after glyph (start == index); emits the secondary only when both exist and their bidi levels differ, at the BEFORE glyph's logical-end visual edge (RTL: x; LTR: x+w). Soft-wrapped logical lines emit multiple runs sharing line_i, so the scan CONTINUEs past a non-owning wrap segment and only concludes None after exhausting all runs (impl-review major). Forced by Bevy's 15-tuple extract query cap (PlaceholderBuffer is the 15th slot), the secondary rides CaretVisual as an Option field rather than a new component — reusing Changed/RemovedComponents as its damage + clear triggers, zero new query surface. extract stamps a second solid glyph (same atlas entry/color/clip, no new insert). The blink writer (visual.rs) touches only .visible — preserved, carries secondary through. Tests assert the EXACT data-derived before-glyph edge (equality with primary.x allowed — coincident visual pen-x is two distinct logical insertion points; no spurious inequality). Headless text_caret_geometry 11/11 (incl. boundary + wrapped-line + no-boundary cases); GPU split-caret golden passes on RX 6700 XT (primary+secondary at a mixed-BiDi boundary). Spec §§4.1/5/13 + follow-ups.md LANDED. First text-track slice; built on the 0.19-rc base. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- crates/buiy_core/src/text/components.rs | 19 +- crates/buiy_core/src/text/edit/caret.rs | 106 ++++++- crates/buiy_core/src/text/edit/mod.rs | 5 +- crates/buiy_core/src/text/extract.rs | 19 +- .../buiy_core/tests/text_caret_blink_focus.rs | 3 + crates/buiy_core/tests/text_caret_geometry.rs | 279 +++++++++++++++++- .../buiy_core/tests/text_caret_selection.rs | 5 + .../tests/text_caret_selection_e3_gpu.rs | 148 ++++++++++ .../buiy_core/tests/text_effect_group_gpu.rs | 1 + .../buiy_core/tests/text_golden_suite_gpu.rs | 1 + .../tests/text_selection_caret_gpu.rs | 1 + docs/plans/follow-ups.md | 80 +++-- .../editing-and-ime.md | 51 +++- 13 files changed, 665 insertions(+), 53 deletions(-) diff --git a/crates/buiy_core/src/text/components.rs b/crates/buiy_core/src/text/components.rs index 4bba151..bf7e78b 100644 --- a/crates/buiy_core/src/text/components.rs +++ b/crates/buiy_core/src/text/components.rs @@ -388,15 +388,25 @@ pub struct CaretVisual { pub visible: bool, /// Content-box-local caret rect, logical px, unsnapped. pub rect: bevy::math::Rect, + /// The SECONDARY split-caret indicator rect (§§ 4.1, 5), content-box-local, + /// logical px, unsnapped. `Some` only when the caret sits on a bidirectional + /// DIRECTION BOUNDARY — the BEFORE glyph's logical-end visual edge, a shorter + /// mark that tells the user which direction the next typed char flows. + /// `None` for a normal caret (no second insertion point). Rides this + /// component (not a standalone one) because the extract producer's + /// query/trigger/removal params are at Bevy's 15-tuple cap (extract.rs § 6.1), + /// so it reuses `Changed` as its damage trigger. + pub secondary: Option, } impl Default for CaretVisual { /// Visible (matches the t=0 blink phase and editing § 10's - /// "caret becomes visible" on focus gain), zero rect. + /// "caret becomes visible" on focus gain), zero rect, no secondary. fn default() -> Self { Self { visible: true, rect: bevy::math::Rect::default(), + secondary: None, } } } @@ -691,4 +701,11 @@ mod tests { buffer.invalidate_intrinsics(); assert_eq!(buffer.intrinsics(), None); } + + /// §§ 4.1, 5: a fresh caret has no split-caret secondary indicator — the + /// secondary lands only when the writer finds a bidi direction boundary. + #[test] + fn caret_visual_default_has_no_secondary() { + assert_eq!(CaretVisual::default().secondary, None); + } } diff --git a/crates/buiy_core/src/text/edit/caret.rs b/crates/buiy_core/src/text/edit/caret.rs index 557145f..c6748f0 100644 --- a/crates/buiy_core/src/text/edit/caret.rs +++ b/crates/buiy_core/src/text/edit/caret.rs @@ -6,9 +6,14 @@ //! Disabled editor's selection OUT into `TextSelection` + the T7 paint seats //! (`CaretVisual`, `SelectionVisual`), resets the blink phase on a caret/ //! selection transition, and emits `CaretMoved` / `SelectionChanged` on -//! transition only. E3 paints the SINGLE primary caret; the BiDi split caret is -//! a named deferral (cosmic 0.19 exposes no dual-caret API — see the plan's -//! Architecture + the follow-up filed in Task 7). +//! transition only. The PRIMARY caret is `cursor_position` from the run that +//! owns the cursor. The SECONDARY indicator (the BiDi split caret, §§ 4.1, 5) +//! now lands as `secondary_caret_rect_for`: at a direction boundary it is the +//! BEFORE glyph's (`end == index`) logical-end visual edge — the position +//! cosmic 0.19's single affinity-blind `cursor_glyph` cannot surface (it +//! resolves `index == glyph.start` BEFORE `index == glyph.end`, buffer.rs: +//! 151-174, so its one `cursor_position` only ever reports the start-glyph +//! edge). Non-boundary carets have no secondary (`None`). //! //! It reads the editor through `TextEditState`'s facade accessors //! (`mirror_selection` / `with_buffer`) and names only the pure-data cosmic @@ -39,14 +44,22 @@ pub struct SelectionChanged(pub Entity, pub TextSelection); /// `caret_stamp_rect`). A thin vertical bar; 1.0 logical is the convention. const CARET_W: f32 = 1.0; +/// The secondary indicator's height as a fraction of the line box (§ 4.1: the +/// split caret's SECONDARY mark is a shorter indicator, not a second full-height +/// bar — it tells the user which direction the next typed char flows without +/// reading as a second insertion point). A v1 visual choice; tests assert only +/// that it is `<=` the primary height, so a later tweak doesn't churn them. +const SECONDARY_CARET_H_FRAC: f32 = 0.5; + /// The caret rect for `caret` in CONTENT-BOX-LOCAL coords (logical px), from the /// run that owns the cursor: `cursor_position` gives x, `line_top`/`line_height` /// give y/height (§ 4.1). Returns `None` if no run owns the cursor (degenerate / /// not-yet-shaped buffer). /// -/// `pub(crate)` so `ime.rs`'s `write_ime_window` reuses it for the caret rect -/// the IME popup anchors to (`Window.ime_position`, editing-and-ime § 6.3). -pub(crate) fn caret_rect_for(buffer: &cosmic_text::Buffer, caret: &Cursor) -> Option { +/// `pub` so `ime.rs`'s `write_ime_window` reuses it for the caret rect the IME +/// popup anchors to (`Window.ime_position`, editing-and-ime § 6.3), and so the +/// E3 caret-geometry tests can pin it against a shaped buffer directly. +pub fn caret_rect_for(buffer: &cosmic_text::Buffer, caret: &Cursor) -> Option { for run in buffer.layout_runs() { if let Some(x) = run.cursor_position(caret) { return Some(Rect::new( @@ -60,6 +73,78 @@ pub(crate) fn caret_rect_for(buffer: &cosmic_text::Buffer, caret: &Cursor) -> Op None } +/// The SECONDARY split-caret rect (§§ 4.1, 5) in CONTENT-BOX-LOCAL coords +/// (logical px), or `None` when `caret` does not sit on a bidirectional +/// DIRECTION BOUNDARY. +/// +/// At a direction boundary two glyphs abut the caret's byte index: a BEFORE +/// glyph (`end == caret.index`) and an AFTER glyph (`start == caret.index`). +/// cosmic 0.19's `cursor_glyph` (buffer.rs:151-174) resolves the AFTER glyph +/// first (`index == glyph.start` before `index == glyph.end`), so the PRIMARY +/// caret (`caret_rect_for`) already lands at the AFTER glyph's edge. The +/// SECONDARY is the OTHER abutting glyph — the BEFORE glyph — at its +/// LOGICAL-END visual edge, per cosmic's own direction convention (buffer.rs: +/// 120-142, mirrored by `cursor_from_glyph_right` buffer.rs:191-197): an LTR +/// glyph's logical end is its right edge (`x + w`), an RTL glyph's is its left +/// edge (`x`). This is exactly the position cosmic's single affinity-blind +/// `cursor_position` cannot surface — hence a dedicated walk here. +/// +/// Returns `None` unless BOTH glyphs exist within ONE `line_i`-matching run AND +/// they have OPPOSITE directions (`before.level.is_rtl() != after.level.is_rtl()`). +/// A run extremity (only one abutting glyph) or a same-direction join is a normal +/// caret with no second insertion point. +/// +/// A logical line that SOFT-WRAPS emits MULTIPLE `LayoutRun`s sharing the same +/// `line_i` (cosmic 0.19 `LayoutRunIter`: one run per wrapped `layout_line`, all +/// with that line's `line_i`; each run's `glyphs` is the line-relative sub-slice +/// for its wrap segment). The caret's index may live on a CONTINUATION segment, +/// whose glyphs are in a LATER run — so a `line_i`-matching run that holds +/// NEITHER an abutting before NOR after glyph at the index is the WRONG wrap +/// segment, not a verdict. Mirror `caret_rect_for`'s all-runs scan: CONTINUE +/// past such a run; only conclude `None` after exhausting every run. (The +/// primary path gets this for free — `run.cursor_position` returns `None` for a +/// non-owning run and the loop continues; this walk must do the same explicitly.) +/// +/// `pub` so `write_caret_and_selection` populates `CaretVisual.secondary` from +/// it (a second solid-stamp instance, CPU geometry only — no new GPU), and so +/// the E3 caret-geometry tests can pin it against a shaped buffer directly. +pub fn secondary_caret_rect_for(buffer: &cosmic_text::Buffer, caret: &Cursor) -> Option { + for run in buffer.layout_runs() { + if run.line_i != caret.line { + continue; + } + // The two abutting glyphs at the caret's byte index. A cluster that + // STRADDLES the index (`start < index < end`) is not a boundary — the + // caret is mid-cluster, a single insertion point. + let before = run.glyphs.iter().find(|g| g.end == caret.index); + let after = run.glyphs.iter().find(|g| g.start == caret.index); + let (Some(before), Some(after)) = (before, after) else { + // Neither/only-one abutting glyph: this is the WRONG wrap segment of + // a soft-wrapped logical line (the owning run is later), or a genuine + // run extremity. Either way, keep scanning — a LATER `line_i`- + // matching run may own both glyphs. Only after exhausting every run + // is `None` the verdict. + continue; + }; + if before.level.is_rtl() == after.level.is_rtl() { + return None; // same direction → no split (this run owns the index) + } + // The BEFORE glyph's logical-end visual edge (cosmic's convention). + let sec_x = if before.level.is_rtl() { + before.x + } else { + before.x + before.w + }; + return Some(Rect::new( + sec_x, + run.line_top, + sec_x + CARET_W, + run.line_top + run.line_height * SECONDARY_CARET_H_FRAC, + )); + } + None +} + /// Render-prep: drive every focused, non-`Disabled` editor's caret + selection /// paint seats from the editor state, emit transition Messages, reset blink. #[allow(clippy::type_complexity)] @@ -117,8 +202,14 @@ pub fn write_caret_and_selection( let Some(new_rect) = state.with_buffer(|buffer| caret_rect_for(buffer, &caret)) else { continue; }; + // The SECONDARY split-caret indicator (§§ 4.1, 5): `Some` only when the + // caret sits on a bidirectional direction boundary, else `None`. + let secondary = state.with_buffer(|buffer| secondary_caret_rect_for(buffer, &caret)); - let caret_changed = prev_caret.map(|c| c.rect) != Some(new_rect); + // Compare the PAIR: a boundary crossing that changes only the secondary + // (same primary x) must still re-emit, else a stale secondary paints. + let caret_changed = + prev_caret.map(|c| (c.rect, c.secondary)) != Some((new_rect, secondary)); if caret_changed { // Preserve the current visibility (the blink writer owns it); a NEW // caret defaults visible. @@ -126,6 +217,7 @@ pub fn write_caret_and_selection( commands.entity(entity).insert(CaretVisual { visible, rect: new_rect, + secondary, }); } diff --git a/crates/buiy_core/src/text/edit/mod.rs b/crates/buiy_core/src/text/edit/mod.rs index fd62026..efde881 100644 --- a/crates/buiy_core/src/text/edit/mod.rs +++ b/crates/buiy_core/src/text/edit/mod.rs @@ -27,7 +27,10 @@ mod undo; pub use access::{ TextBufferAccess, TextBufferAccessItem, TextBufferAccessReadOnly, TextBufferAccessReadOnlyItem, }; -pub use caret::{CaretMoved, SelectionChanged, write_caret_and_selection}; +pub use caret::{ + CaretMoved, SelectionChanged, caret_rect_for, secondary_caret_rect_for, + write_caret_and_selection, +}; pub use clipboard::{ArboardClipboard, Clipboard, ClipboardProvider, MemClipboard}; pub use command::EditCommand; pub use ime::{ diff --git a/crates/buiy_core/src/text/extract.rs b/crates/buiy_core/src/text/extract.rs index 865f72e..8b35d4c 100644 --- a/crates/buiy_core/src/text/extract.rs +++ b/crates/buiy_core/src/text/extract.rs @@ -753,7 +753,10 @@ pub fn extract_buiy_glyphs( // T7 § 6.1 — seat 6: the caret paints last, over glyphs and // line-through, as a solid-stamp instance (pre-phase decision 2). // Painted under Block too (chrome, not ink — decision 9: browsers - // keep the caret in a focused field whose font is loading). + // keep the caret in a focused field whose font is loading). At a + // bidirectional direction boundary an OPTIONAL second (secondary- + // indicator) stamp follows the primary (§§ 4.1, 5) — same + // entry/color/clip/page, CPU geometry only, no new atlas insert. if let Some(cv) = caret_visual && cv.visible { @@ -779,6 +782,20 @@ pub fn extract_buiy_glyphs( clip, page: entry.page as u32, }); + // §§ 4.1, 5: the SECONDARY split-caret indicator — a second + // solid stamp at the boundary's before-glyph logical-end edge, + // present only at a bidi direction boundary. Reuse the SAME + // entry/color/clip/page; the stamp key is already queued below + // (do NOT push it twice). + if let Some(sec) = cv.secondary { + new_glyphs.push(GlyphAlphaInstance { + rect: caret_stamp_rect(origin, sec, scale_factor), + uv: stamp_uv(&entry), + color: linear_color(color), + clip, + page: entry.page as u32, + }); + } // § 6.3: the stamp key joins the un-gated touch pass — a // retained caret idling past eviction_grace must not lose // its cell. diff --git a/crates/buiy_core/tests/text_caret_blink_focus.rs b/crates/buiy_core/tests/text_caret_blink_focus.rs index c7ce0b4..a10fdee 100644 --- a/crates/buiy_core/tests/text_caret_blink_focus.rs +++ b/crates/buiy_core/tests/text_caret_blink_focus.rs @@ -36,6 +36,7 @@ fn unfocused_editor_caret_is_forced_hidden() { CaretVisual { visible: true, rect: Rect::new(0.0, 0.0, 1.0, 16.0), + secondary: None, }, )) .id(); @@ -59,6 +60,7 @@ fn focused_editor_caret_blinks_visible_at_phase_zero() { CaretVisual { visible: false, rect: Rect::new(0.0, 0.0, 1.0, 16.0), + secondary: None, }, )) .id(); @@ -84,6 +86,7 @@ fn bare_caret_without_editor_keeps_global_phase() { CaretVisual { visible: false, rect: Rect::new(0.0, 0.0, 1.0, 16.0), + secondary: None, }, )) .id(); diff --git a/crates/buiy_core/tests/text_caret_geometry.rs b/crates/buiy_core/tests/text_caret_geometry.rs index 532fc51..46a1c3f 100644 --- a/crates/buiy_core/tests/text_caret_geometry.rs +++ b/crates/buiy_core/tests/text_caret_geometry.rs @@ -1,7 +1,10 @@ //! E3 headless — caret/selection geometry, the per-entity blink phase, the -//! split caret, and the SelectionChanged/CaretMoved Messages (editing-and-ime -//! §§ 4.1, 4.3, 5, 11). No GPU: the geometry is pure CPU math over a shaped -//! buffer; pixels are the additive `_gpu` golden. +//! split caret (PRIMARY + SECONDARY indicator), and the SelectionChanged/ +//! CaretMoved Messages (editing-and-ime §§ 4.1, 4.3, 5, 11). No GPU: the +//! geometry is pure CPU math over a shaped buffer; pixels are the additive +//! `_gpu` golden. + +mod support; use std::time::Duration; @@ -10,11 +13,17 @@ use bevy::input::keyboard::{Key, KeyboardInput}; use bevy::input::{ButtonInput, ButtonState}; use bevy::prelude::*; use buiy_core::layout::{LayoutPlugin, Style}; -use buiy_core::text::edit::{CaretBlink, CaretMoved, SelectionChanged, TextEditState}; -use buiy_core::text::{BuiyTextPlugin, CaretVisual, SelectionVisual, Text}; +use buiy_core::text::edit::{ + CaretBlink, CaretMoved, SelectionChanged, TextEditState, caret_rect_for, + secondary_caret_rect_for, +}; +use buiy_core::text::{ + BuiyTextPlugin, CaretVisual, FamilyEntry, FontFamily, FontStack, SelectionVisual, Text, + TextBuffer, +}; use buiy_core::theme::UserPreferences; use buiy_core::{CorePlugin, FocusedEntity, Node}; -use cosmic_text::Metrics; +use cosmic_text::{Cursor, Metrics}; #[test] fn caret_blink_origin_defaults_to_zero_and_resets() { @@ -357,3 +366,261 @@ fn reduced_motion_keeps_the_caret_steady() { ); } } + +// --- secondary split-caret geometry (editing-and-ime §§ 4.1, 5) ------------- +// +// These pin the PURE CPU math directly: spawn a `Text` entity, settle so the +// display `TextBuffer.buffer` is SHAPED (the `text_decoration.rs` idiom), then +// call `caret_rect_for` / `secondary_caret_rect_for` on that buffer. No editor +// or focus needed — the geometry is a function of the shaped glyph array. + +/// A headless app that runs the full text pipeline so a spawned `Text` entity +/// gets a SHAPED `TextBuffer`. ThemePlugin supplies the default font-matching +/// resources the resolver needs (the `text_decoration.rs` `text_app` idiom). +fn geometry_app() -> App { + let mut app = App::new(); + app.add_plugins(MinimalPlugins) + .add_plugins(buiy_core::theme::ThemePlugin) + .add_plugins(CorePlugin) + .add_plugins(LayoutPlugin) + .add_plugins(BuiyTextPlugin::default()); + app +} + +fn settle(app: &mut App) { + for _ in 0..3 { + app.update(); + } +} + +#[test] +fn pure_ltr_editor_caret_has_no_secondary() { + // End-to-end through the focused-editor writer: typing ASCII yields a caret + // with NO secondary (no direction boundary) — the follow-up's explicit + // negative, proving the writer threads `None` through `CaretVisual.secondary`. + let (mut app, window) = caret_app(); + let editor = spawn_editor(&mut app, ""); + app.update(); + app.world_mut().resource_mut::().0 = Some(editor); + type_char(&mut app, window, "h"); + type_char(&mut app, window, "i"); + app.update(); // OQ#1: geometry comes current at N+1 + + let caret = app + .world() + .get::(editor) + .expect("a focused editor has a caret"); + assert!( + caret.secondary.is_none(), + "a pure-LTR editor caret carries no secondary indicator: {:?}", + caret.secondary + ); +} + +#[test] +fn secondary_caret_rect_for_is_none_without_a_direction_boundary() { + // A pure-LTR line: no direction boundary anywhere, so every caret position + // has a primary but NO secondary indicator. + let mut app = geometry_app(); + app.update(); + let e = app + .world_mut() + .spawn(( + Node, + Style::default().width_px(400.0).height_px(40.0), + Text("hello".into()), + )) + .id(); + settle(&mut app); + + let buffer = &app.world().get::(e).expect("TextBuffer").buffer; + for i in [0usize, 3, 5] { + let caret = Cursor::new(0, i); + assert!( + caret_rect_for(buffer, &caret).is_some(), + "the PRIMARY caret is unaffected at index {i}" + ); + assert_eq!( + secondary_caret_rect_for(buffer, &caret), + None, + "a pure-LTR caret at index {i} has no secondary indicator" + ); + } +} + +#[test] +fn secondary_caret_rect_for_emits_at_a_mixed_bidi_boundary() { + // A mixed LTR/RTL line (the GPU-file corpus): the LTR↔RTL joins are genuine + // direction boundaries where the split caret's secondary indicator lands. + let mut app = geometry_app(); + app.update(); + support::register_fixture_font(&mut app, "Noto Sans Hebrew", "NotoSansHebrew-hebrew.ttf"); + let e = app + .world_mut() + .spawn(( + Node, + Style::default().width_px(400.0).height_px(40.0), + Text("hello עולם world".into()), + FontFamily(FontStack(vec![ + FamilyEntry::Named("Fira Sans".into()), + FamilyEntry::Named("Noto Sans Hebrew".into()), + ])), + )) + .id(); + settle(&mut app); + + let buffer = &app.world().get::(e).expect("TextBuffer").buffer; + let run = buffer.layout_runs().next().expect("one shaped run"); + + // DATA-DERIVE the boundary index AND the expected before-glyph edge from the + // REAL shaped glyph array — pinned against the shaper, never an assumption. + // A boundary byte index `i` has a BEFORE glyph (end == i) and an AFTER glyph + // (start == i) of OPPOSITE direction. + let (boundary, before_x, before_w, before_rtl) = run + .glyphs + .iter() + .find_map(|b| { + run.glyphs.iter().find_map(|a| { + (a.start == b.end && a.level.is_rtl() != b.level.is_rtl()).then_some(( + b.end, + b.x, + b.w, + b.level.is_rtl(), + )) + }) + }) + .expect("the mixed corpus has a direction boundary"); + + let caret = Cursor::new(0, boundary); + let primary = caret_rect_for(buffer, &caret).expect("primary caret at the boundary"); + let secondary = secondary_caret_rect_for(buffer, &caret).expect("secondary at the boundary"); + + // The COMMITTED rule: the secondary is the BEFORE glyph's logical-end visual + // edge — LTR → right edge (x + w), RTL → left edge (x). Equality with the + // primary x is ALLOWED (reordered runs can share a visual pen-x while being + // two distinct logical insertion points); we assert the EXACT data-derived + // edge, never `primary.x != secondary.x`. + let expected = if before_rtl { + before_x + } else { + before_x + before_w + }; + assert_eq!( + secondary.min.x, expected, + "secondary sits at the before-glyph logical-end edge" + ); + // Built from the before glyph's RUN geometry: top-anchored, a SHORTER mark. + assert_eq!( + secondary.min.y, run.line_top, + "secondary is top-anchored to the line box" + ); + assert!( + secondary.height() <= primary.height(), + "secondary is a shorter indicator than the full-height primary \ + (sec {sec}, primary {prim})", + sec = secondary.height(), + prim = primary.height() + ); + assert_eq!( + secondary.width(), + primary.width(), + "secondary shares the caret bar width" + ); +} + +#[test] +fn secondary_caret_rect_for_emits_on_a_wrapped_continuation_run() { + // A logical line that SOFT-WRAPS yields MULTIPLE LayoutRuns sharing one + // `line_i` (cosmic 0.19 emits one run per wrapped layout_line). When the + // direction boundary lives on a CONTINUATION segment, its glyphs are in a + // LATER run — `secondary_caret_rect_for` must scan PAST the first matching + // run (not return None on it). A narrow width forces several wrap segments; + // the mixed LTR/RTL corpus puts a boundary on a non-first run. + let mut app = geometry_app(); + app.update(); + support::register_fixture_font(&mut app, "Noto Sans Hebrew", "NotoSansHebrew-hebrew.ttf"); + let e = app + .world_mut() + .spawn(( + Node, + // Narrow enough that the long mixed string wraps into multiple runs. + Style::default().width_px(90.0).height_px(120.0), + Text("hello world עולם foo שלום bar".into()), + FontFamily(FontStack(vec![ + FamilyEntry::Named("Fira Sans".into()), + FamilyEntry::Named("Noto Sans Hebrew".into()), + ])), + )) + .id(); + settle(&mut app); + + let buffer = &app.world().get::(e).expect("TextBuffer").buffer; + + // Confirm the precondition this test exists to cover: the logical line + // (line 0) actually wrapped into MORE THAN ONE LayoutRun. Without this the + // assertion below would silently degrade to the single-run case the sibling + // test already covers. + let run_count = buffer.layout_runs().filter(|r| r.line_i == 0).count(); + assert!( + run_count > 1, + "the narrow width must soft-wrap line 0 into multiple runs (got {run_count})" + ); + + // Find a direction boundary that lives on a NON-FIRST run of line 0 — the + // exact case the first-run-only walk dropped. Data-derive it from the real + // shaped glyph arrays, per-run (glyph byte indices are line-relative, shared + // across the wrap segments of one logical line). + let mut found = None; + for (run_idx, run) in buffer + .layout_runs() + .filter(|r| r.line_i == 0) + .enumerate() + .skip(1) + { + for b in run.glyphs { + if let Some((boundary, before_x, before_w, before_rtl)) = + run.glyphs.iter().find_map(|a| { + (a.start == b.end && a.level.is_rtl() != b.level.is_rtl()).then_some(( + b.end, + b.x, + b.w, + b.level.is_rtl(), + )) + }) + { + found = Some(( + run_idx, + run.line_top, + boundary, + before_x, + before_w, + before_rtl, + )); + break; + } + } + if found.is_some() { + break; + } + } + let (_run_idx, run_top, boundary, before_x, before_w, before_rtl) = + found.expect("a direction boundary on a wrapped continuation run of line 0"); + + let caret = Cursor::new(0, boundary); + let secondary = secondary_caret_rect_for(buffer, &caret) + .expect("secondary surfaces even when the boundary is on a continuation run"); + + let expected = if before_rtl { + before_x + } else { + before_x + before_w + }; + assert_eq!( + secondary.min.x, expected, + "secondary sits at the before-glyph logical-end edge (continuation run)" + ); + assert_eq!( + secondary.min.y, run_top, + "secondary uses the OWNING continuation run's line_top, not the first run's" + ); +} diff --git a/crates/buiy_core/tests/text_caret_selection.rs b/crates/buiy_core/tests/text_caret_selection.rs index f39ad9c..94021b4 100644 --- a/crates/buiy_core/tests/text_caret_selection.rs +++ b/crates/buiy_core/tests/text_caret_selection.rs @@ -597,6 +597,7 @@ fn caret_emits_one_snapped_stamp_after_all_glyphs() { CaretVisual { visible: true, rect: Rect::new(12.3, 0.0, 13.3, 19.2), + secondary: None, }, )) .id(); @@ -649,6 +650,7 @@ fn caret_color_chain_resolves_at_emission() { CaretVisual { visible: true, rect: Rect::new(0.0, 0.0, 1.0, 19.2), + secondary: None, }, )) .id(); @@ -700,6 +702,7 @@ fn invisible_or_removed_caret_emits_nothing() { CaretVisual { visible: true, rect: Rect::new(0.0, 0.0, 1.0, 19.2), + secondary: None, }, )) .id(); @@ -743,6 +746,7 @@ fn empty_text_still_carries_a_caret() { CaretVisual { visible: true, rect: Rect::new(0.0, 0.0, 1.0, 19.2), + secondary: None, }, )); h.settle(); @@ -769,6 +773,7 @@ fn blink_edges_rebuild_glyphs_only_and_steady_phases_rebuild_nothing() { CaretVisual { visible: true, rect: Rect::new(2.0, 0.0, 3.0, 19.2), + secondary: None, }, )); h.settle(); diff --git a/crates/buiy_core/tests/text_caret_selection_e3_gpu.rs b/crates/buiy_core/tests/text_caret_selection_e3_gpu.rs index 571cc67..cabe181 100644 --- a/crates/buiy_core/tests/text_caret_selection_e3_gpu.rs +++ b/crates/buiy_core/tests/text_caret_selection_e3_gpu.rs @@ -222,3 +222,151 @@ fn e3_editor_driven_caret_paints_a_bar_right_of_the_ink() { let diff = perceptual_diff(&frame, &frame_b); assert!(diff < 1e-4, "two fresh captures diverged: {diff}"); } + +/// Drive the FOCUSED editor's caret to the mixed-corpus DIRECTION BOUNDARY (the +/// LTR↔RTL join) so the E3 writer stamps BOTH the primary caret and the +/// secondary split-caret indicator (§§ 4.1, 5). The boundary byte index is +/// DATA-DERIVED from the shaped buffer (not hand-counted), then reached with +/// LOGICAL-order `Motion::Next` steps (NOT the §Scope-flagged visual-order +/// `Right`): `Motion::Next` advances in logical order, so looping until +/// `state.caret().index` equals the derived boundary is fully deterministic. +fn capture_boundary() -> Vec { + let _cfg = GoldenConfig::deterministic(); + let mut app = support::gpu_render_app(W, H); + app.add_plugins(buiy_core::focus::FocusPlugin); + app.world_mut() + .insert_resource(ButtonInput::::default()); + app.world_mut().resource_mut::>().pause(); + support::finish_and_run(&mut app, 0); + support::register_fixture_font(&mut app, "Noto Sans Hebrew", "NotoSansHebrew-hebrew.ttf"); + { + let mut theme = app.world_mut().resource_mut::(); + theme.colors.insert(TEXT_TOKEN.into(), Color::WHITE); + theme.colors.insert(CARET_COLOR_TOKEN.into(), caret_red()); + } + let editor = app + .world_mut() + .spawn(( + Node, + Style::default(), + Text(String::from("hello עולם world")), + FontFamily(FontStack(vec![ + FamilyEntry::Named(String::from("Fira Sans")), + FamilyEntry::Named(String::from("Noto Sans Hebrew")), + ])), + FontSize(20.0), + TextColor(ColorToken::Token(Cow::Borrowed(TEXT_TOKEN))), + CaretColor(ColorToken::Token(Cow::Borrowed(CARET_COLOR_TOKEN))), + TextEditState::new(Metrics::new(20.0, 24.0)), + )) + .id(); + app.world_mut() + .spawn(( + Node, + Style::default() + .flex_column() + .width_px(W as f32) + .height_px(H as f32), + )) + .add_child(editor); + app.update(); + app.update(); + app.world_mut().resource_mut::().0 = Some(editor); + + { + let fonts = app.world().resource::().clone(); + let mut state = app.world_mut().get_mut::(editor).unwrap(); + // DATA-DERIVE the first LTR↔RTL boundary byte index from the editor's + // own shaped buffer (the same convention the headless geometry unit + // pins). + let boundary = state + .with_buffer(|b| { + b.layout_runs().next().and_then(|run| { + run.glyphs.iter().find_map(|before| { + run.glyphs.iter().find_map(|after| { + (after.start == before.end + && after.level.is_rtl() != before.level.is_rtl()) + .then_some(before.end) + }) + }) + }) + }) + .expect("the mixed corpus has a direction boundary"); + + let mut fs = fonts.lock(); + state.apply( + &mut fs, + EditCommand::Motion(Motion::Home, false), + false, + false, + ); + // Walk LOGICAL-order Next until the caret lands on the boundary index. + // Bounded to avoid a runaway if the buffer changes shape. + for _ in 0..64 { + if state.caret().index == boundary { + break; + } + state.apply( + &mut fs, + EditCommand::Motion(Motion::Next, false), + false, + false, + ); + } + drop(fs); + assert_eq!( + state.caret().index, + boundary, + "logical Next walk reached the derived boundary index" + ); + } + app.update(); // E3 writer mirrors the caret into CaretVisual (+ secondary) + + let target = support::render_to_image(&mut app, W, H); + support::spawn_capture_camera(&mut app, target.clone()); + support::wait_for_text_ready(&mut app, 60); + support::readback_rgba(&mut app, target) +} + +#[test] +#[ignore = "needs a wgpu adapter; E3 split-caret secondary indicator golden (editing-and-ime §§ 4.1, 5)"] +fn e3_split_caret_paints_primary_plus_secondary_at_a_bidi_boundary() { + let frame = capture_boundary(); + + // At the direction boundary the writer stamps TWO red caret bands: the + // primary full-height bar plus the shorter secondary indicator. They may + // share a column or sit apart depending on the reordered run geometry, so + // assert the COUNT of bands, not their separation. + let caret_bands = bands(&cols_where(&frame, W, H, is_strong_red)); + assert_eq!( + caret_bands.len(), + 2, + "two E3 split-caret column bands at the bidi boundary: {caret_bands:?}" + ); + + // The secondary indicator is SHORTER in row-extent than the full-height + // primary: measure each band's vertical red span and assert one is strictly + // taller (the primary) than the other (the secondary). + let row_span = |band: &Range| -> u32 { + let rows: Vec = (0..H) + .filter(|&y| { + (band.start..band.end).any(|x| is_strong_red(support::px(&frame, W, x, y))) + }) + .collect(); + match (rows.first(), rows.last()) { + (Some(&lo), Some(&hi)) => hi - lo + 1, + _ => 0, + } + }; + let mut spans: Vec = caret_bands.iter().map(row_span).collect(); + spans.sort_unstable(); + assert!( + spans[0] < spans[1], + "the secondary indicator is shorter than the full-height primary caret: {spans:?}" + ); + + // Re-capture determinism: an independent fresh capture matches. + let frame_b = capture_boundary(); + let diff = perceptual_diff(&frame, &frame_b); + assert!(diff < 1e-4, "two fresh boundary captures diverged: {diff}"); +} diff --git a/crates/buiy_core/tests/text_effect_group_gpu.rs b/crates/buiy_core/tests/text_effect_group_gpu.rs index 73e2942..1e85408 100644 --- a/crates/buiy_core/tests/text_effect_group_gpu.rs +++ b/crates/buiy_core/tests/text_effect_group_gpu.rs @@ -131,6 +131,7 @@ fn text_in_opacity_group_dims_exactly_once() { CaretVisual { visible: true, rect: Rect::new(60.0, 0.0, 61.0, 24.0), + secondary: None, }, CaretColor(ColorToken::Token(Cow::Borrowed(CARET_TOKEN))), )); diff --git a/crates/buiy_core/tests/text_golden_suite_gpu.rs b/crates/buiy_core/tests/text_golden_suite_gpu.rs index 675395d..55880a8 100644 --- a/crates/buiy_core/tests/text_golden_suite_gpu.rs +++ b/crates/buiy_core/tests/text_golden_suite_gpu.rs @@ -312,6 +312,7 @@ fn capture_input_state(filled: bool) -> Vec { CaretVisual { visible: true, rect: Rect::new(100.0, 0.0, 101.0, 24.0), + secondary: None, }, )); } else { diff --git a/crates/buiy_core/tests/text_selection_caret_gpu.rs b/crates/buiy_core/tests/text_selection_caret_gpu.rs index eb73839..45f13c9 100644 --- a/crates/buiy_core/tests/text_selection_caret_gpu.rs +++ b/crates/buiy_core/tests/text_selection_caret_gpu.rs @@ -252,6 +252,7 @@ fn spawn_blink_fixture(app: &mut App) -> Entity { CaretVisual { visible: true, rect: Rect::new(80.0, 0.0, 81.0, 48.0), + secondary: None, }, CaretColor(ColorToken::Token(Cow::Borrowed(CARET_TOKEN))), )) diff --git a/docs/plans/follow-ups.md b/docs/plans/follow-ups.md index 00df6ca..b1dcd26 100644 --- a/docs/plans/follow-ups.md +++ b/docs/plans/follow-ups.md @@ -1083,34 +1083,70 @@ design's numbers, never this backlog's. ## Text editing — BiDi split caret (E3 deferral) -**Status:** deferred from E3 ([2026-06-13-buiy-text-editing-e3-caret-selection.md](2026-06-13-buiy-text-editing-e3-caret-selection.md)). +**Status: LANDED** (follow-up slice after E3–E6; `followups-drain` worktree). +Superseded the E3 deferral. Was: deferred from E3 +([2026-06-13-buiy-text-editing-e3-caret-selection.md](2026-06-13-buiy-text-editing-e3-caret-selection.md)). **What it is:** when the caret sits on a bidirectional direction boundary, the spec (editing-and-ime.md §§ 4.1, 5) calls for **two** caret marks — a primary full-height bar at one run's edge plus a secondary indicator at the other — so the user can tell which direction the next typed character will flow. -**Why deferred:** cosmic-text 0.19 exposes no API for the dual position. -`LayoutRun::cursor_position` → `cursor_glyph` (vendored `buffer.rs:148-179`) -compares **only** `(cursor.line, cursor.index)` and never reads -`cursor.affinity`, and a single unwrapped line is **one** `LayoutRun` — so -repeated `cursor_position` calls cannot surface the second position, and there -is no second run to scan. cosmic's own `Editor::draw` paints a single caret -(`find_map` first run). E3 therefore paints the **single primary caret** (zero -correctness risk — the caret is always present and correct; only the -mixed-direction *secondary indicator* is missing), exactly as the multi-range -selection *behavior* is a named deferral. - -**Correct approach (the work):** compute the secondary x from glyph-level -geometry at the boundary byte index — the LTR glyph's trailing edge vs the RTL -glyph's leading edge, using the affinity pair, à la `cursor_from_glyph_left` / -`cursor_from_glyph_right` (vendored `buffer.rs:181-197`, which DO encode -affinity). Paint as a secondary `CaretVisual`-style rect + a second solid stamp -(CPU geometry only — no new GPU, the E3 paint path already stamps the primary). - -**Owner:** the text-editing campaign, as a focused follow-up slice after E3–E6. - -**Spec touchpoint:** editing-and-ime.md §§ 4.1, 5, 13 (as-landed deferral notes). +**As landed.** The secondary indicator rides +`CaretVisual.secondary: Option` (a FIELD, not a standalone component — the +extract producer's query/`Changed`/`RemovedComponents` params are at Bevy's +15-tuple cap, so the field reuses the primary's damage trigger and clear). +`secondary_caret_rect_for(buffer, caret)` (caret.rs) returns `Some` only at a +direction boundary — where a BEFORE glyph (`end == index`) and an AFTER glyph +(`start == index`) abut with OPPOSITE `level.is_rtl()`. The secondary x is the +**BEFORE glyph's logical-end visual edge**: LTR → `x + w`, RTL → `x` (cosmic's +own convention, `buffer.rs:120-142` / `cursor_from_glyph_right`). It is +top-anchored at `SECONDARY_CARET_H_FRAC` (0.5) of the line box — a shorter mark +than the full-height primary. The paint is a SECOND solid-stamp instance in +extract.rs (CPU geometry only, reusing the primary's atlas entry/color/clip/page +— no new GPU, no new atlas insert). The writer compares the `(rect, secondary)` +pair so a boundary crossing that changes only the secondary still re-emits. + +**Why a field, not a new component.** The §6.3-style primary `CaretVisual` is +already the seat; the cap on the extract producer's tuples (extract.rs § 6.1) +makes a 16th seat impossible without a risky refactor of the hottest text +system. Recorded in the §5 spec update so a future "cleanup" to a standalone +component is not silently attempted. + +**The cosmic gotcha (why E3 couldn't surface it).** `cursor_glyph` +(`buffer.rs:151-174`) is affinity-blind AND order-defined — it resolves +`index == glyph.start` BEFORE `index == glyph.end`, so its single +`cursor_position` only ever reports the AFTER (start-glyph) edge, which the +PRIMARY already paints. The SECONDARY is the OTHER abutting glyph's logical-end +edge — a glyph-level position cosmic's one cursor call cannot reach (the second +position is glyph-level, not run-level). + +**Soft-wrap (multiple runs per logical line).** A logical line that wraps emits +SEVERAL `LayoutRun`s sharing one `line_i` (cosmic 0.19 `LayoutRunIter`: one run +per wrapped `layout_line`; glyph byte indices are line-relative across the +segments). So `secondary_caret_rect_for` mirrors `caret_rect_for`'s all-runs +scan: a `line_i`-matching run that holds neither abutting glyph at the index is +the WRONG wrap segment (or a run extremity) — it CONTINUES rather than concluding +`None`, and the rect uses the OWNING run's `line_top`. Only single-line editors +force `Wrap::None` (sync.rs:521); multi-line wrapping editors and display text +produce multi-run logical lines, so a boundary on a continuation segment must +still surface the secondary. + +**Tests.** Headless `text_caret_geometry.rs`: `secondary_caret_rect_for` returns +`None` for a pure-LTR caret and `Some` at a data-derived mixed-BiDi boundary +(asserting the EXACT before-glyph edge x — equality with the primary allowed, +never `primary.x != secondary.x`); a narrow soft-wrapped mixed line surfaces the +secondary on a NON-FIRST run of the logical line (asserting the owning +continuation run's `line_top`); the end-to-end editor caret carries no +secondary on ASCII; `CaretVisual::default().secondary == None`. GPU `#[ignore]` +`text_caret_selection_e3_gpu.rs`: the existing single-band primary golden is +unchanged; an additive boundary golden drives the caret to the data-derived +boundary via logical-order `Motion::Next` and asserts TWO red bands with the +secondary shorter in row-extent. + +**Owner:** the text-editing campaign (delivered). + +**Spec touchpoint:** editing-and-ime.md §§ 4.1, 5, 13 + abstract (as-landed notes). ## Text editing — multi-range selection *behavior* (E-campaign deferral) diff --git a/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md b/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md index de9becc..4aa7857 100644 --- a/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md +++ b/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md @@ -51,9 +51,10 @@ required by anything in this file (§ 5). > the placeholder, the § 11 Message taxonomy, and the > `buiy_widgets::TextInput` bundle. Per-section **"As landed (E_n)"** notes > below record the mechanical errata folded at closure. The **named deferrals** -> (multi-range selection *behavior*, HTML/image clipboard, the BiDi split caret, +> (multi-range selection *behavior*, HTML/image clipboard, > compose-over-selection) are filed in -> [follow-ups.md](../../plans/follow-ups.md) (§ 13). +> [follow-ups.md](../../plans/follow-ups.md) (§ 13). The **BiDi split caret** +> secondary indicator landed as a post-E3 follow-up (§§ 4.1, 5). > > *(Superseded 2026-06-13 by the as-landed paragraph above — the > proposal-time record, kept for history.)* **Status: design-only (deferred @@ -273,13 +274,20 @@ When the caret sits on a direction boundary, **both** positions are emitted (BiDi split caret: primary full-height + secondary indicator), resolved from the two candidate runs the affinity pair names. **F** -> **As landed (E3): the split caret is deferred to a follow-up.** E3 paints the -> single primary caret. cosmic-text 0.19 surfaces no dual-caret position — -> `LayoutRun::cursor_position`/`cursor_glyph` ignore `cursor.affinity` and a -> line is one `LayoutRun`, so the "two candidate runs" the draft assumed do not -> exist; the secondary mark needs glyph-edge geometry (a separate slice). See -> [follow-ups.md § Text editing — BiDi split caret](../../plans/follow-ups.md). -> Zero correctness risk — the caret is always present and correct. +> **As landed: the secondary indicator now lands (follow-up after E3).** E3 +> shipped only the primary caret; a post-E3 slice added the secondary +> (`secondary_caret_rect_for`, caret.rs). The honest residual the draft's +> "two candidate runs" framing got wrong: cosmic 0.19's `cursor_glyph` +> (buffer.rs:151-174) is affinity-blind AND order-defined — it resolves +> `index == glyph.start` BEFORE `index == glyph.end`, so its single +> `cursor_position` only ever surfaces the AFTER (start-glyph) edge, which the +> primary already paints. Buiy computes the SECONDARY directly as the BEFORE +> glyph's (`end == index`) LOGICAL-END visual edge — LTR → `x + w`, RTL → `x` +> (cosmic's own convention, buffer.rs:120-142 / `cursor_from_glyph_right`). It +> rides `CaretVisual.secondary: Option` and paints as a second solid +> stamp (CPU geometry only — no new GPU). A line is one `LayoutRun`, so there +> are no "two candidate runs"; the second position is glyph-level, not run-level. +> See [follow-ups.md § Text editing — BiDi split caret](../../plans/follow-ups.md). ### § 4.2 Decision: a multi-range-shaped Buiy selection type @@ -341,8 +349,15 @@ seats).** All editor visuals are existing-primitive emissions: glyphs, and a "next layer" would misuse the `painters_z` stacking index for a within-node ordering concern. Split caret (§ 4.1) = a **secondary `CaretVisual` rect + a second stamp** (CPU geometry only — still no GPU work); - *as landed (E3): the secondary indicator is deferred (§§ 4.1, 13) — cosmic 0.19 - surfaces no dual-caret position — so v1 paints only the single primary stamp.* + *as landed (follow-up after E3): the secondary indicator NOW lands as a + `secondary: Option` FIELD on `CaretVisual` (not a standalone component) + + a second solid-stamp instance reusing the primary's entry/color/clip/page; + the secondary sits at the boundary's BEFORE-glyph logical-end visual edge. + Why a field, not a component: the extract producer's component + query/`Changed` trigger/`RemovedComponents` params are all at Bevy's 15-tuple + cap (extract.rs § 6.1), so a 16th seat is impossible without refactoring the + hottest text system — the field rides `Changed` damage and + `RemovedComponents` clear for free.* - **Caret blink** is a `CaretVisual { visible, rect }` state edge written by render-prep ([decoration-and-paint.md § 6.3](decoration-and-paint.md)); the edge rebuilds `ExtractedGlyphs` through the **independent glyph damage @@ -633,11 +648,17 @@ cut/copy/paste; `UndoStack` with composition grouping + typing coalescing; reduced-motion; auto-scroll via `ScrollOffset`; the § 11 taxonomy; the `TextInput` bundle. +**Landed as a follow-up after E3:** the **BiDi split caret** secondary indicator +(§§ 4.1, 5) — `secondary_caret_rect_for` (the before-glyph logical-end edge) on +`CaretVisual.secondary: Option` + a second solid stamp. No residual: +the secondary now paints at every LTR↔RTL direction boundary, INCLUDING one on a +soft-wrapped continuation segment — `secondary_caret_rect_for` mirrors the +primary's all-runs scan over the multiple `LayoutRun`s a wrapped logical line +emits (continue past a non-owning wrap run; only conclude `None` after the last), +rather than inspecting just the first `line_i`-matching run. + **Deferred within F (named, next slice, not dropped):** multi-range selection -*behavior* (§ 4.2); HTML + image clipboard flavors (§ 7); the **BiDi split -caret** secondary indicator (§§ 4.1, 5 — cosmic 0.19 has no dual-caret API; -needs glyph-edge geometry; E3 ships the single primary caret — -[follow-ups.md § Text editing — BiDi split caret](../../plans/follow-ups.md)); +*behavior* (§ 4.2); HTML + image clipboard flavors (§ 7); **compose-over-selection** (§§ 6.1, 6.2 — E5 splices the preedit at the caret and does not replace an active selection first; [follow-ups.md § Text editing — compose-over-selection](../../plans/follow-ups.md)). From 10cc34ee4da80997f932d6d41907e9c2d5dd0afb Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 23:15:10 -0700 Subject: [PATCH 2/3] feat(text-editing): compose-over-selection (E5 deferral) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an IME composition starts while text is selected, the selection is now REPLACED by the preedit (platform/web convention) instead of leaving the selected text in place beside the preedit. editing-and-ime §§6.1/6.2. splice_preedit, on the FIRST splice of a composition (preedit.is_none) over a non-collapsed selection, deletes the selection inside a start_change/ finish_change pair and STASHES the reversible Change + pre-delete caret/ selection on TextEditState (a separate field, so PreeditSpan stays Eq), then splices the preedit at the collapsed caret and returns true so apply_ime fires TextChanged (the one §11 exception — the delete removed user text). At commit, the stashed delete's items are folded with the commit-insert items into ONE GroupKind::Composition undo unit whose caret_before/selection_before are the true pre-composition state — so a single Undo reverses BOTH the delete and the commit and restores the original selection. Cancel (empty preedit / Escape / blur) reverse-applies the stash (re-inserts the deleted text, restores the selection) → a true no-op, firing TextChanged back to the original value. Approach A over B (recorded in the spec note): Composition never coalesces (§6.2c), and the intervening preedit splices break caret-adjacency, so the delete cannot be a separate coalesced unit — it must be folded into the one commit unit. The unselected-caret path takes no branch and returns false → byte-identical to the E5 plain-text IME path (regression-guarded). No new GPU, no new event surface; §3.2 extract tripwire respected (delete rides the same mid-frame mutation + dirty-mark as the existing splice). Tests: text_ime_ops 9/9 (delete+stash, one-unit commit, single-undo restores both, redo), text_ime_system 10/10 (TextChanged on the delete), lib text 30/30; clippy -D warnings clean. Spec §§6.1/6.2/13 + follow-ups.md LANDED. 0.19-rc base. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- crates/buiy_core/src/text/edit/ime.rs | 152 ++++++++++++++- crates/buiy_core/src/text/edit/input.rs | 20 +- crates/buiy_core/src/text/edit/state.rs | 29 ++- crates/buiy_core/tests/text_ime_ops.rs | 183 ++++++++++++++++++ crates/buiy_core/tests/text_ime_system.rs | 182 +++++++++++++++++ docs/plans/follow-ups.md | 33 +++- .../editing-and-ime.md | 54 +++++- 7 files changed, 631 insertions(+), 22 deletions(-) diff --git a/crates/buiy_core/src/text/edit/ime.rs b/crates/buiy_core/src/text/edit/ime.rs index d23a88e..241c11d 100644 --- a/crates/buiy_core/src/text/edit/ime.rs +++ b/crates/buiy_core/src/text/edit/ime.rs @@ -22,7 +22,7 @@ use cosmic_text::{Action, Attrs, AttrsList, BufferLine, Cursor, Edit, FontSystem use super::caret::caret_rect_for; use super::input::TextChanged; -use super::state::{Disabled, ReadOnly, TextEditState}; +use super::state::{ComposeDelete, Disabled, ReadOnly, TextEditState}; use super::undo::{GroupKind, UndoUnit}; use crate::FocusedEntity; use crate::layout::LayoutTree; @@ -101,16 +101,36 @@ impl TextEditState { /// the last. Records the new `PreeditSpan` (with the in-preedit /// `cursor`). Reshape is deferred (the line's caches reset; the caller /// dirty-marks for next-frame measure). + /// + /// **Compose-over-selection (§ 6.2).** On the FIRST splice of a composition + /// (`self.preedit.is_none()`), if a non-collapsed selection is active it is + /// DELETED first (replace-selection convention) and the reversible delete is + /// STASHED on `self.compose_delete` — folded into the commit's Composition + /// unit, or reverse-applied on cancel. Returns `true` iff that delete ran, + /// so the caller can flag the logical value change (the one exception to "no + /// `TextChanged` on preedit" — the delete removed user text). The + /// unselected-caret path takes no branch and returns `false` (byte-identical + /// to the plain-text IME path). The boolean is consumed at the system seam + /// (`apply_ime`); direct unit-test callers may ignore it. pub fn splice_preedit( &mut self, font_system: &mut FontSystem, value: &str, cursor: Option<(usize, usize)>, - ) { + ) -> bool { let _ = font_system; // splice needs no FontSystem (reshape is deferred) + // First splice over a non-collapsed selection: delete it (and stash the + // reversible delete) before splicing at the now-collapsed caret. Guarded + // on `preedit.is_none()` so replace-splices never re-enter (by + // construction a live composition has no selection). + let deleted_selection = if self.preedit.is_none() && !value.is_empty() { + self.take_compose_delete_if_selected() + } else { + false + }; self.remove_preedit_inner(); if value.is_empty() { - return; // an empty preedit is a removal, not a splice + return false; // an empty preedit is a removal, not a splice } let caret = self.editor.cursor(); let line = caret.line; @@ -127,14 +147,70 @@ impl TextEditState { len, cursor, }); + deleted_selection + } + + /// If a non-collapsed selection is active, delete it and STASH the + /// reversible delete on `self.compose_delete` (capturing the pre-delete + /// caret + selection). Returns `true` iff a delete ran. Captures the delete + /// as a real cosmic `Change` (a throwaway `start_change`/`finish_change` + /// pair drives `delete_selection`'s recording) but does NOT touch the undo + /// stack — the stash is folded into the commit's Composition unit (Task 3) + /// or reverse-applied on cancel (Task 5). No-op when nothing is selected. + fn take_compose_delete_if_selected(&mut self) -> bool { + if self.mirror_selection().is_collapsed() { + return false; + } + let caret_before = self.editor.cursor(); + let selection_before = self.mirror_selection(); + self.editor.start_change(); + let deleted = self.editor.delete_selection(); + let change = self.editor.finish_change().unwrap_or_default(); + if !deleted || change.items.is_empty() { + // Defensive: a selection that produced no delete leaves no stash. + return false; + } + self.compose_delete = Some(ComposeDelete { + change, + caret_before, + selection_before, + }); + true } /// Remove the live preedit span (invariant d) — DIRECT surgery, NO /// `Change`. Restores the editor cursor to the span start. No-op if /// nothing is composing. - pub fn remove_preedit(&mut self, font_system: &mut FontSystem) { + /// + /// **Compose-over-selection cancel (§ 6.2).** If a pending + /// `compose_delete` stash exists, this is a CANCEL of a composition that + /// replaced a selection: reverse-apply the stash (re-insert the deleted + /// text, restore the pre-delete selection) so the value returns to its + /// pre-composition state, then clear the stash. Returns `true` iff that + /// reverse-apply ran (a genuine value change → the caller fires + /// `TextChanged`). The unselected-caret path carries no stash and returns + /// `false` (byte-identical to E5). The boolean is consumed at the system + /// seam / keyboard Escape; direct unit-test callers may ignore it. + pub fn remove_preedit(&mut self, font_system: &mut FontSystem) -> bool { let _ = font_system; self.remove_preedit_inner(); + self.restore_compose_delete() + } + + /// Reverse-apply a pending compose-over-selection delete (cancel path): the + /// stashed delete `Change` reversed re-inserts the deleted text; the + /// pre-delete caret + selection are restored. Clears the stash. Returns + /// `true` iff a stash was present (the value changed back). Shared by the + /// `remove_preedit` cancel and the keyboard Escape cancel (input.rs). + pub(crate) fn restore_compose_delete(&mut self) -> bool { + let Some(stash) = self.compose_delete.take() else { + return false; + }; + let mut reversed = stash.change; + reversed.reverse(); + self.editor.apply_change(&reversed); + self.restore_cursor(stash.caret_before, stash.selection_before); + true } /// Internal: delete the recorded span's bytes from its line via direct @@ -246,18 +322,46 @@ impl TextEditState { /// coalescing run first so the commit is never folded into prior typing. pub fn commit_preedit(&mut self, font_system: &mut FontSystem, value: &str, now: Duration) { self.remove_preedit_inner(); + // A pending compose-over-selection delete (§ 6.2) is FOLDED into this + // commit's Composition unit (NOT reverse-applied — the selection stays + // deleted, replaced by the committed text). Take it now; its pre-delete + // caret/selection become the unit's `_before` so one undo reverses both. + let stash = self.compose_delete.take(); if value.is_empty() { + // Commit of an empty string over a deleted selection: the delete + // still stands as its own one-step undo (the selection IS gone). + // Record it as the Composition unit so a single undo restores it. + if let Some(stash) = stash { + self.record_compose_delete_only(stash, now); + } return; } self.undo.seal(); - let caret_before = self.editor.cursor(); - let selection_before = self.mirror_selection(); + // `_before` is the PRE-DELETE state when a selection was replaced, else + // the live caret (the plain-commit path — byte-identical to E5). + let caret_before = stash + .as_ref() + .map(|s| s.caret_before) + .unwrap_or_else(|| self.editor.cursor()); + let selection_before = stash + .as_ref() + .map(|s| s.selection_before.clone()) + .unwrap_or_else(|| self.mirror_selection()); self.editor.start_change(); for ch in value.chars() { self.editor.action(font_system, Action::Insert(ch)); } - let change = self.editor.finish_change().unwrap_or_default(); + let mut change = self.editor.finish_change().unwrap_or_default(); + + // Fold the stashed delete items BEFORE the commit-insert items, so the + // combined `Change` replays forward as delete-then-insert and its + // `reverse()` replays un-insert-then-un-delete (one undo restores both). + if let Some(stash) = stash { + let mut items = stash.change.items; + items.append(&mut change.items); + change.items = items; + } let caret_after = self.editor.cursor(); let selection_after = self.mirror_selection(); @@ -273,6 +377,27 @@ impl TextEditState { now, ); } + + /// Record a stashed compose-over-selection delete as a standalone + /// `Composition` undo unit (the empty-commit-over-selection corner: the + /// committed string is empty but the selection IS gone, so the delete must + /// remain one undoable step). `_after` is the post-delete state. + fn record_compose_delete_only(&mut self, stash: ComposeDelete, now: Duration) { + self.undo.seal(); + let caret_after = self.editor.cursor(); + let selection_after = self.mirror_selection(); + self.undo.record_grouped( + UndoUnit { + change: stash.change, + caret_before: stash.caret_before, + caret_after, + selection_before: stash.selection_before, + selection_after, + group: GroupKind::Composition, + }, + now, + ); + } } /// Emitted when a composition begins (editing-and-ime § 11; § 6.3 transition @@ -349,13 +474,18 @@ pub fn apply_ime( if value.is_empty() { // Cancel: remove the span (invariant d). End if one was active. if state.has_preedit() { - state.remove_preedit(&mut font_system); + // A compose-over-selection cancel reverse-applies the + // stashed delete (value returns to original → TextChanged). + value_changed |= state.remove_preedit(&mut font_system); end.write(CompositionEnd(entity, String::new())); span_changed = true; } } else { let was_composing = state.has_preedit(); - state.splice_preedit(&mut font_system, &value, cursor); + // The first splice over a selection deletes it (a genuine + // value change → TextChanged); the unselected path returns + // false (no TextChanged on a plain preedit — § 11). + value_changed |= state.splice_preedit(&mut font_system, &value, cursor); if was_composing { update.write(CompositionUpdate(entity, value)); } else { @@ -372,7 +502,9 @@ pub fn apply_ime( } Ime::Disabled { .. } => { if state.has_preedit() { - state.remove_preedit(&mut font_system); + // Disabled mid-composition is a cancel: reverse-apply any + // compose-over-selection delete (value returns → TextChanged). + value_changed |= state.remove_preedit(&mut font_system); end.write(CompositionEnd(entity, String::new())); span_changed = true; } diff --git a/crates/buiy_core/src/text/edit/input.rs b/crates/buiy_core/src/text/edit/input.rs index a63839b..4718d2e 100644 --- a/crates/buiy_core/src/text/edit/input.rs +++ b/crates/buiy_core/src/text/edit/input.rs @@ -186,12 +186,20 @@ impl TextEditState { // never in `value()`), so flag `reshaped` for the M1 re-measure // without emitting `TextChanged`. let cleared = self.has_preedit(); - if cleared { - self.remove_preedit(font_system); - } + // `remove_preedit` reverse-applies any compose-over-selection + // delete (re-inserting the deleted text): that IS a logical + // value change, so route it to `value_changed` → `TextChanged`. + // The plain-preedit cancel restores nothing (returns false) and + // only `reshaped` fires (buffer changed, value did not). + let restored = if cleared { + self.remove_preedit(font_system) + } else { + false + }; self.editor.action(font_system, Action::Escape); EditOutcome { reshaped: cleared, + value_changed: restored, ..Default::default() } } @@ -336,7 +344,11 @@ impl TextEditState { /// Restore the caret + the editor's selection after an undo/redo. The /// editor's `Selection` is the authoritative one E3 mirrors OUT next pass; /// we set both the cursor and (for a non-collapsed range) the anchor. - fn restore_cursor(&mut self, caret: Cursor, selection: super::selection::TextSelection) { + pub(crate) fn restore_cursor( + &mut self, + caret: Cursor, + selection: super::selection::TextSelection, + ) { self.editor.set_cursor(caret); if selection.is_collapsed() { self.editor.set_selection(Selection::None); diff --git a/crates/buiy_core/src/text/edit/state.rs b/crates/buiy_core/src/text/edit/state.rs index d116e95..2de0ea3 100644 --- a/crates/buiy_core/src/text/edit/state.rs +++ b/crates/buiy_core/src/text/edit/state.rs @@ -15,13 +15,33 @@ use std::time::Duration; use bevy::prelude::*; -use cosmic_text::{Buffer, Editor, Metrics}; +use cosmic_text::{Buffer, Change, Cursor, Editor, Metrics}; use super::ime::PreeditSpan; use super::selection::TextSelection; use super::undo::UndoStack; use crate::text::IntrinsicWidths; +/// The pending compose-over-selection delete (editing-and-ime § 6.2, +/// "Compose-over-selection"). When a composition STARTS over a non-collapsed +/// selection, the selection is deleted first and the reversible `Change` is +/// STASHED here (not pushed onto the undo stack — invariant a holds for the +/// splice). It is consumed at `Ime::Commit` (folded into the ONE Composition +/// undo unit) or reverse-applied on cancel (re-inserting the deleted text). +/// +/// It lives on `TextEditState`, NOT on `PreeditSpan`: `PreeditSpan` derives +/// `Eq` (tests compare spans), and `cosmic_text::Change` is not `Eq`. +#[derive(Clone, Debug)] +pub(crate) struct ComposeDelete { + /// The reversible delete `Change` (delete-of-the-selection items). + pub change: Change, + /// The caret BEFORE the delete (the true pre-composition caret — recorded + /// as the combined unit's `caret_before` so undo restores it). + pub caret_before: Cursor, + /// The selection BEFORE the delete (restored on undo / cancel). + pub selection_before: TextSelection, +} + /// The per-entity caret-blink phase origin (editing-and-ime §§ 5, 10). The T7 /// `write_caret_blink` writer was deliberately GLOBAL + stateless — a square /// wave of the raw app clock (visual.rs module doc: "per-entity phase reset on @@ -97,6 +117,12 @@ pub struct TextEditState { /// read by `value()` (byte-range exclusion, invariant b) and the geometry /// writer (`PreeditVisual`). pub(crate) preedit: Option, + /// The pending compose-over-selection delete (§ 6.2). `Some` only while a + /// composition that STARTED over a non-collapsed selection is live; the + /// stashed delete is folded into the commit's Composition unit or + /// reverse-applied on cancel. Written only by the `ime.rs` splice/commit/ + /// remove methods. + pub(crate) compose_delete: Option, } impl TextEditState { @@ -132,6 +158,7 @@ impl TextEditState { blink: CaretBlink::default(), undo: UndoStack::default(), preedit: None, + compose_delete: None, } } diff --git a/crates/buiy_core/tests/text_ime_ops.rs b/crates/buiy_core/tests/text_ime_ops.rs index 455be98..2abb95c 100644 --- a/crates/buiy_core/tests/text_ime_ops.rs +++ b/crates/buiy_core/tests/text_ime_ops.rs @@ -114,6 +114,189 @@ fn cosmic_motion_left() -> EditCommand { EditCommand::Motion(cosmic_text::Motion::Left, false) } +/// Compose-over-selection: when a composition STARTS over a non-collapsed +/// selection, the first splice DELETES the selection first (replace-selection +/// convention) and splices the preedit at the now-collapsed caret. The stashed +/// delete is NOT yet on the undo stack — invariant (a) still holds for the +/// splice itself. +#[test] +fn compose_over_selection_splice_deletes_and_stashes() { + let fonts = SharedFontSystem::new(); + let mut state = TextEditState::new(Metrics::new(16.0, 19.2)); + let mut fs = fonts.lock(); + + // "abc", then select "bc" (SelectAll selects all; shrink by moving the + // anchor — simplest is: caret to index 1, then extend Right ×2). + state.apply(&mut fs, EditCommand::Insert("abc".into()), false, false); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Home, false), + false, + false, + ); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Right, false), + false, + false, + ); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Right, true), + false, + false, + ); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Right, true), + false, + false, + ); + assert!( + !state.mirror_selection().is_collapsed(), + "bc is selected before composition starts" + ); + let undo_before = state.undo_depth(); + + // Start a composition over the selection. + state.splice_preedit(&mut fs, "X", Some((0, 1))); + + // The selection is gone, the preedit sits at the collapsed caret. + assert_eq!(state.buffer_text_for_test(), "aX"); + // Logical value excludes the preedit AND the selection is already deleted. + assert_eq!(state.value(), "a"); + assert!(state.has_preedit()); + // The stashed delete has NOT reached the undo stack yet (invariant a holds + // for the splice; the delete is folded in only at commit). + assert_eq!( + state.undo_depth(), + undo_before, + "the stashed compose-delete is not yet an undo unit" + ); +} + +/// Compose-over-selection: commit folds the stashed delete + the commit-insert +/// into ONE `GroupKind::Composition` unit. A single Undo restores BOTH the +/// deleted selection text and the committed text; redo replays both. +#[test] +fn compose_over_selection_commit_is_one_unit_and_one_undo_restores_both() { + use buiy_core::text::edit::GroupKind; + let fonts = SharedFontSystem::new(); + let mut state = TextEditState::new(Metrics::new(16.0, 19.2)); + let mut fs = fonts.lock(); + + // "abc", select "bc". + state.apply(&mut fs, EditCommand::Insert("abc".into()), false, false); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Home, false), + false, + false, + ); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Right, false), + false, + false, + ); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Right, true), + false, + false, + ); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Right, true), + false, + false, + ); + let undo_before = state.undo_depth(); + + state.splice_preedit(&mut fs, "ni", None); + state.commit_preedit(&mut fs, "你", Duration::ZERO); + + assert_eq!(state.value(), "a你", "selection replaced by the commit"); + assert!(!state.has_preedit()); + assert_eq!( + state.undo_depth(), + undo_before + 1, + "delete + commit fold into ONE unit" + ); + assert_eq!( + state.undo_top_group_for_test(), + Some(GroupKind::Composition) + ); + + // ONE undo restores BOTH the deleted "bc" and the committed "你". + state.apply(&mut fs, EditCommand::Undo, false, false); + assert_eq!( + state.value(), + "abc", + "one undo reverses both the delete and the commit" + ); + assert!( + !state.mirror_selection().is_collapsed(), + "the bc selection is restored on undo" + ); + + // Redo replays both. + state.apply(&mut fs, EditCommand::Redo, false, false); + assert_eq!(state.value(), "a你", "redo replays delete + commit"); +} + +/// Compose-over-selection cancel (Escape / empty preedit at the unit level): +/// reverse-applies the stashed delete so the value returns to the original and +/// the selection is restored. The unselected path (no stash) is a no-op. +#[test] +fn compose_over_selection_cancel_restores_via_remove() { + let fonts = SharedFontSystem::new(); + let mut state = TextEditState::new(Metrics::new(16.0, 19.2)); + let mut fs = fonts.lock(); + + state.apply(&mut fs, EditCommand::Insert("abc".into()), false, false); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Home, false), + false, + false, + ); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Right, false), + false, + false, + ); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Right, true), + false, + false, + ); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Right, true), + false, + false, + ); + + state.splice_preedit(&mut fs, "ni", None); + assert_eq!(state.value(), "a", "selection deleted at compose start"); + + // Cancel via remove_preedit: the deleted "bc" comes back, selection restored. + state.remove_preedit(&mut fs); + assert_eq!( + state.value(), + "abc", + "cancel reverse-applies the compose-delete" + ); + assert!(!state.has_preedit()); + assert!( + !state.mirror_selection().is_collapsed(), + "the bc selection is restored on cancel" + ); +} + use std::time::Duration; /// Invariant (c): a full composition (one or more Preedit splices then a diff --git a/crates/buiy_core/tests/text_ime_system.rs b/crates/buiy_core/tests/text_ime_system.rs index 076c6a2..87577a1 100644 --- a/crates/buiy_core/tests/text_ime_system.rs +++ b/crates/buiy_core/tests/text_ime_system.rs @@ -83,6 +83,188 @@ fn drain_count(app: &mut App) -> usize { .count() } +/// A focused editor whose buffer is SEEDED with `text` through the real +/// TextSync seam (spawn with `Text(text)`, settle the N→N+1 latency), then with +/// `[from, to)` (byte indices on line 0) SELECTED via the editor's own motion +/// path. The selection is established AFTER TextSync settles, so the +/// `Added` lazy re-apply (sync.rs:188) cannot clobber it. This is +/// the only way to drive a real non-collapsed selection into the system tests. +fn app_with_selection(text: &str, from: usize, to: usize) -> (App, Entity) { + use buiy_core::text::SharedFontSystem; + use buiy_core::text::edit::EditCommand; + let mut app = App::new(); + app.add_plugins(MinimalPlugins); + app.add_plugins(buiy_core::CorePlugin); + app.add_plugins(buiy_core::layout::LayoutPlugin); + app.add_plugins(buiy_core::text::BuiyTextPlugin::default()); + app.add_plugins(buiy_core::focus::FocusPlugin); + app.add_message::(); + app.add_message::(); + app.insert_resource(ButtonInput::::default()); + app.insert_resource(Clipboard(Box::new(MemClipboard::default()))); + let editor = app + .world_mut() + .spawn(( + Node, + Style::default().width_px(300.0).height_px(60.0), + Text(text.to_string()), + TextEditState::new(Metrics::new(16.0, 19.2)), + )) + .id(); + app.world_mut().resource_mut::().0 = Some(editor); + app.world_mut().resource_mut::>().pause(); + // Settle TextSync into the editor buffer (N→N+1): the authored `Text` + // routes into the editor-owned buffer on the second update. + app.update(); + app.update(); + + // Now select [from, to) via the editor's BiDi-correct motion path. + let fonts = app.world().resource::().clone(); + { + let mut state = app.world_mut().get_mut::(editor).unwrap(); + let mut fs = fonts.lock(); + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Home, false), + false, + false, + ); + for _ in 0..from { + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Right, false), + false, + false, + ); + } + for _ in from..to { + state.apply( + &mut fs, + EditCommand::Motion(cosmic_text::Motion::Right, true), + false, + false, + ); + } + assert!( + !state.mirror_selection().is_collapsed(), + "selection established before composition" + ); + } + (app, editor) +} + +/// Compose-over-selection at the system level: the first non-empty Preedit +/// over an active selection DELETES the selection — a genuine value change, so +/// `TextChanged` fires (contrast the unselected preedit, which fires 0). The +/// preedit is excluded from the value, so the value reads as the post-delete +/// remainder. +#[test] +fn compose_over_selection_delete_fires_textchanged() { + use buiy_core::text::edit::TextChanged; + let (mut app, editor) = app_with_selection("abc", 1, 3); // select "bc" + + send_preedit(&mut app, "ni", None); + app.update(); + + assert_eq!( + count::(&app), + 1, + "the compose-over-selection delete is a genuine value change" + ); + assert_eq!( + value(&app, editor), + "a", + "selection deleted, preedit excluded" + ); + assert!( + app.world() + .get::(editor) + .unwrap() + .has_preedit() + ); +} + +/// Compose-over-selection cancel (empty Preedit): the stashed delete is +/// reverse-applied so the value returns to the original "abc", and a SECOND +/// `TextChanged` fires (the symmetric value transition). +#[test] +fn compose_over_selection_cancel_restores_selection() { + use buiy_core::text::edit::TextChanged; + let (mut app, editor) = app_with_selection("abc", 1, 3); + + send_preedit(&mut app, "ni", None); + app.update(); + assert_eq!(value(&app, editor), "a"); + // Drain the first TextChanged so the next frame's count is clean. + let _ = drain_count::(&mut app); + + send_preedit(&mut app, "", None); // cancel + app.update(); + assert_eq!( + value(&app, editor), + "abc", + "cancel reverse-applies the compose-delete" + ); + assert!( + !app.world() + .get::(editor) + .unwrap() + .has_preedit() + ); + assert_eq!( + drain_count::(&mut app), + 1, + "the restore is a genuine value change" + ); +} + +/// Compose-over-selection cancel via `Ime::Disabled`: same restore + TextChanged. +#[test] +fn compose_over_selection_disabled_restores_selection() { + use buiy_core::text::edit::TextChanged; + let (mut app, editor) = app_with_selection("abc", 1, 3); + + send_preedit(&mut app, "ni", None); + app.update(); + let _ = drain_count::(&mut app); + + app.world_mut().write_message(Ime::Disabled { + window: Entity::PLACEHOLDER, + }); + app.update(); + assert_eq!( + value(&app, editor), + "abc", + "Disabled cancel restores the value" + ); + assert_eq!( + drain_count::(&mut app), + 1, + "the restore fires TextChanged" + ); +} + +/// Compose-over-selection commit through the system: ONE undo unit; one Undo +/// (driven via the keyboard Ctrl+Z path) restores both the deleted selection +/// and the committed text. +#[test] +fn compose_over_selection_commit_one_unit_system() { + let (mut app, editor) = app_with_selection("abc", 1, 3); + let undo_before = undo_depth(&app, editor); + + send_preedit(&mut app, "ni", None); + app.update(); + send_commit(&mut app, "你"); + app.update(); + + assert_eq!(value(&app, editor), "a你"); + assert_eq!( + undo_depth(&app, editor), + undo_before + 1, + "delete + commit = ONE unit" + ); +} + /// Invariant (a) + (b) at the system level: during composition the undo stack /// is unchanged and the logical value excludes the preedit. #[test] diff --git a/docs/plans/follow-ups.md b/docs/plans/follow-ups.md index b1dcd26..33ef722 100644 --- a/docs/plans/follow-ups.md +++ b/docs/plans/follow-ups.md @@ -1187,9 +1187,36 @@ facade makes adding flavors local (no API churn). **Spec touchpoint:** editing-and-ime.md §§ 7, 13 (named deferral); OQ#3. -## Text editing — compose-over-selection (E5 deferral) - -**Status:** deferred from the `buiy-text-editing` campaign (E5 IME; +## Text editing — compose-over-selection (E5 deferral) — LANDED + +**Status:** **Landed.** When a composition starts over a non-collapsed +selection, the selection is now deleted first (replace-selection convention) and +the preedit is spliced at the now-collapsed caret. The selection-delete is +captured as a reversible cosmic `Change`, **stashed** on the new +`TextEditState::compose_delete` field (not pushed onto the undo stack — invariant +(a) still holds for the splice), and at `Ime::Commit` it is **folded into the +same `GroupKind::Composition` undo unit** as the commit-insert +(`caret_before`/`selection_before` captured pre-delete): one undo restores BOTH +the deleted text and the committed text, one redo replays both. A **cancel** +(empty `Preedit` / `Ime::Disabled` / `Escape`) reverse-applies the stash, +re-inserting the deleted text and restoring the selection. `TextChanged` fires on +the delete (a genuine value change — the one documented exception to "never +preedit") and again on the cancel-restore. The plain-text unselected-caret path +is byte-identical to E5. *Approach rejected:* recording the delete as its own +Composition unit and coalescing the commit into it — `Composition` never +coalesces (§ 6.2c) and the intervening preedit splice breaks caret-adjacency, so +it would yield two units. **Implementing files:** `crates/buiy_core/src/text/edit/ime.rs`, +`crates/buiy_core/src/text/edit/state.rs` (`ComposeDelete` + `compose_delete` +field), with the keyboard Escape value-change routed through +`crates/buiy_core/src/text/edit/input.rs`. **Tests:** +`crates/buiy_core/tests/text_ime_ops.rs` (splice-deletes-and-stashes, +commit-is-one-unit + one-undo-restores-both + redo, cancel-restores-via-remove) and +`crates/buiy_core/tests/text_ime_system.rs` (delete-fires-TextChanged, +cancel/Disabled-restores + re-fires-TextChanged, commit-one-unit). Spec +editing-and-ime.md §§ 6.1 / 6.2 / 13 updated (deferral reversed). No new GPU, no +new event surface. The original deferral context is preserved below. + +**Originally deferred** from the `buiy-text-editing` campaign (E5 IME; [E5 plan](2026-06-13-buiy-text-editing-e5-ime.md)). **What it is:** when text is selected and the user starts an IME composition, diff --git a/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md b/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md index 4aa7857..1b0e3e4 100644 --- a/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md +++ b/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md @@ -393,6 +393,13 @@ editor's (display) Buffer** at the caret as a metadata-marked `Attrs` span; each subsequent `Preedit` replaces the span; `Preedit` with empty value, or `Ime::Disabled`, or focus loss removes it. **F** +**Addendum (landed as a follow-up after E5/E6):** the display-splice now +**replaces an active selection** — when a composition starts over a +non-collapsed selection, the selection is deleted first and the preedit is +spliced at the now-collapsed caret (the platform/web replace-selection +convention). See § 6.2 "Compose-over-selection" for the undo/`TextChanged` +contract. + **Rationale.** Web parity requires preedit to **reflow** the line: composing CJK mid-paragraph shifts following text and can re-wrap, and the preedit-cursor F row is only correct when the preedit participates in real shaping. @@ -417,6 +424,35 @@ span. The preedit underline styles the marked span (§ 5); the in-preedit cursor from `Preedit.cursor` (byte range into the preedit string) renders as a caret inside the span. **F** +**Compose-over-selection (landed as a follow-up after E5/E6).** When a +composition *starts* over a **non-collapsed selection**, the selection is +deleted first (platform/web replace-selection convention, § 6.1). The +selection-delete is folded into the SAME `GroupKind::Composition` unit as the +eventual commit: `caret_before`/`selection_before` are captured **pre-delete**, +the stashed delete `Change` is prepended to the commit-insert items, and the +combined `Change` is recorded as ONE unit — so a single Undo restores BOTH the +deleted text and the committed text (and the redo replays both). The stash lives +on a dedicated `TextEditState::compose_delete` field (NOT on `PreeditSpan`, which +derives `Eq` — `cosmic_text::Change` is not `Eq`). It is captured by running +`delete_selection()` inside a throwaway `start_change`/`finish_change` pair at the +FIRST splice of a composition, so invariant (a) still holds for the splice itself +(the delete `Change` is stashed, never pushed onto the undo stack until commit). + +This is the **one exception** to invariant (b)'s "value reads exclude preedit / +§ 11 never preedit": the pre-splice selection-delete IS a genuine logical value +change (it removed user text), so it emits `TextChanged`. A **cancel** (empty +`Preedit` / `Ime::Disabled` / `Escape`) over a stashed compose-delete +**reverse-applies** the stash (re-inserts the deleted text, restores the +selection) and clears it — so cancel returns the value to its pre-composition +state and fires `TextChanged` again (the symmetric value transition). The +unselected-caret path carries no stash and is **byte-identical** to E5: no +delete, no extra `TextChanged`. *Approach rejected:* recording the delete +immediately as its own Composition unit and coalescing the commit into it — +`GroupKind::Composition` never coalesces (§ 6.2c, `undo.rs`), and the intervening +preedit splices break caret-adjacency anyway, so it would yield two undo units or +require weakening the never-coalesce rule. Stashing on the live composition and +folding at commit keeps the one-unit guarantee with no rule change. + ### § 6.3 Popup positioning through Bevy 0.18 **Decision.** Use the supported `Window` fields: set `Window.ime_enabled = true` @@ -657,11 +693,21 @@ primary's all-runs scan over the multiple `LayoutRun`s a wrapped logical line emits (continue past a non-owning wrap run; only conclude `None` after the last), rather than inspecting just the first `line_i`-matching run. +**Landed as a follow-up after E5/E6:** **compose-over-selection** (§§ 6.1, 6.2). +As built: when a composition starts over a non-collapsed selection the selection +is deleted first (replace-selection convention, § 6.1), and the +selection-delete is folded into the SAME `GroupKind::Composition` undo unit as +the commit (`caret_before`/`selection_before` captured pre-delete) — one undo +restores BOTH the deleted text and the committed text, one redo replays both. +`TextChanged` fires on the delete (a genuine value change — the documented one +exception to "never preedit") and again on a cancel-restore (empty `Preedit` / +`Ime::Disabled` / `Escape` reverse-applies the stashed delete). The plain-text +unselected-caret path is byte-identical to E5 (no delete, no extra +`TextChanged`). Implemented in `text/edit/ime.rs` + `state.rs` +(`compose_delete` stash); tested in `text_ime_ops.rs` + `text_ime_system.rs`. + **Deferred within F (named, next slice, not dropped):** multi-range selection -*behavior* (§ 4.2); HTML + image clipboard flavors (§ 7); -**compose-over-selection** (§§ 6.1, 6.2 — E5 splices the preedit at the caret -and does not replace an active selection first; -[follow-ups.md § Text editing — compose-over-selection](../../plans/follow-ups.md)). +*behavior* (§ 4.2); HTML + image clipboard flavors (§ 7). **Out (E-tier):** rich-text edit surface, document virtualization. --- From 4ee5a8bd1dcd03020da1b691f7088bdae2e0ccf1 Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 23:43:16 -0700 Subject: [PATCH 3/3] feat(text-editing): HTML + image clipboard flavors (E-campaign deferral) E4 shipped plain-text clipboard only; the F row names text + HTML + image MIME. Adds HTML + image flavors to the ClipboardProvider facade without touching the plain-text path or the section 3.3 newline policy. ClipboardProvider gains get_html/set_html (always available) and get_image/set_image (behind a new clipboard-image cargo feature -> arboard/image-data). MemClipboard stores independent html/image slots (the headless-testable path); ArboardClipboard delegates to arboard 3.6.1's get().html()/set_html() and (gated) get_image()/set_image(). A Buiy-owned ClipboardImage {width,height,bytes} owned struct keeps arboard's borrowed ImageData<'a> out of the trait signature. Copy/Cut now ALSO set an html flavor (the plain text escaped via a single-pass escape_html); Paste's section 3.3 text path is unchanged (the html getter is available to callers but Paste does not consult it) - E4 plain-text behavior is byte-identical (regression-guarded). arboard 3.6.1's HTML API verified present on the locked pin (no bump); image requires turning arboard's image-data feature back on, hence the opt-in clipboard-image feature (default build stays text-only). The gated image lane (cargo test -p buiy_core --features clipboard-image) is documented in CLAUDE.md Build and Test plus the follow-ups.md LANDED note, since the default workspace gate does not enable it. Tests: text_clipboard_undo 13/13 default + 15/15 with clipboard-image (html round-trip, copy/cut dual-set html, paste-prefers-text, image round-trip); clippy -D warnings clean both lanes. OQ#3 resolved. Spec sections 7/13 + follow-ups.md LANDED. Final text-track slice. 0.19-rc base. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- CLAUDE.md | 4 + crates/buiy_core/Cargo.toml | 7 ++ crates/buiy_core/src/text/edit/clipboard.rs | 105 ++++++++++++++++-- crates/buiy_core/src/text/edit/input.rs | 28 ++++- crates/buiy_core/src/text/edit/mod.rs | 2 + crates/buiy_core/tests/text_clipboard_undo.rs | 70 ++++++++++++ crates/buiy_core/tests/text_undo_ops.rs | 73 ++++++++++++ docs/plans/follow-ups.md | 55 +++++++-- .../editing-and-ime.md | 27 +++-- 9 files changed, 340 insertions(+), 31 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index dba8eb0..a4d344d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -93,6 +93,10 @@ Other useful one-offs: - `BUIY_ACCEPT_SHAPING=1 cargo test -p buiy_core --test text_shaping_snapshots` — regenerate the `.snap` shaping snapshots (curated: review the diff before committing). +- `cargo test -p buiy_core --features clipboard-image` — exercise the clipboard + image flavor (`ClipboardImage`, `get_image`/`set_image`). The default + workspace gate runs with this feature **OFF** (the image module compiles out), + so this gated lane must be run separately to keep the image path from rotting. ## Code Conventions diff --git a/crates/buiy_core/Cargo.toml b/crates/buiy_core/Cargo.toml index 7899884..600c269 100644 --- a/crates/buiy_core/Cargo.toml +++ b/crates/buiy_core/Cargo.toml @@ -57,6 +57,13 @@ default = ["default_font"] # Family::SansSerif etc. resolve identically on every host. Disable to ship # zero font bytes — the app must then register fonts before any text renders. default_font = [] +# The clipboard image flavor (editing-and-ime § 7). OFF by default: it turns on +# arboard's `image-data` feature, which drags in the image-clipboard deps +# (objc2-core-graphics on macOS, gdi on Windows; the `image` crate is already +# in-tree). The text + HTML flavors need none of this, so the text-only build +# stays lean. Exercised with `cargo test -p buiy_core --features clipboard-image` +# — the default `cargo test --workspace` gate runs with this OFF. +clipboard-image = ["arboard/image-data"] [dev-dependencies] naga = "29" diff --git a/crates/buiy_core/src/text/edit/clipboard.rs b/crates/buiy_core/src/text/edit/clipboard.rs index 0442664..1f265af 100644 --- a/crates/buiy_core/src/text/edit/clipboard.rs +++ b/crates/buiy_core/src/text/edit/clipboard.rs @@ -1,23 +1,52 @@ //! The clipboard facade (editing-and-ime § 7). `arboard` behind a //! `ClipboardProvider` Resource trait-object so tests inject a fake and the -//! dependency stays swappable. v1 is PLAIN TEXT only (HTML/image deferred — -//! pre-campaign decision 4). This file names NO cosmic type, so the -//! facade-boundary tripwire does not constrain it — it lives in `text::edit` -//! for cohesion (it is editing mechanism), not because it must. +//! dependency stays swappable. v1 shipped PLAIN TEXT only; the named follow-up +//! slice adds an HTML flavor (always available) and an image flavor (behind the +//! `clipboard-image` cargo feature, which turns on arboard's `image-data`). +//! This file names NO cosmic type, so the facade-boundary tripwire does not +//! constrain it — it lives in `text::edit` for cohesion (it is editing +//! mechanism), not because it must. use bevy::prelude::Resource; -/// The swappable clipboard backend. Plain text only in v1. Both methods take -/// `&mut self`: a real clipboard owns OS handles that mutate on read on some -/// platforms, and the fake owns interior state — `&mut` keeps the trait -/// honest for both. Errors are swallowed to `None` / no-op (a clipboard that -/// is unavailable must never crash an editor; spec § 7 "must not be optional" -/// is about presence, not infallibility). +/// An owned clipboard image (RGBA8, row-major, `width * height * 4` bytes). +/// Buiy-owned so the [`ClipboardProvider`] trait signature names no arboard +/// `ImageData<'a>` borrowed-lifetime type; conversion to/from arboard happens +/// at the [`ArboardClipboard`] boundary (a one-time byte copy — clipboard +/// payloads are not a hot path). Behind the `clipboard-image` feature because +/// the image flavor pulls arboard's `image-data` deps. +#[cfg(feature = "clipboard-image")] +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ClipboardImage { + pub width: usize, + pub height: usize, + pub bytes: Vec, +} + +/// The swappable clipboard backend. Carries a plain-text flavor (always) and an +/// HTML flavor (always); the image flavor is behind the `clipboard-image` +/// feature. All methods take `&mut self`: a real clipboard owns OS handles that +/// mutate on read on some platforms, and the fake owns interior state — `&mut` +/// keeps the trait honest for both. Errors are swallowed to `None` / no-op (a +/// clipboard that is unavailable must never crash an editor; spec § 7 "must not +/// be optional" is about presence, not infallibility). pub trait ClipboardProvider: Send + Sync + 'static { /// The current clipboard text, or `None` if empty / unavailable. fn get_text(&mut self) -> Option; /// Replace the clipboard text. A failure is a silent no-op. fn set_text(&mut self, text: String); + /// The current clipboard HTML flavor, or `None` if absent / unavailable. + /// A plain-text editor reads text by preference; the HTML getter is here + /// for rich-content callers, not for the § 3.3 plain-text Paste path. + fn get_html(&mut self) -> Option; + /// Replace the clipboard HTML flavor. A failure is a silent no-op. + fn set_html(&mut self, html: String); + /// The current clipboard image, or `None` if absent / unavailable. + #[cfg(feature = "clipboard-image")] + fn get_image(&mut self) -> Option; + /// Replace the clipboard image. A failure is a silent no-op. + #[cfg(feature = "clipboard-image")] + fn set_image(&mut self, image: ClipboardImage); } /// The active provider, a Resource newtype over the boxed trait object @@ -59,6 +88,41 @@ impl ClipboardProvider for ArboardClipboard { let _ = h.set_text(text); } } + + fn get_html(&mut self) -> Option { + // `get().html()` is on arboard's cross-platform Get builder and is NOT + // behind `image-data` (verified against arboard 3.6.1). + self.handle()?.get().html().ok() + } + + fn set_html(&mut self, html: String) { + if let Some(h) = self.handle() { + // No separate alt text: a plain-text editor's html flavor is just + // escaped text, and the text flavor is set independently. + let _ = h.set_html(html, None); + } + } + + #[cfg(feature = "clipboard-image")] + fn get_image(&mut self) -> Option { + let img = self.handle()?.get_image().ok()?; + Some(ClipboardImage { + width: img.width, + height: img.height, + bytes: img.bytes.into_owned(), + }) + } + + #[cfg(feature = "clipboard-image")] + fn set_image(&mut self, image: ClipboardImage) { + if let Some(h) = self.handle() { + let _ = h.set_image(arboard::ImageData { + width: image.width, + height: image.height, + bytes: std::borrow::Cow::Owned(image.bytes), + }); + } + } } /// An in-memory clipboard for tests (PUBLIC so integration-crate tests can @@ -68,6 +132,9 @@ impl ClipboardProvider for ArboardClipboard { #[derive(Default)] pub struct MemClipboard { text: Option, + html: Option, + #[cfg(feature = "clipboard-image")] + image: Option, } impl ClipboardProvider for MemClipboard { @@ -78,4 +145,22 @@ impl ClipboardProvider for MemClipboard { fn set_text(&mut self, text: String) { self.text = Some(text); } + + fn get_html(&mut self) -> Option { + self.html.clone() + } + + fn set_html(&mut self, html: String) { + self.html = Some(html); + } + + #[cfg(feature = "clipboard-image")] + fn get_image(&mut self) -> Option { + self.image.clone() + } + + #[cfg(feature = "clipboard-image")] + fn set_image(&mut self, image: ClipboardImage) { + self.image = Some(image); + } } diff --git a/crates/buiy_core/src/text/edit/input.rs b/crates/buiy_core/src/text/edit/input.rs index 4718d2e..456424d 100644 --- a/crates/buiy_core/src/text/edit/input.rs +++ b/crates/buiy_core/src/text/edit/input.rs @@ -213,11 +213,15 @@ impl TextEditState { EditCommand::Undo => self.apply_undo(), EditCommand::Redo => self.apply_redo(), - // ── Clipboard (§ 7) — plain text only (decision 4) ─────────── + // ── Clipboard (§ 7) — text + html flavors ──────────────────── EditCommand::Copy => { // copy_selection() is None when there is no selection; a bare // caret Copy is a no-op (web parity). if let Some(text) = self.editor.copy_selection() { + // Set BOTH flavors: the raw text (the § 7 path) and an + // escaped-html flavor (a plain-text editor has no rich runs, + // so its html representation is just the escaped text). + ctx.clipboard.set_html(escape_html(&text)); ctx.clipboard.set_text(text); } EditOutcome::default() @@ -229,6 +233,7 @@ impl TextEditState { let Some(text) = self.editor.copy_selection() else { return EditOutcome::default(); // nothing selected }; + ctx.clipboard.set_html(escape_html(&text)); ctx.clipboard.set_text(text); // Delete the selection as one DISCRETE undoable unit (a cut is // a deliberate single action — never coalesced with neighbors). @@ -371,6 +376,27 @@ impl TextEditState { } } +/// Escape a plain-text string into an HTML-safe fragment for the clipboard's +/// HTML flavor. A plain-text editor has no rich runs, so its html +/// representation is simply the text with the five markup-significant +/// characters escaped (`& < > " '`). Single-pass: each input char maps to its +/// escape token in one match and emitted tokens are never re-scanned, so there +/// is no double-escape and ordering is irrelevant. +fn escape_html(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for ch in s.chars() { + match ch { + '&' => out.push_str("&"), + '<' => out.push_str("<"), + '>' => out.push_str(">"), + '"' => out.push_str("""), + '\'' => out.push_str("'"), + _ => out.push(ch), + } + } + out +} + use bevy::input::keyboard::{Key, KeyboardInput}; use bevy::input::{ButtonInput, ButtonState}; diff --git a/crates/buiy_core/src/text/edit/mod.rs b/crates/buiy_core/src/text/edit/mod.rs index efde881..048d166 100644 --- a/crates/buiy_core/src/text/edit/mod.rs +++ b/crates/buiy_core/src/text/edit/mod.rs @@ -31,6 +31,8 @@ pub use caret::{ CaretMoved, SelectionChanged, caret_rect_for, secondary_caret_rect_for, write_caret_and_selection, }; +#[cfg(feature = "clipboard-image")] +pub use clipboard::ClipboardImage; pub use clipboard::{ArboardClipboard, Clipboard, ClipboardProvider, MemClipboard}; pub use command::EditCommand; pub use ime::{ diff --git a/crates/buiy_core/tests/text_clipboard_undo.rs b/crates/buiy_core/tests/text_clipboard_undo.rs index 58207aa..ad7fbb5 100644 --- a/crates/buiy_core/tests/text_clipboard_undo.rs +++ b/crates/buiy_core/tests/text_clipboard_undo.rs @@ -22,6 +22,32 @@ fn mem_clipboard_is_usable_as_a_trait_object() { let mut boxed: Box = Box::new(MemClipboard::default()); boxed.set_text("via dyn".to_string()); assert_eq!(boxed.get_text(), Some("via dyn".to_string())); + // The html flavor is reachable through the trait object too (T3.1). + boxed.set_html("via dyn".to_string()); + assert_eq!(boxed.get_html(), Some("via dyn".to_string())); +} + +#[test] +fn mem_clipboard_round_trips_html() { + let mut c = MemClipboard::default(); + assert_eq!(c.get_html(), None, "empty clipboard reads None html"); + c.set_html("x".to_string()); + assert_eq!(c.get_html(), Some("x".to_string())); + c.set_html("y".to_string()); + assert_eq!(c.get_html(), Some("y".to_string()), "set overwrites"); + + // The html and text slots are INDEPENDENT: writing one never touches the + // other (a plain-text editor must be able to keep distinct flavors). + let mut c = MemClipboard::default(); + c.set_text("plain".to_string()); + assert_eq!(c.get_html(), None, "setting text does not invent html"); + c.set_html("rich".to_string()); + assert_eq!( + c.get_text(), + Some("plain".to_string()), + "setting html leaves text untouched" + ); + assert_eq!(c.get_html(), Some("rich".to_string())); } use buiy_core::text::edit::{GroupKind, UndoStack, UndoUnit}; @@ -233,3 +259,47 @@ fn discrete_and_composition_never_coalesce() { ); assert_eq!(stack.undo_len(), 4, "each composition is its own unit"); } + +// ── Image flavor (behind the `clipboard-image` cargo feature) ──────────── +// Run with: cargo test -p buiy_core --features clipboard-image +// The default `cargo test --workspace` gate compiles this module out. +#[cfg(feature = "clipboard-image")] +mod image_flavor { + use buiy_core::text::edit::{ClipboardImage, ClipboardProvider, MemClipboard}; + + /// A 2x2 RGBA image (16 bytes) — enough to prove the round-trip carries + /// width/height/bytes intact. + fn sample() -> ClipboardImage { + ClipboardImage { + width: 2, + height: 2, + bytes: (0..16u8).collect(), + } + } + + #[test] + fn mem_clipboard_round_trips_image() { + let mut c = MemClipboard::default(); + assert_eq!(c.get_image(), None, "empty clipboard reads None image"); + + c.set_image(sample()); + assert_eq!(c.get_image(), Some(sample()), "image round-trips"); + + let other = ClipboardImage { + width: 1, + height: 1, + bytes: vec![9, 9, 9, 9], + }; + c.set_image(other.clone()); + assert_eq!(c.get_image(), Some(other), "set overwrites"); + } + + #[test] + fn image_and_text_slots_are_independent() { + let mut c = MemClipboard::default(); + c.set_text("plain".to_string()); + c.set_image(sample()); + assert_eq!(c.get_text(), Some("plain".to_string())); + assert_eq!(c.get_image(), Some(sample())); + } +} diff --git a/crates/buiy_core/tests/text_undo_ops.rs b/crates/buiy_core/tests/text_undo_ops.rs index 13e0a20..15d08f9 100644 --- a/crates/buiy_core/tests/text_undo_ops.rs +++ b/crates/buiy_core/tests/text_undo_ops.rs @@ -210,3 +210,76 @@ fn paste_with_an_empty_clipboard_is_a_no_op() { assert!(!out.value_changed); assert_eq!(state.undo_depth(), 0, "nothing to paste, nothing recorded"); } + +// ── HTML flavor (the follow-up slice) ──────────────────────────────────── +// Copy/Cut now set BOTH the plain-text flavor (unchanged § 7 path) AND an +// HTML flavor (the escaped plain text — a plain-text editor has no rich runs). +// Paste still PREFERS the § 3.3 text path and never consults the HTML getter. + +#[test] +fn copy_also_sets_an_escaped_html_flavor() { + let fonts = SharedFontSystem::new(); + let mut state = TextEditState::new(Metrics::new(16.0, 19.2)); + let mut fs = fonts.lock(); + let mut clip = MemClipboard::default(); + + // Characters that MUST be escaped in html: & < > " ' + state.apply_tracked( + &mut fs, + EditCommand::Insert("a&\"'".into()), + &mut ctx(0, &mut clip), + ); + select_all(&mut state, &mut fs, &mut clip); + state.apply_tracked(&mut fs, EditCommand::Copy, &mut ctx(10, &mut clip)); + + // The plain-text flavor is the raw selection (§ 7 path, unchanged). + assert_eq!(clip.get_text(), Some("a&\"'".to_string())); + // The html flavor is the escaped form. + assert_eq!( + clip.get_html(), + Some("a<b>&"'".to_string()), + "copy sets an escaped-html flavor alongside the text" + ); +} + +#[test] +fn cut_sets_both_text_and_html_flavors() { + let fonts = SharedFontSystem::new(); + let mut state = TextEditState::new(Metrics::new(16.0, 19.2)); + let mut fs = fonts.lock(); + let mut clip = MemClipboard::default(); + + state.apply_tracked( + &mut fs, + EditCommand::Insert("xzzz".to_string()); + + let out = state.apply_tracked(&mut fs, EditCommand::Paste, &mut ctx(0, &mut clip)); + assert!(out.value_changed); + assert_eq!( + state.value(), + "abc", + "paste takes the § 3.3 text path, never the html flavor" + ); + assert_eq!(state.undo_depth(), 1, "paste is one undoable unit"); +} diff --git a/docs/plans/follow-ups.md b/docs/plans/follow-ups.md index 33ef722..5a5d474 100644 --- a/docs/plans/follow-ups.md +++ b/docs/plans/follow-ups.md @@ -1169,21 +1169,52 @@ reshape needed). **Spec touchpoint:** editing-and-ime.md §§ 4.2, 13 (named deferral). -## Text editing — HTML + image clipboard flavors (E-campaign deferral) - -**Status:** deferred from the `buiy-text-editing` campaign (E4 shipped plain +## Text editing — HTML + image clipboard flavors (E-campaign deferral) — LANDED + +**Status:** **Landed.** The `ClipboardProvider` facade gained `get_html`/ +`set_html` (always available) and `get_image`/`set_image` over a Buiy-owned +`ClipboardImage { width, height, bytes }` (behind the new `buiy_core` +`clipboard-image` cargo feature, which forwards to arboard's `image-data`). +Both impls carry the new flavors: `MemClipboard` stores an html slot and (gated) +an image slot — the headless-testable path; `ArboardClipboard` delegates to +arboard `Get::html()`/`Set::html()` and (gated) `get_image`/`set_image`, +converting at the borrowed-`ImageData<'a>` boundary. `Cut`/`Copy` now set BOTH +the plain-text flavor and an escaped-html flavor (`escape_html` escapes +`& < > " '`; a plain-text editor has no rich runs, so its html is just the +escaped text). **`Paste` is unchanged** — it takes the § 3.3 newline-strip text +path and never consults the html getter (the getter is for rich-content +callers); a regression test (`paste_prefers_text_and_ignores_html`) pins this. +OQ#3 **resolved**: arboard 3.6.1 `Get::html()`/`Set::html()` are on the +cross-platform builder and not feature-gated (verified against the locked +source); only image needs `image-data`. **Implementing files:** +`crates/buiy_core/src/text/edit/clipboard.rs` (trait + `ClipboardImage` + both +impls), `crates/buiy_core/src/text/edit/input.rs` (Copy/Cut dual-set + +`escape_html`), `crates/buiy_core/src/text/edit/mod.rs` (gated re-export), +`crates/buiy_core/Cargo.toml` (`clipboard-image = ["arboard/image-data"]`). +**Tests:** `crates/buiy_core/tests/text_clipboard_undo.rs` (MemClipboard html +round-trip + independent slots + trait-object; `#[cfg(feature="clipboard-image")]` +image round-trip + independent slots) and +`crates/buiy_core/tests/text_undo_ops.rs` (copy/cut dual-set the escaped-html +flavor; paste-prefers-text-and-ignores-html). **CI note:** the default +`cargo test --workspace` / `clippy --workspace --all-targets` gate runs with +`clipboard-image` **OFF** (the image module compiles out); the gated lane is +`cargo test -p buiy_core --features clipboard-image` (and a matching clippy run) +— add it to CI so the image path cannot rot silently. No new GPU, no new event +surface; arboard real-OS clipboard is not headless-testable, so only MemClipboard +is asserted (matching the E4 decision). The original deferral context is +preserved below. + +**Originally deferred** from the `buiy-text-editing` campaign (E4 shipped plain text). -**What it is:** the F row names text + HTML + image MIME for cut/copy/paste; E4 -ships **plain text only** (`Cut`/`Copy` via `copy_selection`, `Paste` through the -§ 3.3 newline policy) behind the `ClipboardProvider` facade. HTML + image flavors -are the named next slice. - -**Why deferred:** arboard's HTML *read-side* support is unverified -(editing-and-ime OQ#3) and must be confirmed before the slice is promised; the -facade makes adding flavors local (no API churn). +**What it was:** the F row names text + HTML + image MIME for cut/copy/paste; E4 +shipped **plain text only** (`Cut`/`Copy` via `copy_selection`, `Paste` through +the § 3.3 newline policy) behind the `ClipboardProvider` facade. HTML + image +flavors were the named next slice. -**Owner:** a focused follow-up slice; gated on confirming arboard HTML read. +**Why it was deferred:** arboard's HTML *read-side* support was unverified +(editing-and-ime OQ#3) and had to be confirmed before the slice was promised; the +facade made adding flavors local (no API churn). **Spec touchpoint:** editing-and-ime.md §§ 7, 13 (named deferral); OQ#3. diff --git a/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md b/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md index 1b0e3e4..e0c68e4 100644 --- a/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md +++ b/docs/specs/2026-06-09-buiy-text-rendering-design/editing-and-ime.md @@ -503,11 +503,17 @@ arboard's platform matrix for no supply-chain win; arboard's transitive deps are the same crates. **Also rejected:** deferral — cut/copy/paste is F and cheap once the facade exists. -**Phasing.** v1 ships plain text (`Cut`/`Copy` from `copy_selection()`, `Paste` -through the § 3.3 newline policy). The F row names text + HTML + image MIME: -HTML/image flavors are the named follow-up slice — arboard's HTML *read-side* -support is **unverified** and must be confirmed before that slice is promised -(§ 13). +**Phasing.** v1 shipped plain text (`Cut`/`Copy` from `copy_selection()`, +`Paste` through the § 3.3 newline policy). The follow-up slice **LANDED** the +HTML + image flavors behind the same `ClipboardProvider` facade: `get_html`/ +`set_html` are always available (arboard `Get::html()`/`Set::html()`, verified +not feature-gated — OQ#3 resolved); the image flavor (`get_image`/`set_image` +over a Buiy-owned `ClipboardImage`) is behind the `buiy_core` `clipboard-image` +cargo feature, which turns on arboard's `image-data`. `Cut`/`Copy` now set BOTH +the plain-text flavor and an escaped-html flavor (a plain-text editor has no +rich runs, so its html is just the escaped text); **`Paste` is unchanged** — it +takes the § 3.3 text path and never consults the html getter (the getter is for +rich-content callers). MemClipboard carries all three slots for headless tests. --- @@ -707,7 +713,9 @@ unselected-caret path is byte-identical to E5 (no delete, no extra (`compose_delete` stash); tested in `text_ime_ops.rs` + `text_ime_system.rs`. **Deferred within F (named, next slice, not dropped):** multi-range selection -*behavior* (§ 4.2); HTML + image clipboard flavors (§ 7). +*behavior* (§ 4.2). +**Landed within F:** HTML + image clipboard flavors (§ 7) — image behind the +`clipboard-image` cargo feature. **Out (E-tier):** rich-text edit surface, document virtualization. --- @@ -749,8 +757,11 @@ unselected-caret path is byte-identical to E5 (no delete, no extra `Send + Sync`; `Motion` has 22 variants. The `docs/prior-art/cosmic-text/` folder should receive a correction note (outside this spec folder's write scope), and sibling files citing those claims must re-verify. -3. **arboard HTML read-side** is unverified (§ 7) — confirm before scheduling - the HTML-clipboard slice. +3. **arboard HTML read-side** — **RESOLVED.** arboard 3.6.1 `Get::html()` is on + the cross-platform `Get` builder and is **not** feature-gated; `Set::html()` + likewise. Image (`get_image`/`set_image`, `ImageData`) is gated behind + arboard's `image-data` feature, which the `buiy_core` `clipboard-image` + feature turns on. HTML + image flavors landed on the facade (§ 7). 4. **Shared-accessor type for the editor-owned Buffer** (§ 2.2a): the concrete QueryData shape is pinned by [measure-and-layout.md](measure-and-layout.md); if that file lands a Buffer-as-separate-component model incompatible with