From 6ce41f8d0d9aeed920724e059cb3f6ac5ed5a7e4 Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Mon, 9 Dec 2024 12:17:59 +0100 Subject: [PATCH] Fix `Cursor::previous_logical_word` Currently, in `foo b|ar` where `|` indicates the cursor position, `Cursor::previous_logical_word` will return `foo| bar`. This doesn't match the behavior of `Cursor::{next_logical_word, previous_visual_word}` which place the cursor at the boundary of the current word. This happens because `Cluster::previous_logical_word` is called from the upstream cluster (the "b") and the cursor then lands between the "o" and the space. This also renames "left", "right" -> "upstream", "downstream" (naming copied from `Cursor::logical_clusters`) to make it clear we're operating in logical order and not visual, and sets `moving_right` based on the text direction we're jumping to, to affine the cursor towards the word whose boundary it lands on. (Note the behavior between `{previous, next}_logical_word` and {previous, next}_visual_word` aren't quite the same yet: the `visual` methods jump over whitespace, the `logical` ones don't.) --- parley/src/layout/cursor.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/parley/src/layout/cursor.rs b/parley/src/layout/cursor.rs index d25c1d6f..c4edb457 100644 --- a/parley/src/layout/cursor.rs +++ b/parley/src/layout/cursor.rs @@ -243,14 +243,15 @@ impl Cursor { /// in logical order. #[must_use] pub fn next_logical_word(&self, layout: &Layout) -> Self { - let [left, right] = self.logical_clusters(layout); - if let Some(cluster) = right.or(left) { + let [upstream, downstream] = self.logical_clusters(layout); + if let Some(cluster) = downstream.or(upstream) { let start = cluster.clone(); let cluster = cluster.next_logical_word().unwrap_or(cluster); if cluster.path == start.path { return Self::from_byte_index(layout, usize::MAX, Affinity::Downstream); } - return Self::from_cluster(layout, cluster, true); + let moving_right = !cluster.is_rtl(); + return Self::from_cluster(layout, cluster, moving_right); } *self } @@ -259,10 +260,11 @@ impl Cursor { /// in logical order. #[must_use] pub fn previous_logical_word(&self, layout: &Layout) -> Self { - let [left, right] = self.logical_clusters(layout); - if let Some(cluster) = left.or(right) { + let [upstream, downstream] = self.logical_clusters(layout); + if let Some(cluster) = downstream.or(upstream) { let cluster = cluster.previous_logical_word().unwrap_or(cluster); - return Self::from_cluster(layout, cluster, true); + let moving_right = cluster.is_rtl(); + return Self::from_cluster(layout, cluster, moving_right); } *self }