From bd728c470563eaede92e4c1d9dcaafe60337b03b Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 10 Jun 2026 10:50:11 -0400 Subject: [PATCH] =?UTF-8?q?CUA=20region=20semantics=20=E2=80=94=20shift=20?= =?UTF-8?q?selection,=20region-aware=20delete=20+=20type-over?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - S-/S-/S- (+C-S word/paragraph variants) extend a selection; the TUI grid paints it reverse-video; double-click selects the word at point. - Backspace / Delete consume the active region (delete_region first, falling back to single-codepoint semantics). - Typing replaces the region: buffer.self-insert / newline / tab delete_region before inserting. pmacs-gpu cooperates by round-tripping keys while an own-window selection is active, so the region-aware commands run instead of a raw optimistic op. - tests/cua_region_acceptance.rs drives the real dispatch path: select -> BS/DEL/char/Enter, plus the no-region fallbacks. Co-Authored-By: Claude Fable 5 --- builtin/commands/default.lua | 43 ++++-- builtin/keymaps/default.lua | 5 +- src/editor.rs | 241 ++++++++++++++++++++++++++++++++- src/editor_core.rs | 32 +++++ tests/cua_region_acceptance.rs | 139 +++++++++++++++++++ 5 files changed, 444 insertions(+), 16 deletions(-) create mode 100644 tests/cua_region_acceptance.rs diff --git a/builtin/commands/default.lua b/builtin/commands/default.lua index 68872c3..f210f39 100644 --- a/builtin/commands/default.lua +++ b/builtin/commands/default.lua @@ -40,10 +40,20 @@ cmd { name = "cursor.paragraph-down", -- Buffer editing ------------------------------------------------------------- -cmd { name = "buffer.delete-backward", description = "Delete the codepoint before the cursor.", - fn = function() ed.backspace() end } -cmd { name = "buffer.delete-forward", description = "Delete the codepoint at the cursor.", - fn = function() ed.delete_forward() end } +-- CUA region semantics: with an active selection, Backspace / Delete +-- consume the region (cursor lands at its start, selection clears). +-- `delete_region` returns false when no region is active, so the +-- single-codepoint behavior is untouched outside selections. +cmd { name = "buffer.delete-backward", + description = "Delete the active region, or the codepoint before the cursor.", + fn = function() + if not ed.delete_region() then ed.backspace() end + end } +cmd { name = "buffer.delete-forward", + description = "Delete the active region, or the codepoint at the cursor.", + fn = function() + if not ed.delete_region() then ed.delete_forward() end + end } cmd { name = "buffer.delete-word-backward", description = "Delete from the cursor back to the start of the previous word.", fn = function() ed.delete_word_backward() end } @@ -79,18 +89,31 @@ cmd { name = "cursor.select-word-left", cmd { name = "cursor.select-word-right", description = "Extend selection by one word right.", fn = function() ensure_anchor(); ed.move_word_right() end } +cmd { name = "cursor.select-paragraph-up", + description = "Extend selection to the previous paragraph break.", + fn = function() ensure_anchor(); ed.move_paragraph_up() end } +cmd { name = "cursor.select-paragraph-down", + description = "Extend selection to the next paragraph break.", + fn = function() ensure_anchor(); ed.move_paragraph_down() end } cmd { name = "cursor.select-line-start", description = "Extend selection to start of line.", fn = function() ensure_anchor(); ed.move_line_start() end } cmd { name = "cursor.select-line-end", description = "Extend selection to end of line.", fn = function() ensure_anchor(); ed.move_line_end() end } -cmd { name = "buffer.newline", description = "Insert a newline at the cursor.", - fn = function() ed.insert_char(10) end } -cmd { name = "buffer.tab", description = "Insert a tab at the cursor.", - fn = function() ed.insert_char(9) end } -cmd { name = "buffer.self-insert", description = "Insert the codepoint argument at the cursor.", - fn = function(codepoint) ed.insert_char(codepoint) end } +-- CUA type-over: inserting with an active selection replaces it +-- (`delete_region` is a no-op without one). The pmacs-gpu frontend +-- relies on this: its optimistic-insert path detects an own-window +-- selection and round-trips the key so these commands run. +cmd { name = "buffer.newline", + description = "Insert a newline at the cursor, replacing the active region.", + fn = function() ed.delete_region(); ed.insert_char(10) end } +cmd { name = "buffer.tab", + description = "Insert a tab at the cursor, replacing the active region.", + fn = function() ed.delete_region(); ed.insert_char(9) end } +cmd { name = "buffer.self-insert", + description = "Insert the codepoint argument at the cursor, replacing the active region.", + fn = function(codepoint) ed.delete_region(); ed.insert_char(codepoint) end } -- History -------------------------------------------------------------------- diff --git a/builtin/keymaps/default.lua b/builtin/keymaps/default.lua index e2b01ad..16adab5 100644 --- a/builtin/keymaps/default.lua +++ b/builtin/keymaps/default.lua @@ -73,7 +73,8 @@ bind("M-d", "buffer.delete-word-forward") -- CUA-style Shift+motion selection. Each Shift+arrow extends a -- selection from the cursor (anchoring at the current position if no -- region is yet active). Ctrl+Shift+Left/Right extend by whole words; --- Shift+Home/End extend to line edges. Plain motion (without Shift) +-- Ctrl+Shift+Up/Down extend by paragraphs; Shift+Home/End extend to +-- line edges. Plain motion (without Shift) -- is unchanged --- it preserves any existing selection rather than -- dropping it (Emacs-flavored default; users who want strict-CUA -- "drop-on-plain-motion" can rebind their motion commands). @@ -85,6 +86,8 @@ bind("S-", "cursor.select-line-start") bind("S-", "cursor.select-line-end") bind("C-S-", "cursor.select-word-left") bind("C-S-", "cursor.select-word-right") +bind("C-S-", "cursor.select-paragraph-up") +bind("C-S-", "cursor.select-paragraph-down") -- Undo / redo ---------------------------------------------------------------- -- diff --git a/src/editor.rs b/src/editor.rs index 6b20371..d5999bf 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -16,7 +16,7 @@ use std::cell::RefCell; use std::io; use std::path::PathBuf; use std::rc::Rc; -use std::time::Duration; +use std::time::{Duration, Instant}; use crossterm::event::{KeyCode, KeyModifiers}; @@ -97,8 +97,21 @@ pub struct EditorState { /// Snippet store (T M4.11). Co-owned with the snippet /// provider closure inside [`Self::completion_registry`]. pub snippets: crate::completion_framework::SharedSnippetRegistry, + /// Last left-button down event, used to synthesize terminal double + /// clicks from crossterm's plain Down/Up mouse event stream. + mouse_click: Option, } +#[derive(Copy, Clone)] +struct MouseClickState { + frontend_id: FrontendId, + window_id: WindowId, + cell: CellCoord, + at: Instant, +} + +const DOUBLE_CLICK_MAX_DELAY: Duration = Duration::from_millis(500); + impl EditorState { /// Construct a fresh editor for an unnamed scratch buffer. /// @@ -322,6 +335,7 @@ impl EditorState { project_indexer, completion_registry, snippets, + mouse_click: None, } } @@ -662,6 +676,8 @@ impl EditorState { /// positions the buffer cursor at the corresponding rope /// position. Starts an empty selection at that position so /// a drag continues the region from there. + /// * A second `Down(Left)` in the same cell within the double-click + /// threshold selects the word at the click position. /// * `Drag(Left)` updates the cursor as the mouse moves; the /// anchor stays put, so the region grows. /// * `Up(Left)` ends a drag. If anchor and cursor coincide @@ -699,14 +715,28 @@ impl EditorState { match ev.kind { MouseEventKind::Down(MouseButton::Left) => { if local_row >= inner_rows { + self.mouse_click = None; return; // Mode-line click: reserved. } + let click_cell = CellCoord::new(cell_row, cell_col); + let is_double_click = self.is_double_click(frontend_id, win_id, click_cell); self.activate_and_position(win_id, local_row, local_col); - let mut core = self.core.borrow_mut(); - let pos = core.cursor(); - core.begin_selection(pos); + if is_double_click && self.core.borrow_mut().select_word_at_cursor() { + self.mouse_click = None; + } else { + let mut core = self.core.borrow_mut(); + let pos = core.cursor(); + core.begin_selection(pos); + self.mouse_click = Some(MouseClickState { + frontend_id, + window_id: win_id, + cell: click_cell, + at: Instant::now(), + }); + } } MouseEventKind::Drag(MouseButton::Left) => { + self.mouse_click = None; if local_row >= inner_rows { return; } @@ -721,15 +751,34 @@ impl EditorState { } } MouseEventKind::ScrollUp => { + self.mouse_click = None; self.scroll_window(win_id, -SCROLL_LINES); } MouseEventKind::ScrollDown => { + self.mouse_click = None; self.scroll_window(win_id, SCROLL_LINES); } - _ => {} + _ => { + self.mouse_click = None; + } } } + fn is_double_click( + &self, + frontend_id: FrontendId, + window_id: WindowId, + cell: CellCoord, + ) -> bool { + let Some(prev) = self.mouse_click else { + return false; + }; + prev.frontend_id == frontend_id + && prev.window_id == window_id + && prev.cell == cell + && prev.at.elapsed() <= DOUBLE_CLICK_MAX_DELAY + } + /// Make `win_id` the active window and place its cursor at the /// buffer position corresponding to `(local_row, local_col)`, /// where the coordinates are relative to the window's viewport @@ -1110,6 +1159,7 @@ pub fn paint_frame( for overlay in &mut window.overlays { overlay.render(buf, viewport, grid); } + paint_local_selection(grid, buf, window, &rect, inner_rows); // Mode line for this window. Painted last so the line // itself is always visible regardless of overlay activity. let coord = window @@ -1199,6 +1249,62 @@ fn inner_rows(rect: &crate::window::Rect) -> u32 { rect.size.rows.saturating_sub(1) } +fn paint_local_selection( + grid: &mut crate::cell::CellGrid<'_>, + buf: &crate::buffer::Buffer, + window: &crate::window::Window, + rect: &crate::window::Rect, + inner_rows: u32, +) { + let Some((sel_start, sel_end)) = window.region() else { + return; + }; + if inner_rows == 0 || rect.size.cols == 0 || sel_start >= sel_end { + return; + } + + let first_row = window.view_top; + let last_row = first_row.saturating_add(inner_rows as usize); + for display_row in first_row..last_row { + let Some(line_start) = window.text_view.line_offset(display_row) else { + continue; + }; + let Some(line_len) = window.text_view.line_len(buf, display_row) else { + continue; + }; + let line_end = line_start.saturating_add(line_len); + let paint_start = sel_start.max(line_start); + let paint_end = sel_end.min(line_end); + if paint_start >= paint_end { + continue; + } + + let Some(start_coord) = window.text_view.pos_to_display(buf, paint_start) else { + continue; + }; + let Some(end_coord) = window.text_view.pos_to_display(buf, paint_end) else { + continue; + }; + if start_coord.row as usize != display_row || end_coord.row as usize != display_row { + continue; + } + + let row_offset = display_row.saturating_sub(first_row) as u32; + let start_col = start_coord.col.min(rect.size.cols); + let end_col = end_coord.col.min(rect.size.cols); + if start_col >= end_col { + continue; + } + for col in start_col..end_col { + let cell = grid.at(CellCoord::new( + rect.origin.row + row_offset, + rect.origin.col + col, + )); + cell.style.reverse = true; + } + } +} + #[allow( clippy::too_many_arguments, reason = "the mode line packs eight unrelated facts; bundling them into a struct just adds ceremony" @@ -4055,6 +4161,94 @@ mod tests { assert!(core.active_region().is_none()); } + #[test] + fn mouse_drag_selection_paints_in_tui_grid() { + use crossterm::event::{MouseButton, MouseEventKind}; + let mut s = fresh_with(b"hello world\n"); + s.dispatch_mouse( + FrontendId::LOCAL, + mouse(MouseEventKind::Down(MouseButton::Left), 0, 0), + term_size_24x80(), + ); + s.dispatch_mouse( + FrontendId::LOCAL, + mouse(MouseEventKind::Drag(MouseButton::Left), 0, 5), + term_size_24x80(), + ); + + let (cells, _, _) = render_to_grid(&s, 24, 80); + for col in 0..5 { + let style = cells[col as usize].style; + assert!(style.reverse, "selected col {col} was not reverse video"); + } + assert!( + !cells[5].style.reverse, + "unselected cell after mouse selection was reverse video" + ); + } + + #[test] + fn mouse_double_click_selects_word_and_paints_in_tui_grid() { + use crossterm::event::{MouseButton, MouseEventKind}; + let mut s = fresh_with(b"hello world\n"); + + s.dispatch_mouse( + FrontendId::LOCAL, + mouse(MouseEventKind::Down(MouseButton::Left), 0, 7), + term_size_24x80(), + ); + s.dispatch_mouse( + FrontendId::LOCAL, + mouse(MouseEventKind::Up(MouseButton::Left), 0, 7), + term_size_24x80(), + ); + s.dispatch_mouse( + FrontendId::LOCAL, + mouse(MouseEventKind::Down(MouseButton::Left), 0, 7), + term_size_24x80(), + ); + s.dispatch_mouse( + FrontendId::LOCAL, + mouse(MouseEventKind::Up(MouseButton::Left), 0, 7), + term_size_24x80(), + ); + + assert_eq!(s.core.borrow().cursor(), 11); + assert_eq!(s.core.borrow().active_region(), Some((6, 11))); + + let (cells, _, _) = render_to_grid(&s, 24, 80); + assert!(!cells[5].style.reverse, "selection leaked into separator"); + for col in 6..11 { + assert!( + cells[col as usize].style.reverse, + "double-click selected word missing col {col}" + ); + } + assert!(!cells[11].style.reverse, "selection leaked past word"); + } + + #[test] + fn mouse_double_click_on_separator_leaves_no_region() { + use crossterm::event::{MouseButton, MouseEventKind}; + let mut s = fresh_with(b"hello world\n"); + + for _ in 0..2 { + s.dispatch_mouse( + FrontendId::LOCAL, + mouse(MouseEventKind::Down(MouseButton::Left), 0, 5), + term_size_24x80(), + ); + s.dispatch_mouse( + FrontendId::LOCAL, + mouse(MouseEventKind::Up(MouseButton::Left), 0, 5), + term_size_24x80(), + ); + } + + assert_eq!(s.core.borrow().cursor(), 5); + assert!(s.core.borrow().active_region().is_none()); + } + /// Acceptance bullet 3: mouse events are coalesced at frame /// boundaries — many drag events between renders all apply, and /// the cursor ends up at the last position. @@ -4317,6 +4511,43 @@ mod tests { assert_eq!(s.core.borrow().cursor(), 14); } + #[test] + fn shift_arrow_extends_selection_and_paints_in_tui_grid() { + let mut s = fresh_with(b"abcdef\n"); + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Right, KeyModifiers::SHIFT)); + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Right, KeyModifiers::SHIFT)); + + assert_eq!(s.core.borrow().cursor(), 2); + assert_eq!(s.core.borrow().active_region(), Some((0, 2))); + + let (cells, stride, _) = render_to_grid(&s, 24, 80); + assert!(cells[0].style.reverse, "selection did not paint col 0"); + assert!(cells[1].style.reverse, "selection did not paint col 1"); + assert!(!cells[2].style.reverse, "selection leaked into col 2"); + assert_eq!(glyph_at(&cells, stride, 0, 0), 'a'); + assert_eq!(glyph_at(&cells, stride, 0, 1), 'b'); + } + + #[test] + fn ctrl_shift_arrow_extends_selection_by_words_and_paragraphs() { + let mut s = fresh_with(b"alpha beta\n\nsecond\n"); + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Right, KeyModifiers::CONTROL | KeyModifiers::SHIFT), + ); + assert_eq!(s.core.borrow().cursor(), 5); + assert_eq!(s.core.borrow().active_region(), Some((0, 5))); + + s.core.borrow_mut().active_window_mut().cursor = 0; + s.core.borrow_mut().clear_selection(); + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Down, KeyModifiers::CONTROL | KeyModifiers::SHIFT), + ); + assert_eq!(s.core.borrow().cursor(), 11); + assert_eq!(s.core.borrow().active_region(), Some((0, 11))); + } + #[test] fn page_down_advances_cursor_and_view_top() { let mut content = Vec::new(); diff --git a/src/editor_core.rs b/src/editor_core.rs index af54432..24798ca 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -757,6 +757,28 @@ impl EditorCore { aw.goal_col = None; } + /// Select the word at the active cursor. Returns `false` when the + /// cursor is not on a word character. + pub fn select_word_at_cursor(&mut self) -> bool { + let id = self.active_buffer_id(); + let cursor = self.active_window().cursor; + let range = { + let reg = self.registry.borrow(); + let Ok(buffer) = reg.get(id) else { + return false; + }; + word_range_at(buffer, cursor) + }; + let Some((start, end)) = range else { + return false; + }; + let aw = self.active_window_mut(); + aw.selection = Some(crate::window::Selection { anchor: start }); + aw.cursor = end; + aw.goal_col = None; + true + } + /// Move the cursor forward to the next paragraph break. /// /// A paragraph break is a blank line (empty or whitespace-only). @@ -1383,6 +1405,16 @@ fn forward_word(buf: &Buffer, mut pos: Position) -> Position { pos } +fn word_range_at(buf: &Buffer, pos: Position) -> Option<(Position, Position)> { + let (ch, _) = char_at(buf, pos)?; + if !is_word_char(ch) { + return None; + } + let start = backward_word(buf, pos); + let end = forward_word(buf, pos); + (start < end).then_some((start, end)) +} + /// True iff `line` is empty or contains only ASCII whitespace. /// Used by paragraph motion: a blank line is a paragraph break. fn line_is_blank(buf: &Buffer, view: &TextView, line: usize) -> bool { diff --git a/tests/cua_region_acceptance.rs b/tests/cua_region_acceptance.rs new file mode 100644 index 0000000..717e402 --- /dev/null +++ b/tests/cua_region_acceptance.rs @@ -0,0 +1,139 @@ +//! CUA region semantics — Backspace / Delete consume the active +//! selection (set by Shift+motion) before falling back to their +//! single-codepoint behavior. +//! +//! Regression for the pmacs-gpu report "select a region with +//! shift+arrows then backspace doesn't delete as expected": the +//! `buffer.delete-backward` / `buffer.delete-forward` commands called +//! straight into the single-codepoint core primitives and never +//! consulted `active_region()`. The behavior is frontend-agnostic +//! (the GPU round-trips BS through the same dispatch), so the TUI +//! dispatch path exercised here covers both. + +use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers}; +use pmacs::editor::EditorState; +use pmacs::protocol::FrontendId; + +fn key(code: KeyCode, mods: KeyModifiers) -> KeyEvent { + KeyEvent { + code, + modifiers: mods, + kind: KeyEventKind::Press, + state: KeyEventState::empty(), + } +} + +fn type_str(s: &mut EditorState, text: &str) { + for ch in text.chars() { + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char(ch), KeyModifiers::NONE), + ); + } +} + +/// `(buffer text, region active?, cursor)` probed through the Lua +/// surface — the same introspection a user-facing script would use. +fn probe(s: &EditorState) -> (String, bool, i64) { + s.lua_host + .lua() + .load( + " + local b = pmacs.window.buffer() + local text = b:slice(0, b:len()) + return text, pmacs.editor.region() ~= nil, pmacs.editor.cursor() + ", + ) + .eval() + .expect("probe buffer state") +} + +#[test] +fn backspace_deletes_the_shift_selected_region() { + let mut s = EditorState::new(); + type_str(&mut s, "hello"); + + // Shift+Left three times: region [2, 5), cursor at 2. + for _ in 0..3 { + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Left, KeyModifiers::SHIFT)); + } + let (_, region_active, _) = probe(&s); + assert!(region_active, "shift+arrows must leave an active region"); + + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Backspace, KeyModifiers::NONE)); + + let (text, region_active, cursor) = probe(&s); + assert_eq!(text, "he", "backspace must delete the whole region"); + assert!(!region_active, "the region clears with its deletion"); + assert_eq!(cursor, 2, "cursor lands at the deleted region's start"); + + // Without a region, backspace keeps single-codepoint semantics. + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Backspace, KeyModifiers::NONE)); + let (text, _, cursor) = probe(&s); + assert_eq!(text, "h", "no region ⇒ plain single-codepoint backspace"); + assert_eq!(cursor, 1); +} + +#[test] +fn typing_replaces_the_shift_selected_region() { + let mut s = EditorState::new(); + type_str(&mut s, "hello"); + + // Select "llo" (region [2, 5), cursor at 2), then type 'X': + // CUA type-over replaces the region with the typed char. + for _ in 0..3 { + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Left, KeyModifiers::SHIFT)); + } + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char('X'), KeyModifiers::SHIFT), + ); + + let (text, region_active, cursor) = probe(&s); + assert_eq!(text, "heX", "typing must replace the selected region"); + assert!(!region_active, "the region is consumed by the replacement"); + assert_eq!(cursor, 3, "cursor sits after the typed char"); + + // Enter over a selection replaces it with a newline. + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Left, KeyModifiers::SHIFT)); + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Enter, KeyModifiers::NONE)); + let (text, region_active, cursor) = probe(&s); + assert_eq!(text, "he\n", "Enter must replace the selected region"); + assert!(!region_active); + assert_eq!(cursor, 3); + + // Without a selection, typing keeps plain insert semantics. + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char('z'), KeyModifiers::NONE), + ); + let (text, _, cursor) = probe(&s); + assert_eq!(text, "he\nz", "no region ⇒ plain insert at the cursor"); + assert_eq!(cursor, 4); +} + +#[test] +fn delete_forward_deletes_the_shift_selected_region() { + let mut s = EditorState::new(); + type_str(&mut s, "world"); + + // Shift+Home-equivalent: extend left over the whole word. + for _ in 0..5 { + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Left, KeyModifiers::SHIFT)); + } + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Delete, KeyModifiers::NONE)); + + let (text, region_active, cursor) = probe(&s); + assert_eq!(text, "", "Delete must consume the whole region"); + assert!(!region_active); + assert_eq!(cursor, 0); + + // Without a region, Delete keeps forward single-codepoint semantics. + type_str(&mut s, "ab"); + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Left, KeyModifiers::NONE)); + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Left, KeyModifiers::NONE)); + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Delete, KeyModifiers::NONE)); + let (text, _, cursor) = probe(&s); + assert_eq!(text, "b", "no region ⇒ plain forward delete at cursor"); + assert_eq!(cursor, 0); +}