From e604d9f8e0fea2223a357be7c9dc6088daef47e7 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Fri, 9 Aug 2024 17:40:34 +0200 Subject: [PATCH] keep (cursor) position when exactly replacing text (#5930) Whenever a document is changed helix maps various positions like the cursor or diagnostics through the `ChangeSet` applied to the document. Currently, this mapping handles replacements as follows: * Move position to the left for `Assoc::Before` (start of selection) * Move position to the right for `Assoc::After` (end of selection) However, when text is exactly replaced this can produce weird results where the cursor is moved when it shouldn't. For example if `foo` is selected and a separate cursor is placed on each character (`s.`) and the text is replaced (for example `rx`) then the cursors are moved to the side instead of remaining in place. This change adds a special case to the mapping code of replacements: If the deleted and inserted text have the same (char) length then the position is returned as if the replacement doesn't exist. only keep selections invariant under replacement Keeping selections unchanged if they are inside an exact replacement is intuitive. However, for diagnostics this is not desirable as helix would otherwise fail to remove diagnostics if replacing parts of the document. --- helix-core/src/selection.rs | 24 ++++++++++++------------ helix-core/src/transaction.rs | 24 ++++++++++++++++++++---- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index eff1fcd70..a382a7186 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -184,16 +184,16 @@ pub fn map(mut self, changes: &ChangeSet) -> Self { let positions_to_map = match self.anchor.cmp(&self.head) { Ordering::Equal => [ - (&mut self.anchor, Assoc::After), - (&mut self.head, Assoc::After), + (&mut self.anchor, Assoc::AfterSticky), + (&mut self.head, Assoc::AfterSticky), ], Ordering::Less => [ - (&mut self.anchor, Assoc::After), - (&mut self.head, Assoc::Before), + (&mut self.anchor, Assoc::AfterSticky), + (&mut self.head, Assoc::BeforeSticky), ], Ordering::Greater => [ - (&mut self.head, Assoc::After), - (&mut self.anchor, Assoc::Before), + (&mut self.head, Assoc::AfterSticky), + (&mut self.anchor, Assoc::BeforeSticky), ], }; changes.update_positions(positions_to_map.into_iter()); @@ -482,16 +482,16 @@ pub fn map_no_normalize(mut self, changes: &ChangeSet) -> Self { range.old_visual_position = None; match range.anchor.cmp(&range.head) { Ordering::Equal => [ - (&mut range.anchor, Assoc::After), - (&mut range.head, Assoc::After), + (&mut range.anchor, Assoc::AfterSticky), + (&mut range.head, Assoc::AfterSticky), ], Ordering::Less => [ - (&mut range.anchor, Assoc::After), - (&mut range.head, Assoc::Before), + (&mut range.anchor, Assoc::AfterSticky), + (&mut range.head, Assoc::BeforeSticky), ], Ordering::Greater => [ - (&mut range.head, Assoc::After), - (&mut range.anchor, Assoc::Before), + (&mut range.head, Assoc::AfterSticky), + (&mut range.anchor, Assoc::BeforeSticky), ], } }); diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index f24f20942..c5c94b750 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -29,6 +29,12 @@ pub enum Assoc { /// Acts like `Before` if a word character is inserted /// before the position, otherwise acts like `After` BeforeWord, + /// Acts like `Before` but if the position is within an exact replacement + /// (exact size) the offset to the start of the replacement is kept + BeforeSticky, + /// Acts like `After` but if the position is within an exact replacement + /// (exact size) the offset to the start of the replacement is kept + AfterSticky, } impl Assoc { @@ -40,13 +46,17 @@ fn stay_at_gaps(self) -> bool { fn insert_offset(self, s: &str) -> usize { let chars = s.chars().count(); match self { - Assoc::After => chars, + Assoc::After | Assoc::AfterSticky => chars, Assoc::AfterWord => s.chars().take_while(|&c| char_is_word(c)).count(), // return position before inserted text - Assoc::Before => 0, + Assoc::Before | Assoc::BeforeSticky => 0, Assoc::BeforeWord => chars - s.chars().rev().take_while(|&c| char_is_word(c)).count(), } } + + pub fn sticky(self) -> bool { + matches!(self, Assoc::BeforeSticky | Assoc::AfterSticky) + } } #[derive(Debug, Default, Clone, PartialEq, Eq)] @@ -456,8 +466,14 @@ macro_rules! map { if pos == old_pos && assoc.stay_at_gaps() { new_pos } else { - // place to end of insert - new_pos + assoc.insert_offset(s) + let ins = assoc.insert_offset(s); + // if the deleted and inserted text have the exact same size + // keep the relative offset into the new text + if *len == ins && assoc.sticky() { + new_pos + (pos - old_pos) + } else { + new_pos + assoc.insert_offset(s) + } } }), i