diff --git a/parley/src/editing/editor.rs b/parley/src/editing/editor.rs index 9582ef628..504278c66 100644 --- a/parley/src/editing/editor.rs +++ b/parley/src/editing/editor.rs @@ -22,6 +22,44 @@ use crate::layout::LayoutAccessibility; #[cfg(feature = "accesskit")] use accesskit::{Node, NodeId, TreeUpdate}; +/// The byte offset of the last grapheme-cluster boundary strictly before the end +/// of `text`, i.e. the start of the grapheme a backspace at the end should +/// remove. Returns `None` for empty text. +/// +/// Grapheme clusters are what a reader perceives as single characters, so they +/// are the unit editing operates in — as distinct from `char`s (a ZWJ emoji +/// family is seven of them) and from layout clusters (which follow the font's +/// glyphs). +fn previous_grapheme_boundary( + layout_cx: &LayoutContext, + text: &str, +) -> Option { + if text.is_empty() { + return None; + } + layout_cx + .analysis_data_sources + .grapheme_segmenter() + .segment_str(text) + .take_while(|boundary| *boundary < text.len()) + .last() +} + +/// The byte offset of the first grapheme-cluster boundary strictly after `from` +/// in `text`, i.e. the end of the grapheme a forward delete at `from` should +/// remove. Returns `None` at the end of the text. +fn next_grapheme_boundary( + layout_cx: &LayoutContext, + text: &str, + from: usize, +) -> Option { + layout_cx + .analysis_data_sources + .grapheme_segmenter() + .segment_str(text) + .find(|boundary| *boundary > from) +} + /// Opaque representation of a generation. /// /// Obtained from [`PlainEditor::generation`]. @@ -243,16 +281,14 @@ where /// Delete the selection or the next cluster (typical ‘delete’ behavior). pub fn delete(&mut self) { if self.editor.selection.is_collapsed() { - // Upstream cluster range - if let Some(range) = self - .editor - .selection - .focus() - .logical_clusters(&self.editor.layout)[1] - .as_ref() - .map(|cluster| cluster.text_range()) - .and_then(|range| (!range.is_empty()).then_some(range)) - { + // Delete one grapheme cluster forward, mirroring `backdelete`. The + // downstream layout cluster is not the right unit: it follows the + // font's glyphs, so it splits a grapheme wherever the font does not + // ligate it — and, unlike `backdelete`, this path never even had the + // newline special-case, so it could split a CRLF pair in two. + let start = self.editor.selection.focus().index(); + if let Some(end) = next_grapheme_boundary(self.layout_cx, &self.editor.buffer, start) { + let range = start..end; self.editor.buffer.replace_range(range.clone(), ""); self.editor.update_compose_for_replaced_range(range, 0); self.update_layout(); @@ -295,20 +331,23 @@ where { let range = cluster.text_range(); let end = range.end; - let start = if cluster.is_hard_line_break() || cluster.is_emoji() { - // For newline sequences and emoji, delete the previous cluster - range.start - } else { - // Otherwise, delete the previous character - let Some((start, _)) = self - .editor - .buffer - .get(..end) - .and_then(|str| str.char_indices().next_back()) - else { - return; - }; - start + // Delete one *grapheme cluster*: that is the unit a reader + // perceives as a character, and what backspace is expected to + // remove. Deleting one `char` instead peels a grapheme apart one + // codepoint at a time — a ZWJ emoji family takes seven presses, + // a regional-indicator flag two, and even `e` + a combining + // acute two. + // + // The layout cluster is not usable for this: it comes from + // shaping, so it tracks glyphs rather than perceived characters + // and splits wherever the font does not ligate the sequence. + let Some(start) = self + .editor + .buffer + .get(..end) + .and_then(|text| previous_grapheme_boundary(self.layout_cx, text)) + else { + return; }; self.editor.buffer.replace_range(start..end, ""); self.editor.update_compose_for_replaced_range(start..end, 0); diff --git a/parley_tests/tests/editor.rs b/parley_tests/tests/editor.rs index 25dfc8bfa..4d79c3b14 100644 --- a/parley_tests/tests/editor.rs +++ b/parley_tests/tests/editor.rs @@ -116,3 +116,79 @@ fn editor_insert_regular_text_set_upstream_affinity() { assert_eq!(sel.focus().index(), 2); assert_eq!(sel.focus().affinity(), Affinity::Upstream); } + +/// Backspace must delete exactly one grapheme cluster — the unit a reader +/// perceives as a character — not one `char`. +/// +/// The cases are deliberately not all emoji: `e` + a combining acute and a +/// Hangul syllable built from jamo are ordinary text that was equally affected, +/// which is why this is grapheme segmentation rather than an emoji special case. +#[test] +fn editor_backdelete_deletes_one_grapheme() { + let mut env = TestEnv::new(test_name!(), None); + for (name, text) in [ + // Family: man, ZWJ, woman, ZWJ, girl, ZWJ, boy — one grapheme of 7 chars. + ("zwj family", "\u{1F468}\u{200D}\u{1F469}\u{200D}\u{1F467}\u{200D}\u{1F466}"), + // Flag: two regional indicators. + ("regional indicator flag", "\u{1F1EF}\u{1F1F5}"), + // Heart plus the emoji presentation selector. + ("emoji presentation selector", "\u{2764}\u{FE0F}"), + // Waving hand plus a skin-tone modifier. + ("skin tone modifier", "\u{1F44B}\u{1F3FD}"), + // Not emoji: `e` plus a combining acute accent. + ("combining mark", "e\u{301}"), + // Not emoji: Hangul jamo composing one syllable. + ("hangul jamo", "\u{1100}\u{1161}\u{11A8}"), + // Not emoji: a CRLF pair is a single grapheme. + ("crlf", "\r\n"), + ] { + let mut editor = env.editor(text); + { + let mut drv = env.driver(&mut editor); + drv.move_to_text_end(); + drv.backdelete(); + } + assert_eq!(editor.text().to_string(), "", "backdelete: {name}"); + } +} + +/// Forward delete must likewise remove exactly one grapheme cluster, mirroring +/// [`editor_backdelete_deletes_one_grapheme`]. +#[test] +fn editor_delete_deletes_one_grapheme() { + let mut env = TestEnv::new(test_name!(), None); + for (name, text) in [ + ("zwj family", "\u{1F468}\u{200D}\u{1F469}\u{200D}\u{1F467}\u{200D}\u{1F466}"), + ("regional indicator flag", "\u{1F1EF}\u{1F1F5}"), + ("emoji presentation selector", "\u{2764}\u{FE0F}"), + ("skin tone modifier", "\u{1F44B}\u{1F3FD}"), + ("combining mark", "e\u{301}"), + ("hangul jamo", "\u{1100}\u{1161}\u{11A8}"), + ("crlf", "\r\n"), + ] { + let mut editor = env.editor(text); + { + let mut drv = env.driver(&mut editor); + drv.move_to_text_start(); + drv.delete(); + } + assert_eq!(editor.text().to_string(), "", "delete: {name}"); + } +} + +/// Deleting must still stop at each grapheme rather than swallowing the line: +/// two separate graphemes take two presses in either direction. +#[test] +fn editor_delete_stops_at_grapheme_boundaries() { + let mut env = TestEnv::new(test_name!(), None); + + let mut editor = env.editor("ab"); + env.driver(&mut editor).move_to_text_end(); + env.driver(&mut editor).backdelete(); + assert_eq!(editor.text().to_string(), "a"); + + let mut editor = env.editor("ab"); + env.driver(&mut editor).move_to_text_start(); + env.driver(&mut editor).delete(); + assert_eq!(editor.text().to_string(), "b"); +}