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.<ret>`)
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.
This commit is contained in:
Pascal Kuthe 2024-08-09 17:40:34 +02:00 committed by GitHub
parent 7e85fd5b77
commit e604d9f8e0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 32 additions and 16 deletions

View file

@ -184,16 +184,16 @@ impl Range {
let positions_to_map = match self.anchor.cmp(&self.head) { let positions_to_map = match self.anchor.cmp(&self.head) {
Ordering::Equal => [ Ordering::Equal => [
(&mut self.anchor, Assoc::After), (&mut self.anchor, Assoc::AfterSticky),
(&mut self.head, Assoc::After), (&mut self.head, Assoc::AfterSticky),
], ],
Ordering::Less => [ Ordering::Less => [
(&mut self.anchor, Assoc::After), (&mut self.anchor, Assoc::AfterSticky),
(&mut self.head, Assoc::Before), (&mut self.head, Assoc::BeforeSticky),
], ],
Ordering::Greater => [ Ordering::Greater => [
(&mut self.head, Assoc::After), (&mut self.head, Assoc::AfterSticky),
(&mut self.anchor, Assoc::Before), (&mut self.anchor, Assoc::BeforeSticky),
], ],
}; };
changes.update_positions(positions_to_map.into_iter()); changes.update_positions(positions_to_map.into_iter());
@ -482,16 +482,16 @@ impl Selection {
range.old_visual_position = None; range.old_visual_position = None;
match range.anchor.cmp(&range.head) { match range.anchor.cmp(&range.head) {
Ordering::Equal => [ Ordering::Equal => [
(&mut range.anchor, Assoc::After), (&mut range.anchor, Assoc::AfterSticky),
(&mut range.head, Assoc::After), (&mut range.head, Assoc::AfterSticky),
], ],
Ordering::Less => [ Ordering::Less => [
(&mut range.anchor, Assoc::After), (&mut range.anchor, Assoc::AfterSticky),
(&mut range.head, Assoc::Before), (&mut range.head, Assoc::BeforeSticky),
], ],
Ordering::Greater => [ Ordering::Greater => [
(&mut range.head, Assoc::After), (&mut range.head, Assoc::AfterSticky),
(&mut range.anchor, Assoc::Before), (&mut range.anchor, Assoc::BeforeSticky),
], ],
} }
}); });

View file

@ -29,6 +29,12 @@ pub enum Assoc {
/// Acts like `Before` if a word character is inserted /// Acts like `Before` if a word character is inserted
/// before the position, otherwise acts like `After` /// before the position, otherwise acts like `After`
BeforeWord, 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 { impl Assoc {
@ -40,13 +46,17 @@ impl Assoc {
fn insert_offset(self, s: &str) -> usize { fn insert_offset(self, s: &str) -> usize {
let chars = s.chars().count(); let chars = s.chars().count();
match self { match self {
Assoc::After => chars, Assoc::After | Assoc::AfterSticky => chars,
Assoc::AfterWord => s.chars().take_while(|&c| char_is_word(c)).count(), Assoc::AfterWord => s.chars().take_while(|&c| char_is_word(c)).count(),
// return position before inserted text // 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(), 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)] #[derive(Debug, Default, Clone, PartialEq, Eq)]
@ -456,9 +466,15 @@ impl ChangeSet {
if pos == old_pos && assoc.stay_at_gaps() { if pos == old_pos && assoc.stay_at_gaps() {
new_pos new_pos
} else { } else {
// place to end of insert 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) new_pos + assoc.insert_offset(s)
} }
}
}), }),
i i
); );