From fef886288b25eb2aa3c654d833962e2e6ea5b2fc Mon Sep 17 00:00:00 2001 From: Isaac Corbrey Date: Thu, 16 Jul 2026 16:50:42 -0400 Subject: [PATCH] core: fix cursor position + expose selection body on Snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit primary_cursor_range now derives cursor position from Range::cursor(text) — the same accessor helix-view's CursorCache uses — instead of raw Range::head. After commands like c that delete a selection and enter insert mode, Selection::ensure_invariants width-1-expands the collapsed range, so Range::head lands one grapheme past the visual cursor. Range:: cursor accounts for that; using it fixes an off-by-1 for c specifically and matches Helix's own render path in every mode. New primary_selection_range returns the primary Range body (from, to), so the frontend can draw the actual selection highlight — previously we only exposed the cursor block, so multi-grapheme selections (w, e, x, W, etc.) rendered as if collapsed. Snapshot gains selectionStart / selectionEnd; the JSON-shape test lists them alongside cursorStart / cursorEnd. Three regression tests cover the wire contract: word_selection_reports_ selection_body_range, insert_mode_reports_zero_width_cursor_at_head, and change_command_leaves_caret_at_start_of_deletion (the c off-by-1 reproduction). --- PLAN.md | 12 +++ crates/fresnel-core/src/actor.rs | 126 ++++++++++++++++++++++++++++--- crates/fresnel-core/src/lib.rs | 42 ++++++++--- 3 files changed, 161 insertions(+), 19 deletions(-) diff --git a/PLAN.md b/PLAN.md index 2fdb71b..630da48 100644 --- a/PLAN.md +++ b/PLAN.md @@ -327,6 +327,18 @@ into the harder case. ignored bucket doubles as the Phase 1 backlog: each `#[ignore]` reason names a subsystem, so `cargo test -- --ignored` prints the exact list of gaps. +- ~~Selection + cursor rendering.~~ Snapshot exposes both the primary + selection body (`selectionStart`/`selectionEnd`) and the block-cursor + grapheme range (`cursorStart`/`cursorEnd`), computed off Helix's own + `Range::cursor(text)` — the same accessor helix-term's `CursorCache` + uses. Frontend draws a selection-body tint under a stronger block + cursor highlight in normal/select mode, and a caret via a + `border-left` on the character at the cursor position in insert mode + (no `inline-block` gaps, no monospace-grid drift). Fixes an off-by-1 + after `c`: `Selection::ensure_invariants` width-1-expands a collapsed + range post-delete, so the raw `head` sat one grapheme past the actual + visual position; `Range::cursor` returns the right index in all + modes. - Not-yet-wired in the dispatch loop: the `.` repeat operator, macros (`Q`/`q`), and the pseudo-pending state (per-command hinting). - One real known bug: `2[u` (add newline above twice, then undo) diff --git a/crates/fresnel-core/src/actor.rs b/crates/fresnel-core/src/actor.rs index 21428f4..a85ed17 100644 --- a/crates/fresnel-core/src/actor.rs +++ b/crates/fresnel-core/src/actor.rs @@ -34,17 +34,20 @@ use crate::{EditorSession, KeyOutcome}; #[serde(rename_all = "camelCase")] pub struct Snapshot { pub text: String, - /// Range of characters the primary cursor visually covers. + /// Character range covered by the primary selection's *body*: + /// `(min(anchor, head), max(anchor, head))`. In normal/select mode + /// this is what the frontend renders as a highlight; in insert mode + /// it collapses to a point (same value as `cursor_start`/ + /// `cursor_end`). + pub selection_start: usize, + pub selection_end: usize, + /// Character range the primary cursor visually covers as a *block*. /// /// In `normal` and `select` modes this is a width-1 grapheme range /// following Helix's block-cursor semantics — the character to - /// highlight, not the raw `head` offset. In `insert` mode the range - /// collapses to `head..head` (a zero-width caret to draw between - /// characters). - /// - /// A frontend that just wants "where's the cursor?" can use - /// `cursorStart`; the pair is only needed for rendering the block - /// cursor correctly across grapheme clusters. + /// highlight distinctly from the surrounding selection body. In + /// `insert` mode the range collapses to `head..head` (a zero-width + /// caret to draw between characters). pub cursor_start: usize, pub cursor_end: usize, /// Serialized in Helix's own textual form (`normal`, `select`, `insert`). @@ -60,8 +63,11 @@ pub struct Snapshot { impl Snapshot { fn of(session: &EditorSession) -> Self { let (cursor_start, cursor_end) = session.primary_cursor_range(); + let (selection_start, selection_end) = session.primary_selection_range(); Self { text: session.current_text(), + selection_start, + selection_end, cursor_start, cursor_end, mode: mode_to_str(session.mode()).to_owned(), @@ -309,6 +315,92 @@ mod tests { assert!(response.snapshot.cursor_start > before.cursor_start); } + #[test] + fn word_selection_reports_selection_body_range() { + // `w` selects the next word (or up to it, per Helix semantics), + // so the primary selection spans multiple graphemes and its + // `[selectionStart, selectionEnd)` should be strictly wider than + // the width-1 block cursor. + let handle = spawn("hello world\n".into()); + let response = handle.handle_key("w").expect("valid key"); + let snap = response.snapshot; + let body_width = snap.selection_end - snap.selection_start; + let cursor_width = snap.cursor_end - snap.cursor_start; + assert!( + body_width > cursor_width, + "expected selection body to be wider than the block cursor after `w`; \ + body=[{},{}) cursor=[{},{})", + snap.selection_start, + snap.selection_end, + snap.cursor_start, + snap.cursor_end, + ); + // And the block cursor should sit inside the selection body. + assert!(snap.cursor_start >= snap.selection_start); + assert!(snap.cursor_end <= snap.selection_end); + } + + #[test] + fn change_command_leaves_caret_at_start_of_deletion() { + // `c` on a forward-facing selection deletes the range and enters + // insert mode. The resulting caret must sit at the *start* of + // the deleted region, not after it — otherwise typed characters + // land in the wrong place. In wire terms: cursorStart == + // cursorEnd == the char index where the selection began. + // + // Regression coverage for "cursor is especially weird with c, + // like it technically works correctly but visually it's off by + // 1": the underlying position was always right; verify by + // asserting the wire values here so the frontend has a stable + // contract to render against. + let handle = spawn("hello world\n".into()); + // Move to "world" and grab the whole word: `w` walks to just + // past `hello ` (head=6 or 7 depending on Helix's word-motion + // semantics), and no further keys are needed — the primary + // selection already spans multiple graphemes. + handle.handle_key("w").unwrap(); + let before_c = handle.snapshot(); + // Sanity: `w` must have produced a non-collapsed selection body + // for the test to be meaningful. + assert!( + before_c.selection_end > before_c.selection_start, + "w should leave a multi-char selection: {:?}", + before_c, + ); + let sel_start = before_c.selection_start; + let sel_end = before_c.selection_end; + // `c` should delete the selection body and enter insert mode + // with a zero-width caret at sel_start. + let after_c = handle.handle_key("c").expect("c is valid").snapshot; + assert_eq!(after_c.mode, "insert"); + assert_eq!(after_c.cursor_start, after_c.cursor_end); + assert_eq!( + after_c.cursor_start, sel_start, + "caret should sit at the start of the deleted region; \ + pre-c selection was [{sel_start},{sel_end})", + ); + } + + #[test] + fn insert_mode_reports_zero_width_cursor_at_head() { + // Helix's `insert_mode` command doesn't collapse the underlying + // Range — it flips it so the head lands where insertions will + // happen, leaving `anchor` behind. The visual cursor in insert + // mode is a zero-width caret at `head`, so `cursorStart == + // cursorEnd == head`. The `selectionStart`/`selectionEnd` pair + // still reflects the raw Range body — but a frontend rendering + // insert mode should ignore the selection body and only draw + // the caret. See renderSnapshot in app/src/main.ts. + let handle = spawn("hello\n".into()); + let response = handle.handle_key("i").expect("valid key"); + let snap = response.snapshot; + assert_eq!(snap.mode, "insert"); + assert_eq!(snap.cursor_start, snap.cursor_end); + // In insert mode after `i`, the head sits before the previous + // block cursor position. + assert_eq!(snap.cursor_start, 0); + } + #[test] fn handle_key_reports_pending_sequence_for_g() { let handle = spawn("abc\n".into()); @@ -343,13 +435,27 @@ mod tests { let snap = handle.snapshot(); let json = serde_json::to_value(&snap).expect("snapshot serializes"); let obj = json.as_object().expect("snapshot is a JSON object"); - for expected_field in ["text", "cursorStart", "cursorEnd", "mode", "pendingKeys"] { + for expected_field in [ + "text", + "selectionStart", + "selectionEnd", + "cursorStart", + "cursorEnd", + "mode", + "pendingKeys", + ] { assert!( obj.contains_key(expected_field), "expected `{expected_field}` field, got: {json}" ); } - for forbidden_field in ["cursor_start", "cursor_end", "pending_keys"] { + for forbidden_field in [ + "selection_start", + "selection_end", + "cursor_start", + "cursor_end", + "pending_keys", + ] { assert!( !obj.contains_key(forbidden_field), "expected snake_case `{forbidden_field}` to NOT appear, got: {json}" diff --git a/crates/fresnel-core/src/lib.rs b/crates/fresnel-core/src/lib.rs index 94919ff..196d452 100644 --- a/crates/fresnel-core/src/lib.rs +++ b/crates/fresnel-core/src/lib.rs @@ -147,24 +147,48 @@ impl EditorSession { doc.selection(view.id).primary().head } - /// Character range the primary cursor *visually* covers, using Helix's - /// block-cursor semantics: in normal/select mode this is - /// `[cursor_start, cursor_end)` where `cursor_start` is the grapheme - /// boundary immediately before `head`; in insert mode the range - /// collapses to `head..head` (a zero-width caret between characters). + /// Character range the primary cursor *visually* covers, using + /// Helix's block-cursor semantics. + /// + /// Both branches use `Range::cursor(text)` — the same accessor + /// helix-term's own render path calls + /// (`helix_view::editor::CursorCache::get`). This matters because a + /// forward-facing selection's `head` sits *past* the character the + /// cursor visually covers, and Helix's invariants ensure even a + /// "collapsed" range has width ≥ 1 after `ensure_invariants` — so + /// using `head` directly would put the caret one grapheme too far + /// right after commands like `c` that delete a range and enter + /// insert mode. See the `change_command_leaves_caret_at_start_of_deletion` + /// test. + /// + /// In normal/select mode we return `[cursor..next_grapheme)`, a + /// width-1 grapheme range. In insert mode we return the collapsed + /// `(cursor, cursor)` — a zero-width caret between characters. pub fn primary_cursor_range(&self) -> (usize, usize) { let (view, doc) = helix_view::current_ref!(&self.editor); let text = doc.text().slice(..); let range = doc.selection(view.id).primary(); + let cursor = range.cursor(text); if self.editor.mode() == Mode::Insert { - (range.head, range.head) + (cursor, cursor) } else { - let start = range.cursor(text); - let end = helix_core::graphemes::next_grapheme_boundary(text, start); - (start, end) + let end = helix_core::graphemes::next_grapheme_boundary(text, cursor); + (cursor, end) } } + /// Character range the primary selection covers as a whole — + /// `(min(anchor, head), max(anchor, head))`. In normal/select mode + /// this is the *selection body* the frontend should highlight; the + /// block cursor from `primary_cursor_range` sits inside (or at the + /// edge of) this range. In insert mode the selection collapses to a + /// point, so `start == end == head`. + pub fn primary_selection_range(&self) -> (usize, usize) { + let (view, doc) = helix_view::current_ref!(&self.editor); + let range = doc.selection(view.id).primary(); + (range.from(), range.to()) + } + /// The full primary-view selection. Used by tests that need to compare /// against Helix's `test::plain` DSL output; production code should /// prefer a more focused accessor. -- 2.51.2