fix(editor): edit complete grapheme clusters (#115)

* docs(editor): define grapheme editing semantics

* fix(editor): edit complete grapheme clusters

* perf(editor): bound grapheme rendering work

Author: owainlewis

Cargo.lock

@@ -24,3 +24,5 @@ tree-sitter-toml-ng = "0.7.0"
 tree-sitter-typescript = "0.23.2"
 unicode-casefold = "0.2.0"
 unicode-normalization = "0.1.25"
+unicode-segmentation = "1.13.3"
+unicode-width = "0.2.2"

Cargo.toml

@@ -0,0 +1,60 @@
+# Issue 63 Plan
+
+## Task
+
+Make extended grapheme clusters the user-visible editing and rendering unit.
+Preserve current macOS editor behavior while ensuring Unicode text is never split at a user-perceived character boundary.
+
+## Product Policy
+
+Point remains stored as a Rope character index, but it may rest only at an extended grapheme cluster boundary.
+Horizontal movement advances or retreats by one extended grapheme cluster.
+Backward and forward deletion remove one complete extended grapheme cluster.
+Mark and region endpoints are grapheme boundaries, so selection, cut, yank, undo, and redo never split a cluster.
+Vertical movement preserves the preferred terminal cell column and chooses the last grapheme boundary that does not pass that column.
+Horizontal clipping includes a complete grapheme cluster or excludes it.
+Combining marks, variation selectors, emoji modifiers, regional-indicator flags, and ZWJ sequences remain attached to their cluster.
+Terminal width comes from the maintained `unicode-width` implementation and is applied to the complete cluster.
+Normal text clusters occupy the reported one or two terminal cells.
+A standalone cluster reported as zero cells occupies one editor cell so it remains visible and cursor-addressable.
+Tabs keep the existing four-column tab-stop behavior.
+Control clusters keep the existing visible-space behavior.
+
+## Implementation
+
+1. Add maintained Unicode grapheme segmentation and terminal-width dependencies.
+2. Add small shared helpers for grapheme boundaries, display width, clipping, and terminal-column lookup.
+3. Make `View` movement and point clamping grapheme-safe.
+4. Make backward and forward deletion operate on complete grapheme ranges.
+5. Keep insertion, region cutting, yanking, undo, redo, search, and reload points on grapheme boundaries.
+6. Build renderer segments and retained cells from complete grapheme clusters.
+7. Remove the temporary context-dependent full-redraw fallback once grapheme cell coordinates are reliable.
+8. Document the policy in the PRD and mark issue #63 complete in the roadmap after implementation is verified.
+
+## Acceptance Criteria
+
+- Decomposed accents render with their combining marks and move or delete as one unit.
+- ZWJ emoji, regional-indicator flags, skin-tone sequences, and variation-selector sequences move, select, delete, undo, and render as complete units.
+- Wide CJK graphemes occupy two cells and are never clipped halfway.
+- Cursor placement uses complete-cluster widths after movement, editing, selection, and renderer transitions.
+- Vertical movement never places point inside a grapheme cluster.
+- Diff rendering skips unchanged grapheme cells and safely transitions between Unicode frames.
+- Existing ASCII editing, tabs, syntax styles, picker behavior, resize handling, and terminal cleanup remain unchanged.
+
+## Checks
+
+- Run focused pure tests for combining marks, ZWJ emoji, flags, skin tones, variation selectors, wide CJK, selection, delete, undo, cursor placement, clipping, and renderer transitions.
+- Run `cargo fmt -- --check`.
+- Run `git diff origin/main...HEAD --check`.
+- Run `cargo test --no-fail-fast`.
+- Run `cargo clippy --all-targets --all-features -- -D warnings`.
+- Run `cargo build --all-targets --all-features`.
+- Run a synchronized PTY smoke that edits and navigates representative Unicode text, resizes, exits, and confirms shell restoration.
+
+## Out of Scope
+
+Unicode normalization is out of scope.
+Word and sentence movement are out of scope.
+Bidirectional text layout is out of scope.
+Soft wrapping and horizontal scrolling are out of scope.
+Configurable Unicode width policies are out of scope.

docs/issues/63-plan.md

@@ -174,6 +174,8 @@ Both a *file window* and a *terminal pane* are leaves in the same window-layout
 | Terminal I/O | `crossterm` | Raw mode, key events, 24-bit truecolor, cursor control, cross-platform. |
 | Rendering | hand-rolled on crossterm | Editors need fine cursor control + minimal diffed repaints. (`ratatui` optional for chrome only — modeline/minibuffer.) |
 | Text buffer | `ropey` | O(log n) edits and slices on million-line files; cheap to view-slice for rendering. |
+| Text boundaries | `unicode-segmentation` | Extended grapheme clusters are the user-visible editing unit. |
+| Display width | `unicode-width` | Complete grapheme clusters map to terminal cells without splitting Unicode sequences. |
 | Syntax highlighting | `tree-sitter` + `tree-sitter-highlight` | Incremental, fast, LSP-independent; the core aesthetic win. |
 | Fuzzy finding | `nucleo` | Helix's matcher; powers find-file and buffer switching. |
 | File watching | `notify` | inotify/FSEvents/etc. for the (optional, later) auto-reload path. |
@@ -192,11 +194,16 @@ Both a *file window* and a *terminal pane* are leaves in the same window-layout
 - Backed by a `ropey::Rope`. (A `Vec<String>` is acceptable for the very first prototype, but `ropey` is the intended default and the migration is cheap.)
 - Tracks: the rope, a dirty flag, the file path, and file metadata needed for save/reload behavior.
 - Point, mark, and scroll offset belong to the window/editor view state, not solely to the buffer.
+- Point and mark use Rope character indices but may rest only at extended grapheme cluster boundaries.
+- Horizontal movement, deletion, selection, kill, yank, undo, and redo operate on complete extended grapheme clusters.
+- Vertical movement preserves terminal cell columns and never places point inside a grapheme cluster.
 - Multiple buffers held in a buffer list; one is active.
 - Undo/redo: an edit-history stack (group keystrokes into sensible undo units; Emacs-style undo can come later).
 
 ### 6.2 Renderer
 - **Diffed paint.** Maintain a shadow of the last drawn screen; only emit escape codes for cells that changed. This is the difference between flicker and butter, and it directly serves the latency goal.
+- Display width is measured for complete grapheme clusters, and clipping includes or excludes each cluster as a whole.
+- Tabs keep four-column tab stops, normal clusters use their reported one or two cells, and standalone zero-width clusters use one visible editor cell.
 - Viewport: scroll offset + visible range derived from point.
 - Line wrapping handled explicitly (soft-wrap toggle is a later nicety).
 - Generous padding; block/bar cursor styles; truecolor throughout.

docs/prd.md

@@ -208,7 +208,7 @@ Tracking issues:
 - [closed] [#60 Preserve file permissions and define symlink save behavior](https://github.com/owainlewis/cortex/issues/60)
 - [closed] [#61 Remove whole-buffer work from input and render paths](https://github.com/owainlewis/cortex/issues/61)
 - [closed] [#62 Preserve syntax highlight context across viewport boundaries](https://github.com/owainlewis/cortex/issues/62)
-- [open] [#63 Render Unicode grapheme clusters and combining marks correctly](https://github.com/owainlewis/cortex/issues/63)
+- [closed] [#63 Render Unicode grapheme clusters and combining marks correctly](https://github.com/owainlewis/cortex/issues/63)
 - [closed] [#64 Mark final-newline-only changes in the gutter](https://github.com/owainlewis/cortex/issues/64)
 
 ## Repository Quality

docs/roadmap.md

@@ -7,6 +7,7 @@ use crate::{
     picker::{DirectoryPicker, DirectoryPickerAction},
     renderer::{Renderer, StatusKind, TerminalSize},
     terminal::TerminalSession,
+    text,
     view::View,
 };
 use crossterm::{
@@ -351,8 +352,8 @@ impl AppState {
 
     fn active_region(&self, buffer: &Buffer, view: &View) -> Option<Range<usize>> {
         let len_chars = buffer.len_chars();
-        let mark = self.mark?.min(len_chars);
-        let point = view.point().min(len_chars);
+        let mark = buffer.grapheme_boundary_at_or_before(self.mark?.min(len_chars));
+        let point = buffer.grapheme_boundary_at_or_before(view.point().min(len_chars));
 
         if mark == point {
             return None;
@@ -374,8 +375,8 @@ impl AppState {
         };
 
         let text = buffer.text_range(region.clone());
-        buffer.delete_with_points(region.clone(), view.point(), region.start);
-        view.set_point(region.start, buffer);
+        let point_after = buffer.delete_with_points(region.clone(), view.point(), region.start);
+        view.set_point(point_after, buffer);
         self.kill_ring = Some(text);
         self.mark = None;
         self.set_status("Cut region", StatusKind::Success);
@@ -390,8 +391,8 @@ impl AppState {
         };
 
         let text = buffer.text_range(region.clone());
-        buffer.delete_with_points(region, point, point);
-        view.set_point(point, buffer);
+        let point_after = buffer.delete_with_points(region, point, point);
+        view.set_point(point_after, buffer);
         self.kill_ring = Some(text);
         self.mark = None;
         self.set_status("Cut line", StatusKind::Success);
@@ -405,8 +406,8 @@ impl AppState {
         };
 
         let point = view.point();
-        buffer.insert(point, &text);
-        view.set_point(point + text.chars().count(), buffer);
+        let point_after = buffer.insert(point, &text);
+        view.set_point(point_after, buffer);
         self.mark = None;
         self.set_status("Yanked", StatusKind::Success);
         AppAction::Continue
@@ -427,7 +428,7 @@ impl AppState {
             }
             crate::input::Key::Backspace => {
                 if let Some(input) = self.command_line.as_mut() {
-                    input.pop();
+                    text::pop_grapheme(input);
                 }
                 AppAction::Continue
             }
@@ -534,7 +535,7 @@ impl AppState {
             return AppAction::Continue;
         };
 
-        let start = view.point().saturating_add(1).min(buffer.len_chars());
+        let start = buffer.next_grapheme_boundary(view.point());
         self.find_search_match(buffer, view, &query, start)
     }
 
@@ -752,7 +753,7 @@ fn kill_line_range(buffer: &Buffer, point: usize) -> Option<Range<usize>> {
     if point < line_end {
         Some(point..line_end)
     } else {
-        Some(point..point + 1)
+        Some(point..buffer.next_grapheme_boundary(point))
     }
 }
 
@@ -1012,6 +1013,24 @@ mod tests {
         assert!(!buffer.is_dirty());
     }
 
+    #[test]
+    fn command_line_backspace_removes_one_complete_grapheme() {
+        let mut app = AppState::default();
+        let mut keymap = Keymap::new();
+        let mut buffer = buffer_with_text("notes.txt", "old");
+        let mut view = View::new();
+
+        app.handle_key(Key::Meta('x'), &mut keymap, &mut buffer, &mut view);
+        for ch in "e\u{301}👨‍💻".chars() {
+            app.handle_key(Key::Char(ch), &mut keymap, &mut buffer, &mut view);
+        }
+
+        app.handle_key(Key::Backspace, &mut keymap, &mut buffer, &mut view);
+        assert_eq!(app.command_line.as_deref(), Some("/e\u{301}"));
+        app.handle_key(Key::Backspace, &mut keymap, &mut buffer, &mut view);
+        assert_eq!(app.command_line.as_deref(), Some("/"));
+    }
+
     #[test]
     fn bare_slash_and_help_commands_list_available_commands() {
         for command in ["/", "/help", "/commands"] {
@@ -1101,6 +1120,62 @@ mod tests {
         assert_eq!(app.status_message.as_deref(), Some("Cut region"));
     }
 
+    #[test]
+    fn selection_cut_yank_and_undo_keep_graphemes_whole() {
+        let mut app = AppState::default();
+        let mut keymap = Keymap::new();
+        let mut buffer = buffer_with_text("notes.txt", "a👨‍💻🇺🇸b");
+        let mut view = View::new();
+
+        app.handle_key(Key::Ctrl('f'), &mut keymap, &mut buffer, &mut view);
+        app.handle_key(Key::Ctrl(' '), &mut keymap, &mut buffer, &mut view);
+        app.handle_key(Key::Ctrl('f'), &mut keymap, &mut buffer, &mut view);
+        app.handle_key(Key::Ctrl('f'), &mut keymap, &mut buffer, &mut view);
+        assert_eq!(app.active_region(&buffer, &view), Some(1..6));
+
+        app.handle_key(Key::Ctrl('w'), &mut keymap, &mut buffer, &mut view);
+        assert_eq!(buffer.text(), "ab");
+        assert_eq!(app.kill_ring.as_deref(), Some("👨‍💻🇺🇸"));
+        assert_eq!(view.point(), 1);
+
+        app.handle_key(Key::Ctrl('y'), &mut keymap, &mut buffer, &mut view);
+        assert_eq!(buffer.text(), "a👨‍💻🇺🇸b");
+        assert_eq!(view.point(), 6);
+
+        app.handle_key(Key::Command('z'), &mut keymap, &mut buffer, &mut view);
+        assert_eq!(buffer.text(), "ab");
+        assert_eq!(view.point(), 1);
+    }
+
+    #[test]
+    fn cut_and_history_keep_point_after_graphemes_merged_by_the_edit() {
+        let mut app = AppState::default();
+        let mut keymap = Keymap::new();
+        let mut buffer = buffer_with_text("notes.txt", "🇦x🇧");
+        let mut view = View::new();
+
+        app.handle_key(Key::Ctrl('f'), &mut keymap, &mut buffer, &mut view);
+        app.handle_key(Key::Ctrl(' '), &mut keymap, &mut buffer, &mut view);
+        app.handle_key(Key::Ctrl('f'), &mut keymap, &mut buffer, &mut view);
+        app.handle_key(Key::Ctrl('w'), &mut keymap, &mut buffer, &mut view);
+        assert_eq!(buffer.text(), "🇦🇧");
+        assert_eq!(view.point(), 2);
+
+        app.dispatch_command(crate::commands::Command::Undo, &mut buffer, &mut view);
+        assert_eq!(buffer.text(), "🇦x🇧");
+        assert_eq!(view.point(), 2);
+        app.dispatch_command(crate::commands::Command::Redo, &mut buffer, &mut view);
+        assert_eq!(buffer.text(), "🇦🇧");
+        assert_eq!(view.point(), 2);
+
+        let mut buffer = buffer_with_text("notes.txt", "e\n\u{301}x");
+        let mut view = View::new();
+        view.move_forward_char(&buffer);
+        app.handle_key(Key::Ctrl('k'), &mut keymap, &mut buffer, &mut view);
+        assert_eq!(buffer.text(), "e\u{301}x");
+        assert_eq!(view.point(), 2);
+    }
+
     #[test]
     fn editing_after_mark_clears_the_region() {
         let mut app = AppState::default();