Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

keep (cursor) position when exactly replacing text #5930

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
keep (cursor) position when exactly replacing text
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.
  • Loading branch information
pascalkuthe committed Apr 2, 2024
commit cea759fca5ff6dab8b6fb4389c10e9e06dc592e5
24 changes: 12 additions & 12 deletions helix-core/src/selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,16 +177,16 @@ impl Range {

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());
Expand Down Expand Up @@ -469,16 +469,16 @@ impl Selection {
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),
],
}
});
Expand Down
24 changes: 20 additions & 4 deletions helix-core/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -40,13 +46,17 @@ impl Assoc {
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)]
Expand Down Expand Up @@ -451,8 +461,14 @@ impl ChangeSet {
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
Expand Down
Loading