From 0ae522f3df433bb778fa2ff98fd3d7047021c6ef Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Mon, 28 Jun 2021 11:40:07 -0700 Subject: Clean up `Selection` to not use so many allocations. --- helix-term/src/commands.rs | 187 +++++++++++++++------------------------------ 1 file changed, 62 insertions(+), 125 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index fa251ff0..29a10c76 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -310,48 +310,44 @@ impl PartialEq for Command { fn move_char_left(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { movement::move_horizontally(text, range, Direction::Backward, count, Movement::Move) }); - doc.set_selection(view.id, selection); } fn move_char_right(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { movement::move_horizontally(text, range, Direction::Forward, count, Movement::Move) }); - doc.set_selection(view.id, selection); } fn move_line_up(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { movement::move_vertically(text, range, Direction::Backward, count, Movement::Move) }); - doc.set_selection(view.id, selection); } fn move_line_down(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { movement::move_vertically(text, range, Direction::Forward, count, Movement::Move) }); - doc.set_selection(view.id, selection); } fn move_line_end(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { let line = text.char_to_line(range.head); let pos = line_end_char_index(&text.slice(..), line); @@ -360,30 +356,26 @@ fn move_line_end(cx: &mut Context) { Range::new(pos, pos) }); - - doc.set_selection(view.id, selection); } fn move_line_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { let line = text.char_to_line(range.head); // adjust to start of the line let pos = text.line_to_char(line); Range::new(pos, pos) }); - - doc.set_selection(view.id, selection); } fn move_first_nonwhitespace(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { let line_idx = text.char_to_line(range.head); if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) { @@ -393,8 +385,6 @@ fn move_first_nonwhitespace(cx: &mut Context) { range } }); - - doc.set_selection(view.id, selection); } // TODO: move vs extend could take an extra type Extend/Move that would @@ -404,73 +394,49 @@ fn move_first_nonwhitespace(cx: &mut Context) { fn move_next_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc - .selection(view.id) - .transform(|range| movement::move_next_word_start(text, range, count)); - - doc.set_selection(view.id, selection); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| movement::move_next_word_start(text, range, count)); } fn move_prev_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc - .selection(view.id) - .transform(|range| movement::move_prev_word_start(text, range, count)); - - doc.set_selection(view.id, selection); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| movement::move_prev_word_start(text, range, count)); } fn move_next_word_end(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc - .selection(view.id) - .transform(|range| movement::move_next_word_end(text, range, count)); - - doc.set_selection(view.id, selection); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| movement::move_next_word_end(text, range, count)); } fn move_next_long_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - - let selection = doc - .selection(view.id) - .transform(|range| movement::move_next_long_word_start(text, range, count)); - doc.set_selection(view.id, selection); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| movement::move_next_long_word_start(text, range, count)); } fn move_prev_long_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc - .selection(view.id) - .transform(|range| movement::move_prev_long_word_start(text, range, count)); - - doc.set_selection(view.id, selection); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| movement::move_prev_long_word_start(text, range, count)); } fn move_next_long_word_end(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - - let selection = doc - .selection(view.id) - .transform(|range| movement::move_next_long_word_end(text, range, count)); - doc.set_selection(view.id, selection); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| movement::move_next_long_word_end(text, range, count)); } fn move_file_start(cx: &mut Context) { @@ -490,42 +456,37 @@ fn move_file_end(cx: &mut Context) { fn extend_next_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|mut range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|mut range| { let word = movement::move_next_word_start(text, range, count); let pos = word.head; Range::new(range.anchor, pos) }); - - doc.set_selection(view.id, selection); } fn extend_prev_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|mut range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|mut range| { let word = movement::move_prev_word_start(text, range, count); let pos = word.head; Range::new(range.anchor, pos) }); - doc.set_selection(view.id, selection); } fn extend_next_word_end(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|mut range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|mut range| { let word = movement::move_next_word_end(text, range, count); let pos = word.head; Range::new(range.anchor, pos) }); - - doc.set_selection(view.id, selection); } #[inline] @@ -568,9 +529,9 @@ where }; let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|mut range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|mut range| { search_fn(text, ch, range.head, count, inclusive).map_or(range, |pos| { if extend { Range::new(range.anchor, pos) @@ -581,8 +542,6 @@ where // or (pos, pos) to move to found val }) }); - - doc.set_selection(view.id, selection); }) } @@ -661,8 +620,8 @@ fn extend_prev_char(cx: &mut Context) { fn extend_first_nonwhitespace(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { let line_idx = text.char_to_line(range.head); if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) { @@ -672,8 +631,6 @@ fn extend_first_nonwhitespace(cx: &mut Context) { range } }); - - doc.set_selection(view.id, selection); } fn replace(cx: &mut Context) { @@ -784,48 +741,44 @@ fn half_page_down(cx: &mut Context) { fn extend_char_left(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { movement::move_horizontally(text, range, Direction::Backward, count, Movement::Extend) }); - doc.set_selection(view.id, selection); } fn extend_char_right(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { movement::move_horizontally(text, range, Direction::Forward, count, Movement::Extend) }); - doc.set_selection(view.id, selection); } fn extend_line_up(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { movement::move_vertically(text, range, Direction::Backward, count, Movement::Extend) }); - doc.set_selection(view.id, selection); } fn extend_line_down(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { movement::move_vertically(text, range, Direction::Forward, count, Movement::Extend) }); - doc.set_selection(view.id, selection); } fn extend_line_end(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { let line = text.char_to_line(range.head); let pos = line_end_char_index(&text.slice(..), line); @@ -834,23 +787,19 @@ fn extend_line_end(cx: &mut Context) { Range::new(range.anchor, pos) }); - - doc.set_selection(view.id, selection); } fn extend_line_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { let line = text.char_to_line(range.head); // adjust to start of the line let pos = text.line_to_char(line); Range::new(range.anchor, pos) }); - - doc.set_selection(view.id, selection); } fn select_all(cx: &mut Context) { @@ -1043,20 +992,16 @@ fn change_selection(cx: &mut Context) { fn collapse_selection(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc - .selection(view.id) - .transform(|range| Range::new(range.head, range.head)); - doc.set_selection(view.id, selection); + doc.selection_mut(view.id) + .transform(|range| Range::new(range.head, range.head)); } fn flip_selections(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc - .selection(view.id) - .transform(|range| Range::new(range.head, range.anchor)); - doc.set_selection(view.id, selection); + doc.selection_mut(view.id) + .transform(|range| Range::new(range.head, range.anchor)); } fn enter_insert_mode(doc: &mut Document) { @@ -1068,10 +1013,8 @@ fn insert_mode(cx: &mut Context) { let (view, doc) = current!(cx.editor); enter_insert_mode(doc); - let selection = doc - .selection(view.id) + doc.selection_mut(view.id) .transform(|range| Range::new(range.to(), range.from())); - doc.set_selection(view.id, selection); } // inserts at the end of each selection @@ -1080,8 +1023,8 @@ fn append_mode(cx: &mut Context) { enter_insert_mode(doc); doc.restore_cursor = true; - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { Range::new( range.from(), graphemes::next_grapheme_boundary(text, range.to()), // to() + next char @@ -1097,8 +1040,6 @@ fn append_mode(cx: &mut Context) { ); doc.apply(&transaction, view.id); } - - doc.set_selection(view.id, selection); } mod cmd { @@ -1904,13 +1845,12 @@ fn append_to_line(cx: &mut Context) { let (view, doc) = current!(cx.editor); enter_insert_mode(doc); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { let line = text.char_to_line(range.head); let pos = line_end_char_index(&text.slice(..), line); Range::new(pos, pos) }); - doc.set_selection(view.id, selection); } /// Sometimes when applying formatting changes we want to mark the buffer as unmodified, for @@ -2040,14 +1980,13 @@ fn normal_mode(cx: &mut Context) { // if leaving append mode, move cursor back by 1 if doc.restore_cursor { - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| { Range::new( range.from(), graphemes::prev_grapheme_boundary(text, range.to()), ) }); - doc.set_selection(view.id, selection); doc.restore_cursor = false; } @@ -2679,11 +2618,9 @@ pub mod insert { pub fn delete_word_backward(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc - .selection(view.id) - .transform(|range| movement::move_prev_word_start(text, range, count)); - doc.set_selection(view.id, selection); + + let (text, selection) = doc.text_and_mut_selection(view.id); + selection.transform(|range| movement::move_prev_word_start(text, range, count)); delete_selection(cx) } } -- cgit v1.2.3-70-g09d2 From 7c7be6d58326725954be7bd16fa3ff5e84610c17 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Wed, 30 Jun 2021 07:45:15 -0700 Subject: Make `Selection`'s normalize and transform methods self-consuming only. --- helix-core/src/object.rs | 2 +- helix-core/src/selection.rs | 25 +-- helix-term/src/commands.rs | 451 ++++++++++++++++++++++++++++---------------- helix-view/src/document.rs | 20 +- 4 files changed, 295 insertions(+), 203 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/object.rs b/helix-core/src/object.rs index 863b6e55..950b7592 100644 --- a/helix-core/src/object.rs +++ b/helix-core/src/object.rs @@ -6,7 +6,7 @@ use smallvec::smallvec; pub fn expand_selection(syntax: &Syntax, text: RopeSlice, selection: &Selection) -> Selection { let tree = syntax.tree(); - selection.clone().transformed(|range| { + selection.clone().transform(|range| { let from = text.char_to_byte(range.from()); let to = text.char_to_byte(range.to()); diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index ebc88f8b..f3119a59 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -261,7 +261,7 @@ impl Selection { pub fn push(mut self, range: Range) -> Self { self.ranges.push(range); - self.normalized() + self.normalize() } // replace_range @@ -308,7 +308,7 @@ impl Selection { } /// Normalizes a `Selection`. - pub fn normalize(&mut self) -> &mut Self { + pub fn normalize(mut self) -> Self { let primary = self.ranges[self.primary_index]; self.ranges.sort_unstable_by_key(Range::from); self.primary_index = self @@ -335,13 +335,6 @@ impl Selection { self } - /// Normalizes a `Selection`. - #[must_use] - pub fn normalized(mut self) -> Self { - self.normalize(); - self - } - // TODO: consume an iterator or a vec to reduce allocations? #[must_use] pub fn new(ranges: SmallVec<[Range; 1]>, primary_index: usize) -> Self { @@ -355,14 +348,14 @@ impl Selection { if selection.ranges.len() > 1 { // TODO: only normalize if needed (any ranges out of order) - selection.normalize(); + selection = selection.normalize(); } selection } /// Takes a closure and maps each `Range` over the closure. - pub fn transform(&mut self, f: F) -> &mut Self + pub fn transform(mut self, f: F) -> Self where F: Fn(Range) -> Range, { @@ -373,16 +366,6 @@ impl Selection { self } - /// Takes a closure and maps each `Range` over the closure. - #[must_use] - pub fn transformed(mut self, f: F) -> Self - where - F: Fn(Range) -> Range, - { - self.transform(f); - self - } - pub fn fragments<'a>(&'a self, text: RopeSlice<'a>) -> impl Iterator> + 'a { self.ranges.iter().map(move |range| range.fragment(text)) } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 29a10c76..d67c91f0 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -310,81 +310,119 @@ impl PartialEq for Command { fn move_char_left(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - movement::move_horizontally(text, range, Direction::Backward, count, Movement::Move) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_horizontally( + doc.text().slice(..), + range, + Direction::Backward, + count, + Movement::Move, + ) + }), + ); } fn move_char_right(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - movement::move_horizontally(text, range, Direction::Forward, count, Movement::Move) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_horizontally( + doc.text().slice(..), + range, + Direction::Forward, + count, + Movement::Move, + ) + }), + ); } fn move_line_up(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - movement::move_vertically(text, range, Direction::Backward, count, Movement::Move) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_vertically( + doc.text().slice(..), + range, + Direction::Backward, + count, + Movement::Move, + ) + }), + ); } fn move_line_down(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - movement::move_vertically(text, range, Direction::Forward, count, Movement::Move) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_vertically( + doc.text().slice(..), + range, + Direction::Forward, + count, + Movement::Move, + ) + }), + ); } fn move_line_end(cx: &mut Context) { let (view, doc) = current!(cx.editor); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + let text = doc.text(); + let line = text.char_to_line(range.head); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - let line = text.char_to_line(range.head); - - let pos = line_end_char_index(&text.slice(..), line); - let pos = graphemes::nth_prev_grapheme_boundary(text.slice(..), pos, 1); - let pos = range.head.max(pos).max(text.line_to_char(line)); + let pos = line_end_char_index(&text.slice(..), line); + let pos = graphemes::nth_prev_grapheme_boundary(text.slice(..), pos, 1); + let pos = range.head.max(pos).max(text.line_to_char(line)); - Range::new(pos, pos) - }); + Range::new(pos, pos) + }), + ); } fn move_line_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + let text = doc.text(); + let line = text.char_to_line(range.head); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - let line = text.char_to_line(range.head); - - // adjust to start of the line - let pos = text.line_to_char(line); - Range::new(pos, pos) - }); + // adjust to start of the line + let pos = text.line_to_char(line); + Range::new(pos, pos) + }), + ); } fn move_first_nonwhitespace(cx: &mut Context) { let (view, doc) = current!(cx.editor); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + let text = doc.text(); + let line_idx = text.char_to_line(range.head); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - let line_idx = text.char_to_line(range.head); - - if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) { - let pos = pos + text.line_to_char(line_idx); - Range::new(pos, pos) - } else { - range - } - }); + if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) { + let pos = pos + text.line_to_char(line_idx); + Range::new(pos, pos) + } else { + range + } + }), + ); } // TODO: move vs extend could take an extra type Extend/Move that would @@ -395,48 +433,67 @@ fn move_next_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| movement::move_next_word_start(text, range, count)); + doc.set_selection( + view.id, + doc.selection(view.id) + .clone() + .transform(|range| movement::move_next_word_start(doc.text().slice(..), range, count)), + ); } fn move_prev_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| movement::move_prev_word_start(text, range, count)); + doc.set_selection( + view.id, + doc.selection(view.id) + .clone() + .transform(|range| movement::move_prev_word_start(doc.text().slice(..), range, count)), + ); } fn move_next_word_end(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| movement::move_next_word_end(text, range, count)); + doc.set_selection( + view.id, + doc.selection(view.id) + .clone() + .transform(|range| movement::move_next_word_end(doc.text().slice(..), range, count)), + ); } fn move_next_long_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| movement::move_next_long_word_start(text, range, count)); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_next_long_word_start(doc.text().slice(..), range, count) + }), + ); } fn move_prev_long_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| movement::move_prev_long_word_start(text, range, count)); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_prev_long_word_start(doc.text().slice(..), range, count) + }), + ); } fn move_next_long_word_end(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| movement::move_next_long_word_end(text, range, count)); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_next_long_word_end(doc.text().slice(..), range, count) + }), + ); } fn move_file_start(cx: &mut Context) { @@ -456,37 +513,40 @@ fn move_file_end(cx: &mut Context) { fn extend_next_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|mut range| { - let word = movement::move_next_word_start(text, range, count); - let pos = word.head; - Range::new(range.anchor, pos) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|mut range| { + let word = movement::move_next_word_start(doc.text().slice(..), range, count); + let pos = word.head; + Range::new(range.anchor, pos) + }), + ); } fn extend_prev_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|mut range| { - let word = movement::move_prev_word_start(text, range, count); - let pos = word.head; - Range::new(range.anchor, pos) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|mut range| { + let word = movement::move_prev_word_start(doc.text().slice(..), range, count); + let pos = word.head; + Range::new(range.anchor, pos) + }), + ); } fn extend_next_word_end(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|mut range| { - let word = movement::move_next_word_end(text, range, count); - let pos = word.head; - Range::new(range.anchor, pos) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|mut range| { + let word = movement::move_next_word_end(doc.text().slice(..), range, count); + let pos = word.head; + Range::new(range.anchor, pos) + }), + ); } #[inline] @@ -530,18 +590,23 @@ where let (view, doc) = current!(cx.editor); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|mut range| { - search_fn(text, ch, range.head, count, inclusive).map_or(range, |pos| { - if extend { - Range::new(range.anchor, pos) - } else { - // select - Range::new(range.head, pos) - } - // or (pos, pos) to move to found val - }) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|mut range| { + search_fn(doc.text().slice(..), ch, range.head, count, inclusive).map_or( + range, + |pos| { + if extend { + Range::new(range.anchor, pos) + } else { + // select + Range::new(range.head, pos) + } + // or (pos, pos) to move to found val + }, + ) + }), + ); }) } @@ -619,18 +684,20 @@ fn extend_prev_char(cx: &mut Context) { fn extend_first_nonwhitespace(cx: &mut Context) { let (view, doc) = current!(cx.editor); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + let text = doc.text(); + let line_idx = text.char_to_line(range.head); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - let line_idx = text.char_to_line(range.head); - - if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) { - let pos = pos + text.line_to_char(line_idx); - Range::new(range.anchor, pos) - } else { - range - } - }); + if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) { + let pos = pos + text.line_to_char(line_idx); + Range::new(range.anchor, pos) + } else { + range + } + }), + ); } fn replace(cx: &mut Context) { @@ -741,65 +808,101 @@ fn half_page_down(cx: &mut Context) { fn extend_char_left(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - movement::move_horizontally(text, range, Direction::Backward, count, Movement::Extend) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_horizontally( + doc.text().slice(..), + range, + Direction::Backward, + count, + Movement::Extend, + ) + }), + ); } fn extend_char_right(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - movement::move_horizontally(text, range, Direction::Forward, count, Movement::Extend) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_horizontally( + doc.text().slice(..), + range, + Direction::Forward, + count, + Movement::Extend, + ) + }), + ); } fn extend_line_up(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - movement::move_vertically(text, range, Direction::Backward, count, Movement::Extend) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_vertically( + doc.text().slice(..), + range, + Direction::Backward, + count, + Movement::Extend, + ) + }), + ); } fn extend_line_down(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - movement::move_vertically(text, range, Direction::Forward, count, Movement::Extend) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_vertically( + doc.text().slice(..), + range, + Direction::Forward, + count, + Movement::Extend, + ) + }), + ); } fn extend_line_end(cx: &mut Context) { let (view, doc) = current!(cx.editor); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + let text = doc.text().slice(..); + let line = text.char_to_line(range.head); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - let line = text.char_to_line(range.head); - - let pos = line_end_char_index(&text.slice(..), line); - let pos = graphemes::nth_prev_grapheme_boundary(text.slice(..), pos, 1); - let pos = range.head.max(pos).max(text.line_to_char(line)); + let pos = line_end_char_index(&text, line); + let pos = graphemes::nth_prev_grapheme_boundary(text, pos, 1); + let pos = range.head.max(pos).max(text.line_to_char(line)); - Range::new(range.anchor, pos) - }); + Range::new(range.anchor, pos) + }), + ); } fn extend_line_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + let text = doc.text(); + let line = text.char_to_line(range.head); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - let line = text.char_to_line(range.head); - - // adjust to start of the line - let pos = text.line_to_char(line); - Range::new(range.anchor, pos) - }); + // adjust to start of the line + let pos = text.line_to_char(line); + Range::new(range.anchor, pos) + }), + ); } fn select_all(cx: &mut Context) { @@ -993,15 +1096,23 @@ fn change_selection(cx: &mut Context) { fn collapse_selection(cx: &mut Context) { let (view, doc) = current!(cx.editor); - doc.selection_mut(view.id) - .transform(|range| Range::new(range.head, range.head)); + doc.set_selection( + view.id, + doc.selection(view.id) + .clone() + .transform(|range| Range::new(range.head, range.head)), + ); } fn flip_selections(cx: &mut Context) { let (view, doc) = current!(cx.editor); - doc.selection_mut(view.id) - .transform(|range| Range::new(range.head, range.anchor)); + doc.set_selection( + view.id, + doc.selection(view.id) + .clone() + .transform(|range| Range::new(range.head, range.anchor)), + ); } fn enter_insert_mode(doc: &mut Document) { @@ -1013,8 +1124,12 @@ fn insert_mode(cx: &mut Context) { let (view, doc) = current!(cx.editor); enter_insert_mode(doc); - doc.selection_mut(view.id) - .transform(|range| Range::new(range.to(), range.from())); + doc.set_selection( + view.id, + doc.selection(view.id) + .clone() + .transform(|range| Range::new(range.to(), range.from())), + ); } // inserts at the end of each selection @@ -1023,15 +1138,14 @@ fn append_mode(cx: &mut Context) { enter_insert_mode(doc); doc.restore_cursor = true; - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { + let selection = doc.selection(view.id).clone().transform(|range| { Range::new( range.from(), - graphemes::next_grapheme_boundary(text, range.to()), // to() + next char + graphemes::next_grapheme_boundary(doc.text().slice(..), range.to()), // to() + next char ) }); - let end = text.len_chars(); + let end = doc.text().len_chars(); if selection.iter().any(|range| range.head == end) { let transaction = Transaction::change( @@ -1040,6 +1154,8 @@ fn append_mode(cx: &mut Context) { ); doc.apply(&transaction, view.id); } + + doc.set_selection(view.id, selection); } mod cmd { @@ -1845,12 +1961,15 @@ fn append_to_line(cx: &mut Context) { let (view, doc) = current!(cx.editor); enter_insert_mode(doc); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - let line = text.char_to_line(range.head); - let pos = line_end_char_index(&text.slice(..), line); - Range::new(pos, pos) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + let text = &doc.text().slice(..); + let line = text.char_to_line(range.head); + let pos = line_end_char_index(text, line); + Range::new(pos, pos) + }), + ); } /// Sometimes when applying formatting changes we want to mark the buffer as unmodified, for @@ -1980,13 +2099,15 @@ fn normal_mode(cx: &mut Context) { // if leaving append mode, move cursor back by 1 if doc.restore_cursor { - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| { - Range::new( - range.from(), - graphemes::prev_grapheme_boundary(text, range.to()), - ) - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + Range::new( + range.from(), + graphemes::prev_grapheme_boundary(doc.text().slice(..), range.to()), + ) + }), + ); doc.restore_cursor = false; } @@ -2619,8 +2740,12 @@ pub mod insert { let count = cx.count(); let (view, doc) = current!(cx.editor); - let (text, selection) = doc.text_and_mut_selection(view.id); - selection.transform(|range| movement::move_prev_word_start(text, range, count)); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + movement::move_prev_word_start(doc.text().slice(..), range, count) + }), + ); delete_selection(cx) } } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 59a1c42c..0f1f3a8f 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -13,8 +13,8 @@ use helix_core::{ history::History, line_ending::auto_detect_line_ending, syntax::{self, LanguageConfiguration}, - ChangeSet, Diagnostic, LineEnding, Rope, RopeBuilder, RopeSlice, Selection, State, Syntax, - Transaction, DEFAULT_LINE_ENDING, + ChangeSet, Diagnostic, LineEnding, Rope, RopeBuilder, Selection, State, Syntax, Transaction, + DEFAULT_LINE_ENDING, }; use helix_lsp::util::LspFormatting; @@ -1000,22 +1000,6 @@ impl Document { &self.selections[&view_id] } - #[inline] - pub fn selection_mut(&mut self, view_id: ViewId) -> &mut Selection { - self.selections - .get_mut(&view_id) - .expect("No selection set with the given ViewId") - } - - pub fn text_and_mut_selection(&mut self, view_id: ViewId) -> (RopeSlice, &mut Selection) { - ( - self.text.slice(..), - self.selections - .get_mut(&view_id) - .expect("No selection set with the given ViewId"), - ) - } - pub fn relative_path(&self) -> Option { let cwdir = std::env::current_dir().expect("couldn't determine current directory"); -- cgit v1.2.3-70-g09d2 From e725957704274b1ec814a34ddf6f75faf35358e7 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Thu, 1 Jul 2021 09:51:24 -0700 Subject: Ensure a minimum selection width on commands that need it. --- helix-core/src/selection.rs | 9 +++- helix-term/src/commands.rs | 128 +++++++++++++++++++++++--------------------- 2 files changed, 73 insertions(+), 64 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index f3119a59..e9dea518 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -308,7 +308,7 @@ impl Selection { } /// Normalizes a `Selection`. - pub fn normalize(mut self) -> Self { + fn normalize(mut self) -> Self { let primary = self.ranges[self.primary_index]; self.ranges.sort_unstable_by_key(Range::from); self.primary_index = self @@ -363,7 +363,12 @@ impl Selection { *range = f(*range) } - self + self.normalize() + } + + /// A convenience short-cut for `transform(|r| r.min_width_1(text))`. + pub fn min_width_1(mut self, text: RopeSlice) -> Self { + self.transform(|r| r.min_width_1(text)) } pub fn fragments<'a>(&'a self, text: RopeSlice<'a>) -> impl Iterator> + 'a { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index d67c91f0..66b05ad2 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -718,24 +718,30 @@ fn replace(cx: &mut Context) { _ => None, }; - if let Some(ch) = ch { - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let max_to = rope_end_without_line_ending(&doc.text().slice(..)); - let to = std::cmp::min(max_to, range.to() + 1); - let text: String = RopeGraphemes::new(doc.text().slice(range.from()..to)) - .map(|g| { - let cow: Cow = g.into(); - if str_is_line_ending(&cow) { - cow - } else { - ch.into() - } - }) - .collect(); + let text = doc.text().slice(..); + let selection = doc.selection(view.id).clone().min_width_1(text); - (range.from(), to, Some(text.into())) - }); + if let Some(ch) = ch { + let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + if !range.is_empty() { + let text: String = + RopeGraphemes::new(doc.text().slice(range.from()..range.to())) + .map(|g| { + let cow: Cow = g.into(); + if str_is_line_ending(&cow) { + cow + } else { + ch.into() + } + }) + .collect(); + + (range.from(), range.to(), Some(text.into())) + } else { + // No change. + (range.from(), range.to(), None) + } + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); @@ -1050,24 +1056,18 @@ fn extend_line(cx: &mut Context) { } fn delete_selection_impl(reg: &mut Register, doc: &mut Document, view_id: ViewId) { - // first yank the selection - let values: Vec = doc - .selection(view_id) - .fragments(doc.text().slice(..)) - .map(Cow::into_owned) - .collect(); + let text = doc.text().slice(..); + let selection = doc.selection(view_id).clone().min_width_1(text); + // first yank the selection + let values: Vec = selection.fragments(text).map(Cow::into_owned).collect(); reg.write(values); // then delete - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view_id), |range| { - let alltext = doc.text().slice(..); - let line = alltext.char_to_line(range.head); - let max_to = rope_end_without_line_ending(&alltext); - let to = std::cmp::min(max_to, range.to() + 1); - (range.from(), to, None) - }); + let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + let line = text.char_to_line(range.head); + (range.from(), range.to(), None) + }); doc.apply(&transaction, view_id); } @@ -1513,11 +1513,13 @@ mod cmd { match cx.editor.clipboard_provider.get_contents() { Ok(contents) => { + let selection = doc + .selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)); let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let max_to = rope_end_without_line_ending(&doc.text().slice(..)); - let to = std::cmp::min(max_to, range.to() + 1); - (range.from(), to, Some(contents.as_str().into())) + Transaction::change_by_selection(doc.text(), &selection, |range| { + (range.from(), range.to(), Some(contents.as_str().into())) }); doc.apply(&transaction, view.id); @@ -2864,17 +2866,18 @@ fn paste_impl( let mut values = values.iter().cloned().map(Tendril::from).chain(repeat); let text = doc.text(); + let selection = doc.selection(view.id).clone().min_width_1(text.slice(..)); - let transaction = Transaction::change_by_selection(text, doc.selection(view.id), |range| { + let transaction = Transaction::change_by_selection(text, &selection, |range| { let pos = match (action, linewise) { // paste linewise before (Paste::Before, true) => text.line_to_char(text.char_to_line(range.from())), // paste linewise after - (Paste::After, true) => text.line_to_char(text.char_to_line(range.to()) + 1), + (Paste::After, true) => text.line_to_char(text.char_to_line(range.to())), // paste insert (Paste::Before, false) => range.from(), // paste append - (Paste::After, false) => range.to() + 1, + (Paste::After, false) => range.to(), }; (pos, pos, Some(values.next().unwrap())) }); @@ -2914,12 +2917,17 @@ fn replace_with_yanked(cx: &mut Context) { if let Some(values) = registers.read(reg_name) { if let Some(yank) = values.first() { - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let max_to = rope_end_without_line_ending(&doc.text().slice(..)); - let to = std::cmp::min(max_to, range.to() + 1); - (range.from(), to, Some(yank.as_str().into())) - }); + let selection = doc + .selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)); + let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + if !range.is_empty() { + (range.from(), range.to(), Some(yank.as_str().into())) + } else { + (range.from(), range.to(), None) + } + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); @@ -2932,12 +2940,13 @@ fn replace_selections_with_clipboard_impl(editor: &mut Editor) { match editor.clipboard_provider.get_contents() { Ok(contents) => { - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let max_to = rope_end_without_line_ending(&doc.text().slice(..)); - let to = std::cmp::min(max_to, range.to() + 1); - (range.from(), to, Some(contents.as_str().into())) - }); + let selection = doc + .selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)); + let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + (range.from(), range.to(), Some(contents.as_str().into())) + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); @@ -3575,18 +3584,13 @@ fn surround_add(cx: &mut Context) { { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id); + let selection = doc.selection(view.id).clone().min_width_1(text); let (open, close) = surround::get_pair(ch); let mut changes = Vec::new(); for (i, range) in selection.iter().enumerate() { - let from = range.from(); - let line = text.char_to_line(range.to()); - let max_to = rope_end_without_line_ending(&text); - let to = std::cmp::min(range.to() + 1, max_to); - - changes.push((from, from, Some(Tendril::from_char(open)))); - changes.push((to, to, Some(Tendril::from_char(close)))); + changes.push((range.from(), range.from(), Some(Tendril::from_char(open)))); + changes.push((range.to(), range.to(), Some(Tendril::from_char(close)))); } let transaction = Transaction::change(doc.text(), changes.into_iter()); @@ -3612,9 +3616,9 @@ fn surround_replace(cx: &mut Context) { { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id); + let selection = doc.selection(view.id).clone().min_width_1(text); - let change_pos = match surround::get_surround_pos(text, selection, from, count) + let change_pos = match surround::get_surround_pos(text, &selection, from, count) { Some(c) => c, None => return, @@ -3646,9 +3650,9 @@ fn surround_delete(cx: &mut Context) { { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id); + let selection = doc.selection(view.id).clone().min_width_1(text); - let change_pos = match surround::get_surround_pos(text, selection, ch, count) { + let change_pos = match surround::get_surround_pos(text, &selection, ch, count) { Some(c) => c, None => return, }; -- cgit v1.2.3-70-g09d2 From 28d2d6880462509f0d3131eb2eb928bb8859e058 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Fri, 2 Jul 2021 09:51:29 -0700 Subject: Make horizontal selection movement work properly. --- helix-core/src/movement.rs | 98 ++++++++++++++++++++++++++++++++-------------- helix-term/src/commands.rs | 20 +++++++++- 2 files changed, 87 insertions(+), 31 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index b810876c..e01786eb 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -5,8 +5,11 @@ use ropey::iter::Chars; use crate::{ chars::{categorize_char, char_is_line_ending, CharCategory}, coords_at_pos, - graphemes::{nth_next_grapheme_boundary, nth_prev_grapheme_boundary}, - line_ending::{get_line_ending, line_end_char_index}, + graphemes::{ + next_grapheme_boundary, nth_next_grapheme_boundary, nth_prev_grapheme_boundary, + prev_grapheme_boundary, + }, + line_ending::get_line_ending, pos_at_coords, Position, Range, RopeSlice, }; @@ -29,25 +32,60 @@ pub fn move_horizontally( count: usize, behaviour: Movement, ) -> Range { - let pos = range.head; - let line = slice.char_to_line(pos); - // TODO: we can optimize clamping by passing in RopeSlice limited to current line. that way - // we stop calculating past start/end of line. - let pos = match dir { - Direction::Backward => { - let start = slice.line_to_char(line); - nth_prev_grapheme_boundary(slice, pos, count).max(start) + match (behaviour, dir) { + (Movement::Move, Direction::Backward) => { + let count = if range.anchor < range.head { + count + 1 + } else { + count + }; + let pos = nth_prev_grapheme_boundary(slice, range.head, count); + Range::new(pos, pos) } - Direction::Forward => { - let end_char_idx = line_end_char_index(&slice, line); - nth_next_grapheme_boundary(slice, pos, count).min(end_char_idx) + (Movement::Move, Direction::Forward) => { + let count = if range.anchor < range.head { + count - 1 + } else { + count + }; + let pos = nth_next_grapheme_boundary(slice, range.head, count); + Range::new(pos, pos) } - }; - let anchor = match behaviour { - Movement::Extend => range.anchor, - Movement::Move => pos, - }; - Range::new(anchor, pos) + (Movement::Extend, Direction::Backward) => { + // Ensure a valid initial selection state. + let range = range.min_width_1(slice); + + // Do the main movement. + let mut head = nth_prev_grapheme_boundary(slice, range.head, count); + let mut anchor = range.anchor; + + // If the head and anchor crossed over each other, we need to + // fiddle around to make it behave like a 1-wide cursor. + if head <= anchor && range.head > range.anchor { + anchor = next_grapheme_boundary(slice, anchor); + head = prev_grapheme_boundary(slice, head); + } + + Range::new(anchor, head) + } + (Movement::Extend, Direction::Forward) => { + // Ensure a valid initial selection state. + let range = range.min_width_1(slice); + + // Do the main movement. + let mut head = nth_next_grapheme_boundary(slice, range.head, count); + let mut anchor = range.anchor; + + // If the head and anchor crossed over each other, we need to + // fiddle around to make it behave like a 1-wide cursor. + if head >= anchor && range.head < range.anchor { + anchor = prev_grapheme_boundary(slice, anchor); + head = next_grapheme_boundary(slice, head); + } + + Range::new(anchor, head) + } + } } pub fn move_vertically( @@ -323,7 +361,7 @@ mod test { } #[test] - fn horizontal_moves_through_single_line_in_single_line_text() { + fn horizontal_moves_through_single_line_text() { let text = Rope::from(SINGLE_LINE_SAMPLE); let slice = text.slice(..); let position = pos_at_coords(slice, (0, 0).into()); @@ -346,7 +384,7 @@ mod test { } #[test] - fn horizontal_moves_through_single_line_in_multiline_text() { + fn horizontal_moves_through_multiline_text() { let text = Rope::from(MULTILINE_SAMPLE); let slice = text.slice(..); let position = pos_at_coords(slice, (0, 0).into()); @@ -354,15 +392,15 @@ mod test { let mut range = Range::point(position); let moves_and_expected_coordinates = IntoIter::new([ - ((Direction::Forward, 1usize), (0, 1)), // M|ultiline\n - ((Direction::Forward, 2usize), (0, 3)), // Mul|tiline\n - ((Direction::Backward, 6usize), (0, 0)), // |Multiline\n - ((Direction::Backward, 999usize), (0, 0)), // |Multiline\n - ((Direction::Forward, 3usize), (0, 3)), // Mul|tiline\n - ((Direction::Forward, 0usize), (0, 3)), // Mul|tiline\n - ((Direction::Backward, 0usize), (0, 3)), // Mul|tiline\n - ((Direction::Forward, 999usize), (0, 9)), // Multiline|\n - ((Direction::Forward, 999usize), (0, 9)), // Multiline|\n + ((Direction::Forward, 11usize), (1, 1)), // Multiline\nt|ext sample\n... + ((Direction::Backward, 1usize), (1, 0)), // Multiline\n|text sample\n... + ((Direction::Backward, 5usize), (0, 5)), // Multi|line\ntext sample\n... + ((Direction::Backward, 999usize), (0, 0)), // |Multiline\ntext sample\n... + ((Direction::Forward, 3usize), (0, 3)), // Mul|tiline\ntext sample\n... + ((Direction::Forward, 0usize), (0, 3)), // Mul|tiline\ntext sample\n... + ((Direction::Backward, 0usize), (0, 3)), // Mul|tiline\ntext sample\n... + ((Direction::Forward, 999usize), (5, 0)), // ...and whitespaced\n| + ((Direction::Forward, 999usize), (5, 0)), // ...and whitespaced\n| ]); for ((direction, amount), coordinates) in moves_and_expected_coordinates { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index e76962f0..5ab0926a 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2193,7 +2193,25 @@ fn goto_mode(cx: &mut Context) { } fn select_mode(cx: &mut Context) { - doc_mut!(cx.editor).mode = Mode::Select; + let (view, doc) = current!(cx.editor); + + // Make sure all selections are at least 1-wide. + // (With the exception of being in an empty document, of course.) + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + if range.is_empty() && range.head == doc.text().len_chars() { + Range::new( + graphemes::prev_grapheme_boundary(doc.text().slice(..), range.anchor), + range.head, + ) + } else { + range.min_width_1(doc.text().slice(..)) + } + }), + ); + + doc.mode = Mode::Select; } fn exit_select_mode(cx: &mut Context) { -- cgit v1.2.3-70-g09d2 From b4c59b444cc4963f95a95fe10f166e58ef857288 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Thu, 8 Jul 2021 16:45:19 -0700 Subject: Update surround commands to work with gap indexing. --- helix-core/src/movement.rs | 6 +- helix-core/src/search.rs | 12 +- helix-core/src/selection.rs | 2 +- helix-core/src/surround.rs | 76 +++++++----- helix-core/src/textobject.rs | 267 ++++++++++++++++++++++--------------------- helix-term/src/commands.rs | 110 +++++++----------- 6 files changed, 234 insertions(+), 239 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index 0e2a2a42..2cb4b40d 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -56,7 +56,7 @@ pub fn move_horizontally( }; // Compute the final new range. - range.put(slice, behaviour == Extend, new_pos) + range.put(slice, new_pos, behaviour == Extend) } pub fn move_vertically( @@ -106,7 +106,7 @@ pub fn move_vertically( new_pos }; - let mut new_range = range.put(slice, true, new_head); + let mut new_range = range.put(slice, new_head, true); new_range.horiz = Some(horiz); new_range } @@ -427,7 +427,7 @@ mod test { #[test] fn vertical_moves_in_single_column() { let text = Rope::from(MULTILINE_SAMPLE); - let slice = dbg!(&text).slice(..); + let slice = text.slice(..); let position = pos_at_coords(slice, (0, 0).into()); let mut range = Range::point(position); let moves_and_expected_coordinates = IntoIter::new([ diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs index 73be68c7..d4eb11a9 100644 --- a/helix-core/src/search.rs +++ b/helix-core/src/search.rs @@ -7,12 +7,11 @@ pub fn find_nth_next( n: usize, inclusive: bool, ) -> Option { - if pos >= text.len_chars() { + if pos >= text.len_chars() || n == 0 { return None; } - // start searching right after pos - let mut chars = text.chars_at(pos + 1); + let mut chars = text.chars_at(pos); for _ in 0..n { loop { @@ -40,14 +39,17 @@ pub fn find_nth_prev( n: usize, inclusive: bool, ) -> Option { - // start searching right before pos + if pos == 0 || n == 0 { + return None; + } + let mut chars = text.chars_at(pos); for _ in 0..n { loop { let c = chars.prev()?; - pos = pos.saturating_sub(1); + pos -= 1; if c == ch { break; diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 8951899b..21a6c108 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -217,7 +217,7 @@ impl Range { /// grapheme-aligned. #[must_use] #[inline] - pub fn put(self, text: RopeSlice, extend: bool, char_idx: usize) -> Range { + pub fn put(self, text: RopeSlice, char_idx: usize, extend: bool) -> Range { let anchor = if !extend { char_idx } else if self.head >= self.anchor && char_idx < self.anchor { diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 52f60cab..af357c96 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -1,3 +1,4 @@ +use crate::graphemes::next_grapheme_boundary; use crate::{search, Selection}; use ropey::RopeSlice; @@ -40,23 +41,35 @@ pub fn find_nth_pairs_pos( ) -> Option<(usize, usize)> { let (open, close) = get_pair(ch); - let (open_pos, close_pos) = if open == close { - let prev = search::find_nth_prev(text, open, pos, n, true); - let next = search::find_nth_next(text, close, pos, n, true); - if text.char(pos) == open { - // cursor is *on* a pair - next.map(|n| (pos, n)).or_else(|| prev.map(|p| (p, pos)))? + if text.len_chars() < 2 || pos >= text.len_chars() { + return None; + } + + if open == close { + if Some(open) == text.get_char(pos) { + // Special case: cursor is directly on a matching char. + match pos { + 0 => Some((pos, search::find_nth_next(text, close, pos + 1, n, true)?)), + _ if (pos + 1) == text.len_chars() => Some(( + search::find_nth_prev(text, open, pos, n, true)?, + text.len_chars(), + )), + // We return no match because there's no way to know which + // side of the char we should be searching on. + _ => None, + } } else { - (prev?, next?) + Some(( + search::find_nth_prev(text, open, pos, n, true)?, + search::find_nth_next(text, close, pos, n, true)?, + )) } } else { - ( + Some(( find_nth_open_pair(text, open, close, pos, n)?, - find_nth_close_pair(text, open, close, pos, n)?, - ) - }; - - Some((open_pos, close_pos)) + next_grapheme_boundary(text, find_nth_close_pair(text, open, close, pos, n)?), + )) + } } fn find_nth_open_pair( @@ -173,12 +186,13 @@ mod test { let slice = doc.slice(..); // cursor on [t]ext - assert_eq!(find_nth_pairs_pos(slice, '(', 6, 1), Some((5, 10))); - assert_eq!(find_nth_pairs_pos(slice, ')', 6, 1), Some((5, 10))); + assert_eq!(find_nth_pairs_pos(slice, '(', 6, 1), Some((5, 11))); + assert_eq!(find_nth_pairs_pos(slice, ')', 6, 1), Some((5, 11))); // cursor on so[m]e assert_eq!(find_nth_pairs_pos(slice, '(', 2, 1), None); // cursor on bracket itself - assert_eq!(find_nth_pairs_pos(slice, '(', 5, 1), Some((5, 10))); + assert_eq!(find_nth_pairs_pos(slice, '(', 5, 1), Some((5, 11))); + assert_eq!(find_nth_pairs_pos(slice, '(', 10, 1), Some((5, 11))); } #[test] @@ -187,9 +201,9 @@ mod test { let slice = doc.slice(..); // cursor on go[o]d - assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((10, 15))); - assert_eq!(find_nth_pairs_pos(slice, '(', 13, 2), Some((4, 21))); - assert_eq!(find_nth_pairs_pos(slice, '(', 13, 3), Some((0, 27))); + assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((10, 16))); + assert_eq!(find_nth_pairs_pos(slice, '(', 13, 2), Some((4, 22))); + assert_eq!(find_nth_pairs_pos(slice, '(', 13, 3), Some((0, 28))); } #[test] @@ -198,14 +212,14 @@ mod test { let slice = doc.slice(..); // cursor on go[o]d - assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 1), Some((10, 15))); - assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 2), Some((4, 21))); - assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 3), Some((0, 27))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 1), Some((10, 16))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 2), Some((4, 22))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 3), Some((0, 28))); // cursor on the quotes - assert_eq!(find_nth_pairs_pos(slice, '\'', 10, 1), Some((10, 15))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 10, 1), None); // this is the best we can do since opening and closing pairs are same - assert_eq!(find_nth_pairs_pos(slice, '\'', 0, 1), Some((0, 4))); - assert_eq!(find_nth_pairs_pos(slice, '\'', 27, 1), Some((21, 27))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 0, 1), Some((0, 5))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 27, 1), Some((21, 28))); } #[test] @@ -214,8 +228,8 @@ mod test { let slice = doc.slice(..); // cursor on go[o]d - assert_eq!(find_nth_pairs_pos(slice, '(', 15, 1), Some((5, 24))); - assert_eq!(find_nth_pairs_pos(slice, '(', 15, 2), Some((0, 31))); + assert_eq!(find_nth_pairs_pos(slice, '(', 15, 1), Some((5, 25))); + assert_eq!(find_nth_pairs_pos(slice, '(', 15, 2), Some((0, 32))); } #[test] @@ -224,9 +238,9 @@ mod test { let slice = doc.slice(..); // cursor on go[o]d - assert_eq!(find_nth_pairs_pos(slice, '{', 13, 1), Some((10, 15))); - assert_eq!(find_nth_pairs_pos(slice, '[', 13, 1), Some((4, 21))); - assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((0, 27))); + assert_eq!(find_nth_pairs_pos(slice, '{', 13, 1), Some((10, 16))); + assert_eq!(find_nth_pairs_pos(slice, '[', 13, 1), Some((4, 22))); + assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((0, 28))); } #[test] @@ -243,7 +257,7 @@ mod test { get_surround_pos(slice, &selection, '(', 1) .unwrap() .as_slice(), - &[0, 5, 7, 13, 15, 23] + &[0, 6, 7, 14, 15, 24] ); } diff --git a/helix-core/src/textobject.rs b/helix-core/src/textobject.rs index fbf66256..ae18d7cf 100644 --- a/helix-core/src/textobject.rs +++ b/helix-core/src/textobject.rs @@ -1,21 +1,16 @@ use ropey::RopeSlice; -use crate::chars::{categorize_char, char_is_line_ending, char_is_whitespace, CharCategory}; -use crate::movement::{self, Direction}; +use crate::chars::{categorize_char, CharCategory}; +use crate::graphemes::{next_grapheme_boundary, prev_grapheme_boundary}; +use crate::movement::Direction; use crate::surround; use crate::Range; -fn this_word_end_pos(slice: RopeSlice, pos: usize) -> usize { - this_word_bound_pos(slice, pos, Direction::Forward) -} - -fn this_word_start_pos(slice: RopeSlice, pos: usize) -> usize { - this_word_bound_pos(slice, pos, Direction::Backward) -} +fn find_word_boundary(slice: RopeSlice, mut pos: usize, direction: Direction) -> usize { + use CharCategory::{Eol, Whitespace}; -fn this_word_bound_pos(slice: RopeSlice, mut pos: usize, direction: Direction) -> usize { let iter = match direction { - Direction::Forward => slice.chars_at(pos + 1), + Direction::Forward => slice.chars_at(pos), Direction::Backward => { let mut iter = slice.chars_at(pos); iter.reverse(); @@ -23,25 +18,32 @@ fn this_word_bound_pos(slice: RopeSlice, mut pos: usize, direction: Direction) - } }; - match categorize_char(slice.char(pos)) { - CharCategory::Eol | CharCategory::Whitespace => pos, - category => { - for peek in iter { - let curr_category = categorize_char(peek); - if curr_category != category - || curr_category == CharCategory::Eol - || curr_category == CharCategory::Whitespace - { + let mut prev_category = match direction { + Direction::Forward if pos == 0 => Whitespace, + Direction::Forward => categorize_char(slice.char(pos - 1)), + Direction::Backward if pos == slice.len_chars() => Whitespace, + Direction::Backward => categorize_char(slice.char(pos)), + }; + + for ch in iter { + match categorize_char(ch) { + Eol | Whitespace => return pos, + category => { + if category != prev_category && pos != 0 && pos != slice.len_chars() { return pos; - } - pos = match direction { - Direction::Forward => pos + 1, - Direction::Backward => pos.saturating_sub(1), + } else { + if direction == Direction::Forward { + pos += 1; + } else { + pos = pos.saturating_sub(1); + } + prev_category = category; } } - pos } } + + pos } #[derive(Copy, Clone, PartialEq, Eq, Debug)] @@ -55,46 +57,42 @@ pub fn textobject_word( slice: RopeSlice, range: Range, textobject: TextObject, - count: usize, + _count: usize, ) -> Range { - let this_word_start = this_word_start_pos(slice, range.head); - let this_word_end = this_word_end_pos(slice, range.head); + // For 1-width cursor semantics. + let head = if range.head > range.anchor { + prev_grapheme_boundary(slice, range.head) + } else { + range.head + }; + + let word_start = find_word_boundary(slice, head, Direction::Backward); + let word_end = match slice.get_char(head).map(categorize_char) { + None | Some(CharCategory::Whitespace | CharCategory::Eol) => head, + _ => find_word_boundary(slice, head + 1, Direction::Forward), + }; + + // Special case. + if word_start == word_end { + return Range::new(word_start, word_end); + } - let (anchor, head); match textobject { - TextObject::Inside => { - anchor = this_word_start; - head = this_word_end; - } - TextObject::Around => { - if slice - .get_char(this_word_end + 1) - .map_or(true, char_is_line_ending) + TextObject::Inside => Range::new(word_start, word_end), + TextObject::Around => Range::new( + match slice + .get_char(word_start.saturating_sub(1)) + .map(categorize_char) { - head = this_word_end; - if slice - .get_char(this_word_start.saturating_sub(1)) - .map_or(true, char_is_line_ending) - { - // single word on a line - anchor = this_word_start; - } else { - // last word on a line, select the whitespace before it too - anchor = movement::move_prev_word_end(slice, range, count).head; - } - } else if char_is_whitespace(slice.char(range.head)) { - // select whole whitespace and next word - head = movement::move_next_word_end(slice, range, count).head; - anchor = movement::backwards_skip_while(slice, range.head, |c| c.is_whitespace()) - .map(|p| p + 1) // p is first *non* whitespace char, so +1 to get whitespace pos - .unwrap_or(0); - } else { - head = movement::move_next_word_start(slice, range, count).head; - anchor = this_word_start; - } - } - }; - Range::new(anchor, head) + None | Some(CharCategory::Eol) => word_start, + _ => prev_grapheme_boundary(slice, word_start), + }, + match slice.get_char(word_end).map(categorize_char) { + None | Some(CharCategory::Eol) => word_end, + _ => next_grapheme_boundary(slice, word_end), + }, + ), + } } pub fn textobject_surround( @@ -106,7 +104,10 @@ pub fn textobject_surround( ) -> Range { surround::find_nth_pairs_pos(slice, ch, range.head, count) .map(|(anchor, head)| match textobject { - TextObject::Inside => Range::new(anchor + 1, head.saturating_sub(1)), + TextObject::Inside => Range::new( + next_grapheme_boundary(slice, anchor), + prev_grapheme_boundary(slice, head), + ), TextObject::Around => Range::new(anchor, head), }) .unwrap_or(range) @@ -126,70 +127,70 @@ mod test { let tests = &[ ( "cursor at beginning of doc", - vec![(0, Inside, (0, 5)), (0, Around, (0, 6))], + vec![(0, Inside, (0, 6)), (0, Around, (0, 7))], ), ( "cursor at middle of word", vec![ - (13, Inside, (10, 15)), - (10, Inside, (10, 15)), - (15, Inside, (10, 15)), - (13, Around, (10, 16)), - (10, Around, (10, 16)), - (15, Around, (10, 16)), + (13, Inside, (10, 16)), + (10, Inside, (10, 16)), + (15, Inside, (10, 16)), + (13, Around, (9, 17)), + (10, Around, (9, 17)), + (15, Around, (9, 17)), ], ), ( "cursor between word whitespace", - vec![(6, Inside, (6, 6)), (6, Around, (6, 13))], + vec![(6, Inside, (6, 6)), (6, Around, (6, 6))], ), ( "cursor on word before newline\n", vec![ - (22, Inside, (22, 28)), - (28, Inside, (22, 28)), - (25, Inside, (22, 28)), - (22, Around, (21, 28)), - (28, Around, (21, 28)), - (25, Around, (21, 28)), + (22, Inside, (22, 29)), + (28, Inside, (22, 29)), + (25, Inside, (22, 29)), + (22, Around, (21, 29)), + (28, Around, (21, 29)), + (25, Around, (21, 29)), ], ), ( "cursor on newline\nnext line", - vec![(17, Inside, (17, 17)), (17, Around, (17, 22))], + vec![(17, Inside, (17, 17)), (17, Around, (17, 17))], ), ( "cursor on word after newline\nnext line", vec![ - (29, Inside, (29, 32)), - (30, Inside, (29, 32)), - (32, Inside, (29, 32)), - (29, Around, (29, 33)), - (30, Around, (29, 33)), - (32, Around, (29, 33)), + (29, Inside, (29, 33)), + (30, Inside, (29, 33)), + (32, Inside, (29, 33)), + (29, Around, (29, 34)), + (30, Around, (29, 34)), + (32, Around, (29, 34)), ], ), ( "cursor on #$%:;* punctuation", vec![ - (13, Inside, (10, 15)), - (10, Inside, (10, 15)), - (15, Inside, (10, 15)), - (13, Around, (10, 16)), - (10, Around, (10, 16)), - (15, Around, (10, 16)), + (13, Inside, (10, 16)), + (10, Inside, (10, 16)), + (15, Inside, (10, 16)), + (13, Around, (9, 17)), + (10, Around, (9, 17)), + (15, Around, (9, 17)), ], ), ( "cursor on punc%^#$:;.tuation", vec![ - (14, Inside, (14, 20)), - (20, Inside, (14, 20)), - (17, Inside, (14, 20)), - (14, Around, (14, 20)), + (14, Inside, (14, 21)), + (20, Inside, (14, 21)), + (17, Inside, (14, 21)), + (14, Around, (13, 22)), // FIXME: edge case // (20, Around, (14, 20)), - (17, Around, (14, 20)), + (17, Around, (13, 22)), ], ), ( @@ -198,14 +199,14 @@ mod test { (9, Inside, (9, 9)), (10, Inside, (10, 10)), (11, Inside, (11, 11)), - (9, Around, (9, 16)), - (10, Around, (9, 16)), - (11, Around, (9, 16)), + (9, Around, (9, 9)), + (10, Around, (10, 10)), + (11, Around, (11, 11)), ], ), ( "cursor at end of doc", - vec![(19, Inside, (17, 19)), (19, Around, (16, 19))], + vec![(19, Inside, (17, 20)), (19, Around, (16, 20))], ), ]; @@ -234,67 +235,67 @@ mod test { "simple (single) surround pairs", vec![ (3, Inside, (3, 3), '(', 1), - (7, Inside, (8, 13), ')', 1), - (10, Inside, (8, 13), '(', 1), - (14, Inside, (8, 13), ')', 1), + (7, Inside, (8, 14), ')', 1), + (10, Inside, (8, 14), '(', 1), + (14, Inside, (8, 14), ')', 1), (3, Around, (3, 3), '(', 1), - (7, Around, (7, 14), ')', 1), - (10, Around, (7, 14), '(', 1), - (14, Around, (7, 14), ')', 1), + (7, Around, (7, 15), ')', 1), + (10, Around, (7, 15), '(', 1), + (14, Around, (7, 15), ')', 1), ], ), ( "samexx 'single' surround pairs", vec![ (3, Inside, (3, 3), '\'', 1), - (7, Inside, (8, 13), '\'', 1), - (10, Inside, (8, 13), '\'', 1), - (14, Inside, (8, 13), '\'', 1), + (7, Inside, (7, 7), '\'', 1), + (10, Inside, (8, 14), '\'', 1), + (14, Inside, (14, 14), '\'', 1), (3, Around, (3, 3), '\'', 1), - (7, Around, (7, 14), '\'', 1), - (10, Around, (7, 14), '\'', 1), - (14, Around, (7, 14), '\'', 1), + (7, Around, (7, 7), '\'', 1), + (10, Around, (7, 15), '\'', 1), + (14, Around, (14, 14), '\'', 1), ], ), ( "(nested (surround (pairs)) 3 levels)", vec![ - (0, Inside, (1, 34), '(', 1), - (6, Inside, (1, 34), ')', 1), - (8, Inside, (9, 24), '(', 1), - (8, Inside, (9, 34), ')', 2), - (20, Inside, (9, 24), '(', 2), - (20, Inside, (1, 34), ')', 3), - (0, Around, (0, 35), '(', 1), - (6, Around, (0, 35), ')', 1), - (8, Around, (8, 25), '(', 1), - (8, Around, (8, 35), ')', 2), - (20, Around, (8, 25), '(', 2), - (20, Around, (0, 35), ')', 3), + (0, Inside, (1, 35), '(', 1), + (6, Inside, (1, 35), ')', 1), + (8, Inside, (9, 25), '(', 1), + (8, Inside, (9, 35), ')', 2), + (20, Inside, (9, 25), '(', 2), + (20, Inside, (1, 35), ')', 3), + (0, Around, (0, 36), '(', 1), + (6, Around, (0, 36), ')', 1), + (8, Around, (8, 26), '(', 1), + (8, Around, (8, 36), ')', 2), + (20, Around, (8, 26), '(', 2), + (20, Around, (0, 36), ')', 3), ], ), ( "(mixed {surround [pair] same} line)", vec![ - (2, Inside, (1, 33), '(', 1), - (9, Inside, (8, 27), '{', 1), - (18, Inside, (18, 21), '[', 1), - (2, Around, (0, 34), '(', 1), - (9, Around, (7, 28), '{', 1), - (18, Around, (17, 22), '[', 1), + (2, Inside, (1, 34), '(', 1), + (9, Inside, (8, 28), '{', 1), + (18, Inside, (18, 22), '[', 1), + (2, Around, (0, 35), '(', 1), + (9, Around, (7, 29), '{', 1), + (18, Around, (17, 23), '[', 1), ], ), ( "(stepped (surround) pairs (should) skip)", - vec![(22, Inside, (1, 38), '(', 1), (22, Around, (0, 39), '(', 1)], + vec![(22, Inside, (1, 39), '(', 1), (22, Around, (0, 40), '(', 1)], ), ( "[surround pairs{\non different]\nlines}", vec![ - (7, Inside, (1, 28), '[', 1), - (15, Inside, (16, 35), '{', 1), - (7, Around, (0, 29), '[', 1), - (15, Around, (15, 36), '{', 1), + (7, Inside, (1, 29), '[', 1), + (15, Inside, (16, 36), '{', 1), + (7, Around, (0, 30), '[', 1), + (15, Around, (15, 37), '{', 1), ], ), ]; diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index fbeae5ff..51e633f6 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1,9 +1,6 @@ use helix_core::{ comment, coords_at_pos, find_first_non_whitespace_char, find_root, graphemes, indent, - line_ending::{ - get_line_ending_of_str, line_end_char_index, rope_end_without_line_ending, - str_is_line_ending, - }, + line_ending::{get_line_ending_of_str, line_end_char_index, str_is_line_ending}, match_brackets, movement::{self, Direction}, object, pos_at_coords, @@ -392,20 +389,14 @@ fn goto_line_end(cx: &mut Context) { doc.set_selection( view.id, doc.selection(view.id).clone().transform(|range| { - let text = doc.text(); + let text = doc.text().slice(..); let line = text.char_to_line(range.head); - let pos = line_end_char_index(&text.slice(..), line); - let pos = graphemes::nth_prev_grapheme_boundary(text.slice(..), pos, 1); + let pos = line_end_char_index(&text, line); + let pos = graphemes::nth_prev_grapheme_boundary(text, pos, 1); let pos = range.head.max(pos).max(text.line_to_char(line)); - Range::new( - match doc.mode { - Mode::Normal | Mode::Insert => pos, - Mode::Select => range.anchor, - }, - pos, - ) + range.put(text, pos, doc.mode == Mode::Select) }), ); } @@ -416,11 +407,11 @@ fn goto_line_end_newline(cx: &mut Context) { doc.set_selection( view.id, doc.selection(view.id).clone().transform(|range| { - let text = doc.text(); + let text = doc.text().slice(..); let line = text.char_to_line(range.head); - let pos = line_end_char_index(&text.slice(..), line); - Range::new(pos, pos) + let pos = line_end_char_index(&text, line); + range.put(text, pos, doc.mode == Mode::Select) }), ); } @@ -430,18 +421,12 @@ fn goto_line_start(cx: &mut Context) { doc.set_selection( view.id, doc.selection(view.id).clone().transform(|range| { - let text = doc.text(); + let text = doc.text().slice(..); let line = text.char_to_line(range.head); // adjust to start of the line let pos = text.line_to_char(line); - Range::new( - match doc.mode { - Mode::Normal | Mode::Insert => pos, - Mode::Select => range.anchor, - }, - pos, - ) + range.put(text, pos, doc.mode == Mode::Select) }), ); } @@ -451,18 +436,12 @@ fn goto_first_nonwhitespace(cx: &mut Context) { doc.set_selection( view.id, doc.selection(view.id).clone().transform(|range| { - let text = doc.text(); + let text = doc.text().slice(..); let line_idx = text.char_to_line(range.head); if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) { let pos = pos + text.line_to_char(line_idx); - Range::new( - match doc.mode { - Mode::Normal | Mode::Insert => pos, - Mode::Select => range.anchor, - }, - pos, - ) + range.put(text, pos, doc.mode == Mode::Select) } else { range } @@ -581,9 +560,7 @@ fn goto_file_start(cx: &mut Context) { fn goto_file_end(cx: &mut Context) { push_jump(cx.editor); let (view, doc) = current!(cx.editor); - let text = doc.text(); - let last_line = text.line_to_char(text.len_lines().saturating_sub(2)); - doc.set_selection(view.id, Selection::point(last_line)); + doc.set_selection(view.id, Selection::point(doc.text().len_chars())); } fn extend_next_word_start(cx: &mut Context) { @@ -592,9 +569,10 @@ fn extend_next_word_start(cx: &mut Context) { doc.set_selection( view.id, doc.selection(view.id).clone().transform(|range| { - let word = movement::move_next_word_start(doc.text().slice(..), range, count); + let text = doc.text().slice(..); + let word = movement::move_next_word_start(text, range, count); let pos = word.head; - Range::new(range.anchor, pos) + range.put(text, pos, true) }), ); } @@ -605,9 +583,10 @@ fn extend_prev_word_start(cx: &mut Context) { doc.set_selection( view.id, doc.selection(view.id).clone().transform(|range| { - let word = movement::move_prev_word_start(doc.text().slice(..), range, count); + let text = doc.text().slice(..); + let word = movement::move_prev_word_start(text, range, count); let pos = word.head; - Range::new(range.anchor, pos) + range.put(text, pos, true) }), ); } @@ -618,9 +597,10 @@ fn extend_next_word_end(cx: &mut Context) { doc.set_selection( view.id, doc.selection(view.id).clone().transform(|range| { - let word = movement::move_next_word_end(doc.text().slice(..), range, count); + let text = doc.text().slice(..); + let word = movement::move_next_word_end(text, range, count); let pos = word.head; - Range::new(range.anchor, pos) + range.put(text, pos, true) }), ); } @@ -669,18 +649,9 @@ where doc.set_selection( view.id, doc.selection(view.id).clone().transform(|range| { - search_fn(doc.text().slice(..), ch, range.head, count, inclusive).map_or( - range, - |pos| { - if extend { - Range::new(range.anchor, pos) - } else { - // select - Range::new(range.head, pos) - } - // or (pos, pos) to move to found val - }, - ) + let text = doc.text().slice(..); + search_fn(text, ch, range.head, count, inclusive) + .map_or(range, |pos| range.put(text, pos, extend)) }), ); }) @@ -940,7 +911,7 @@ fn extend_line_down(cx: &mut Context) { fn select_all(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let end = rope_end_without_line_ending(&doc.text().slice(..)); + let end = doc.text().len_chars(); doc.set_selection(view.id, Selection::single(0, end)) } @@ -997,12 +968,10 @@ fn search_impl(doc: &mut Document, view: &mut View, contents: &str, regex: &Rege return; } - let head = end - 1; - let selection = if extend { - selection.clone().push(Range::new(start, head)) + selection.clone().push(Range::new(start, end)) } else { - Selection::single(start, head) + Selection::single(start, end) }; doc.set_selection(view.id, selection); @@ -1143,9 +1112,15 @@ fn collapse_selection(cx: &mut Context) { doc.set_selection( view.id, - doc.selection(view.id) - .clone() - .transform(|range| Range::new(range.head, range.head)), + doc.selection(view.id).clone().transform(|range| { + let pos = if range.head > range.anchor { + // For 1-width cursor semantics. + graphemes::prev_grapheme_boundary(doc.text().slice(..), range.head) + } else { + range.head + }; + Range::new(pos, pos) + }), ); } @@ -1184,10 +1159,13 @@ fn append_mode(cx: &mut Context) { doc.restore_cursor = true; let selection = doc.selection(view.id).clone().transform(|range| { - Range::new( - range.from(), - graphemes::next_grapheme_boundary(doc.text().slice(..), range.to()), // to() + next char - ) + let to = if range.to() == range.from() { + // For 1-width cursor semantics. + graphemes::next_grapheme_boundary(doc.text().slice(..), range.to()) + } else { + range.to() + }; + Range::new(range.from(), to) }); let end = doc.text().len_chars(); -- cgit v1.2.3-70-g09d2 From 954314a7c9661920d878cdbf604354252dca8ce8 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Sat, 17 Jul 2021 10:57:58 -0700 Subject: Update change-case commands to work with gap indexing. --- helix-term/src/commands.rs | 63 ++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 27 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 74b54db7..ae71b701 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -786,24 +786,27 @@ fn replace(cx: &mut Context) { fn switch_case(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let text: Tendril = range - .fragment(doc.text().slice(..)) - .chars() - .flat_map(|ch| { - if ch.is_lowercase() { - ch.to_uppercase().collect() - } else if ch.is_uppercase() { - ch.to_lowercase().collect() - } else { - vec![ch] - } - }) - .collect(); + let selection = doc + .selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)); + let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + let text: Tendril = range + .fragment(doc.text().slice(..)) + .chars() + .flat_map(|ch| { + if ch.is_lowercase() { + ch.to_uppercase().collect() + } else if ch.is_uppercase() { + ch.to_lowercase().collect() + } else { + vec![ch] + } + }) + .collect(); - (range.from(), range.to() + 1, Some(text)) - }); + (range.from(), range.to(), Some(text)) + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); @@ -811,12 +814,15 @@ fn switch_case(cx: &mut Context) { fn switch_to_uppercase(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let text: Tendril = range.fragment(doc.text().slice(..)).to_uppercase().into(); + let selection = doc + .selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)); + let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + let text: Tendril = range.fragment(doc.text().slice(..)).to_uppercase().into(); - (range.from(), range.to() + 1, Some(text)) - }); + (range.from(), range.to(), Some(text)) + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); @@ -824,12 +830,15 @@ fn switch_to_uppercase(cx: &mut Context) { fn switch_to_lowercase(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let text: Tendril = range.fragment(doc.text().slice(..)).to_lowercase().into(); + let selection = doc + .selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)); + let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + let text: Tendril = range.fragment(doc.text().slice(..)).to_lowercase().into(); - (range.from(), range.to() + 1, Some(text)) - }); + (range.from(), range.to(), Some(text)) + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); -- cgit v1.2.3-70-g09d2 From c2fd55e1685d22de2facf5acbfe6b7ddc02c7c84 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Sat, 17 Jul 2021 11:28:20 -0700 Subject: Update extend_line command to work with gap indexing. --- helix-term/src/commands.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index ae71b701..41b342b2 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1100,16 +1100,18 @@ fn extend_line(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let pos = doc.selection(view.id).primary(); let text = doc.text(); + let pos = doc.selection(view.id).primary().min_width_1(text.slice(..)); - let line_start = text.char_to_line(pos.anchor); - let start = text.line_to_char(line_start); - let line_end = text.char_to_line(pos.head); - let mut end = line_end_char_index(&text.slice(..), line_end + count.saturating_sub(1)); + let line_max = text.len_lines(); + let start_line = text.char_to_line(pos.from()).min(line_max); + let end_line = (text.char_to_line(pos.to()) + count).min(line_max); - if pos.anchor == start && pos.head == end && line_end < (text.len_lines() - 2) { - end = line_end_char_index(&text.slice(..), line_end + 1); + let start = text.line_to_char(start_line); + let mut end = text.line_to_char(end_line); + + if pos.from() == start && pos.to() == end { + end = text.line_to_char((end_line + 1).min(line_max)); } doc.set_selection(view.id, Selection::single(start, end)); -- cgit v1.2.3-70-g09d2 From 6c038bb0151c6aeb43fc94bd2dc3d516a71d346c Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Sun, 18 Jul 2021 21:59:31 -0700 Subject: Update word selection/navigation to work with gap indexing. Also tweaked some of the existing behavior that seemed inconsistent and/or buggy. It's mostly identical, just a few corner cases are different. --- helix-core/src/movement.rs | 606 ++++++++++++++++++++++++--------------------- helix-term/src/commands.rs | 75 ++++-- 2 files changed, 377 insertions(+), 304 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index 2cb4b40d..2d9798bf 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -1,4 +1,4 @@ -use std::iter::{self, from_fn}; +use std::iter; use ropey::iter::Chars; @@ -142,8 +142,41 @@ pub fn move_prev_word_end(slice: RopeSlice, range: Range, count: usize) -> Range } fn word_move(slice: RopeSlice, range: Range, count: usize, target: WordMotionTarget) -> Range { - (0..count).fold(range, |range, _| { - slice.chars_at(range.head).range_to_target(target, range) + let is_prev = matches!( + target, + WordMotionTarget::PrevWordStart + | WordMotionTarget::PrevLongWordStart + | WordMotionTarget::PrevWordEnd + ); + + // Special-case early-out. + if (is_prev && range.head == 0) || (!is_prev && range.head == slice.len_chars()) { + return range; + } + + // Prepare the range appropriately based on the target movement + // direction. This is addressing two things at once: + // + // 1. 1-width range sementics. + // 2. The anchor position being irrelevant to the output result. + #[allow(clippy::collapsible_else_if)] // Makes the structure clearer in this case. + let start_range = if is_prev { + if range.anchor < range.head { + Range::new(range.head, prev_grapheme_boundary(slice, range.head)) + } else { + Range::new(next_grapheme_boundary(slice, range.head), range.head) + } + } else { + if range.anchor < range.head { + Range::new(prev_grapheme_boundary(slice, range.head), range.head) + } else { + Range::new(range.head, next_grapheme_boundary(slice, range.head)) + } + }; + + // Do the main work. + (0..count).fold(start_range, |r, _| { + slice.chars_at(r.head).range_to_target(target, r) }) } @@ -200,79 +233,75 @@ pub trait CharHelpers { fn range_to_target(&mut self, target: WordMotionTarget, origin: Range) -> Range; } -enum WordMotionPhase { - Start, - SkipNewlines, - ReachTarget, -} - impl CharHelpers for Chars<'_> { + /// Note: this only changes the anchor of the range if the head is effectively + /// starting on a boundary (either directly or after skipping newline characters). + /// Any other changes to the anchor should be handled by the calling code. fn range_to_target(&mut self, target: WordMotionTarget, origin: Range) -> Range { - // Characters are iterated forward or backwards depending on the motion direction. - let characters: Box> = match target { + let is_prev = matches!( + target, WordMotionTarget::PrevWordStart - | WordMotionTarget::PrevLongWordStart - | WordMotionTarget::PrevWordEnd => { + | WordMotionTarget::PrevLongWordStart + | WordMotionTarget::PrevWordEnd + ); + + // Reverse the iterator if needed for the motion direction. + if is_prev { + self.reverse(); + } + + // Function to advance index in the appropriate motion direction. + let advance: &dyn Fn(&mut usize) = if is_prev { + &|idx| *idx = idx.saturating_sub(1) + } else { + &|idx| *idx += 1 + }; + + // Initialize state variables. + let mut anchor = origin.anchor; + let mut head = origin.head; + let mut prev_ch = { + let ch = self.prev(); + if ch.is_some() { self.next(); - Box::new(from_fn(|| self.prev())) } - _ => Box::new(self), + ch }; - // Index advancement also depends on the direction. - let advance: &dyn Fn(&mut usize) = match target { - WordMotionTarget::PrevWordStart - | WordMotionTarget::PrevLongWordStart - | WordMotionTarget::PrevWordEnd => &|u| *u = u.saturating_sub(1), - _ => &|u| *u += 1, - }; + // Skip any initial newline characters. + while let Some(ch) = self.next() { + if char_is_line_ending(ch) { + prev_ch = Some(ch); + advance(&mut head); + } else { + self.prev(); + break; + } + } + if prev_ch.map(char_is_line_ending).unwrap_or(false) { + anchor = head; + } - let mut characters = characters.peekable(); - let mut phase = WordMotionPhase::Start; - let mut head = origin.head; - let mut anchor: Option = None; - let is_boundary = - |a: char, b: Option| categorize_char(a) != categorize_char(b.unwrap_or(a)); - while let Some(peek) = characters.peek().copied() { - phase = match phase { - WordMotionPhase::Start => { - characters.next(); - if characters.peek().is_none() { - break; // We're at the end, so there's nothing to do. - } - // Anchor may remain here if the head wasn't at a boundary - if !is_boundary(peek, characters.peek().copied()) && !char_is_line_ending(peek) - { - anchor = Some(head); - } - // First character is always skipped by the head - advance(&mut head); - WordMotionPhase::SkipNewlines - } - WordMotionPhase::SkipNewlines => { - if char_is_line_ending(peek) { - characters.next(); - if characters.peek().is_some() { - advance(&mut head); - } - WordMotionPhase::SkipNewlines - } else { - WordMotionPhase::ReachTarget - } - } - WordMotionPhase::ReachTarget => { - characters.next(); - anchor = anchor.or(Some(head)); - if reached_target(target, peek, characters.peek()) { - break; - } else { - advance(&mut head); - } - WordMotionPhase::ReachTarget + // Find our target position(s). + let head_start = head; + while let Some(next_ch) = self.next() { + if reached_target(target, prev_ch.unwrap_or(next_ch), next_ch) { + if head == head_start { + anchor = head; + } else { + break; } } + prev_ch = Some(next_ch); + advance(&mut head); } - Range::new(anchor.unwrap_or(origin.anchor), head) + + // Un-reverse the iterator if needed. + if is_prev { + self.reverse(); + } + + Range::new(anchor, head) } } @@ -289,28 +318,23 @@ fn is_long_word_boundary(a: char, b: char) -> bool { } } -fn reached_target(target: WordMotionTarget, peek: char, next_peek: Option<&char>) -> bool { - let next_peek = match next_peek { - Some(next_peek) => next_peek, - None => return true, - }; - +fn reached_target(target: WordMotionTarget, prev_ch: char, next_ch: char) -> bool { match target { WordMotionTarget::NextWordStart | WordMotionTarget::PrevWordEnd => { - is_word_boundary(peek, *next_peek) - && (char_is_line_ending(*next_peek) || !next_peek.is_whitespace()) + is_word_boundary(prev_ch, next_ch) + && (char_is_line_ending(next_ch) || !next_ch.is_whitespace()) } WordMotionTarget::NextWordEnd | WordMotionTarget::PrevWordStart => { - is_word_boundary(peek, *next_peek) - && (!peek.is_whitespace() || char_is_line_ending(*next_peek)) + is_word_boundary(prev_ch, next_ch) + && (!prev_ch.is_whitespace() || char_is_line_ending(next_ch)) } WordMotionTarget::NextLongWordStart => { - is_long_word_boundary(peek, *next_peek) - && (char_is_line_ending(*next_peek) || !next_peek.is_whitespace()) + is_long_word_boundary(prev_ch, next_ch) + && (char_is_line_ending(next_ch) || !next_ch.is_whitespace()) } WordMotionTarget::NextLongWordEnd | WordMotionTarget::PrevLongWordStart => { - is_long_word_boundary(peek, *next_peek) - && (!peek.is_whitespace() || char_is_line_ending(*next_peek)) + is_long_word_boundary(prev_ch, next_ch) + && (!prev_ch.is_whitespace() || char_is_line_ending(next_ch)) } } } @@ -538,42 +562,42 @@ mod test { fn test_behaviour_when_moving_to_start_of_next_words() { let tests = array::IntoIter::new([ ("Basic forward motion stops at the first space", - vec![(1, Range::new(0, 0), Range::new(0, 5))]), + vec![(1, Range::new(0, 0), Range::new(0, 6))]), (" Starting from a boundary advances the anchor", - vec![(1, Range::new(0, 0), Range::new(1, 9))]), + vec![(1, Range::new(0, 0), Range::new(1, 10))]), ("Long whitespace gap is bridged by the head", - vec![(1, Range::new(0, 0), Range::new(0, 10))]), + vec![(1, Range::new(0, 0), Range::new(0, 11))]), ("Previous anchor is irrelevant for forward motions", - vec![(1, Range::new(12, 0), Range::new(0, 8))]), + vec![(1, Range::new(12, 0), Range::new(0, 9))]), (" Starting from whitespace moves to last space in sequence", - vec![(1, Range::new(0, 0), Range::new(0, 3))]), + vec![(1, Range::new(0, 0), Range::new(0, 4))]), ("Starting from mid-word leaves anchor at start position and moves head", - vec![(1, Range::new(3, 3), Range::new(3, 8))]), + vec![(1, Range::new(3, 3), Range::new(3, 9))]), ("Identifiers_with_underscores are considered a single word", - vec![(1, Range::new(0, 0), Range::new(0, 28))]), + vec![(1, Range::new(0, 0), Range::new(0, 29))]), ("Jumping\n into starting whitespace selects the spaces before 'into'", - vec![(1, Range::new(0, 6), Range::new(8, 11))]), + vec![(1, Range::new(0, 7), Range::new(8, 12))]), ("alphanumeric.!,and.?=punctuation are considered 'words' for the purposes of word motion", vec![ - (1, Range::new(0, 0), Range::new(0, 11)), - (1, Range::new(0, 11), Range::new(12, 14)), - (1, Range::new(12, 14), Range::new(15, 17)) + (1, Range::new(0, 0), Range::new(0, 12)), + (1, Range::new(0, 12), Range::new(12, 15)), + (1, Range::new(12, 15), Range::new(15, 18)) ]), ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 0), Range::new(0, 5)), - (1, Range::new(0, 5), Range::new(6, 9)), + (1, Range::new(0, 0), Range::new(0, 6)), + (1, Range::new(0, 6), Range::new(6, 10)), ]), (".._.._ punctuation is not joined by underscores into a single block", - vec![(1, Range::new(0, 0), Range::new(0, 1))]), + vec![(1, Range::new(0, 0), Range::new(0, 2))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 0), Range::new(0, 7)), - (1, Range::new(0, 7), Range::new(10, 13)), + (1, Range::new(0, 0), Range::new(0, 8)), + (1, Range::new(0, 8), Range::new(10, 14)), ]), ("Jumping\n\n\n\n\n\n from newlines to whitespace selects whitespace.", vec![ - (1, Range::new(0, 8), Range::new(13, 15)), + (1, Range::new(0, 9), Range::new(13, 16)), ]), ("A failed motion does not modify the range", vec![ @@ -581,17 +605,17 @@ mod test { ]), ("oh oh oh two character words!", vec![ - (1, Range::new(0, 0), Range::new(0, 2)), - (1, Range::new(0, 2), Range::new(3, 5)), - (1, Range::new(0, 1), Range::new(2, 2)), + (1, Range::new(0, 0), Range::new(0, 3)), + (1, Range::new(0, 3), Range::new(3, 6)), + (1, Range::new(0, 2), Range::new(1, 3)), ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(0, 0), Range::new(17, 19)), + (3, Range::new(0, 0), Range::new(17, 20)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(0, 0), Range::new(32, 40)), + (999, Range::new(0, 0), Range::new(32, 41)), ]), ("", // Edge case of moving forward in empty string vec![ @@ -599,16 +623,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving forward in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 4)), + (1, Range::new(0, 0), Range::new(5, 5)), ]), ("\n \n \n Jumping through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 0), Range::new(1, 3)), - (1, Range::new(1, 3), Range::new(5, 7)), + (1, Range::new(0, 0), Range::new(1, 4)), + (1, Range::new(1, 4), Range::new(5, 8)), ]), ("ヒーリクス multibyte characters behave as normal characters", vec![ - (1, Range::new(0, 0), Range::new(0, 5)), + (1, Range::new(0, 0), Range::new(0, 6)), ]), ]); @@ -624,40 +648,40 @@ mod test { fn test_behaviour_when_moving_to_start_of_next_long_words() { let tests = array::IntoIter::new([ ("Basic forward motion stops at the first space", - vec![(1, Range::new(0, 0), Range::new(0, 5))]), + vec![(1, Range::new(0, 0), Range::new(0, 6))]), (" Starting from a boundary advances the anchor", - vec![(1, Range::new(0, 0), Range::new(1, 9))]), + vec![(1, Range::new(0, 0), Range::new(1, 10))]), ("Long whitespace gap is bridged by the head", - vec![(1, Range::new(0, 0), Range::new(0, 10))]), + vec![(1, Range::new(0, 0), Range::new(0, 11))]), ("Previous anchor is irrelevant for forward motions", - vec![(1, Range::new(12, 0), Range::new(0, 8))]), + vec![(1, Range::new(12, 0), Range::new(0, 9))]), (" Starting from whitespace moves to last space in sequence", - vec![(1, Range::new(0, 0), Range::new(0, 3))]), + vec![(1, Range::new(0, 0), Range::new(0, 4))]), ("Starting from mid-word leaves anchor at start position and moves head", - vec![(1, Range::new(3, 3), Range::new(3, 8))]), + vec![(1, Range::new(3, 3), Range::new(3, 9))]), ("Identifiers_with_underscores are considered a single word", - vec![(1, Range::new(0, 0), Range::new(0, 28))]), + vec![(1, Range::new(0, 0), Range::new(0, 29))]), ("Jumping\n into starting whitespace selects the spaces before 'into'", - vec![(1, Range::new(0, 6), Range::new(8, 11))]), + vec![(1, Range::new(0, 7), Range::new(8, 12))]), ("alphanumeric.!,and.?=punctuation are not treated any differently than alphanumerics", vec![ - (1, Range::new(0, 0), Range::new(0, 32)), + (1, Range::new(0, 0), Range::new(0, 33)), ]), ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 0), Range::new(0, 5)), - (1, Range::new(0, 5), Range::new(6, 9)), + (1, Range::new(0, 0), Range::new(0, 6)), + (1, Range::new(0, 6), Range::new(6, 10)), ]), (".._.._ punctuation is joined by underscores into a single word, as it behaves like alphanumerics", - vec![(1, Range::new(0, 0), Range::new(0, 6))]), + vec![(1, Range::new(0, 0), Range::new(0, 7))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 0), Range::new(0, 7)), - (1, Range::new(0, 7), Range::new(10, 13)), + (1, Range::new(0, 0), Range::new(0, 8)), + (1, Range::new(0, 8), Range::new(10, 14)), ]), ("Jumping\n\n\n\n\n\n from newlines to whitespace selects whitespace.", vec![ - (1, Range::new(0, 8), Range::new(13, 15)), + (1, Range::new(0, 9), Range::new(13, 16)), ]), ("A failed motion does not modify the range", vec![ @@ -665,17 +689,17 @@ mod test { ]), ("oh oh oh two character words!", vec![ - (1, Range::new(0, 0), Range::new(0, 2)), - (1, Range::new(0, 2), Range::new(3, 5)), - (1, Range::new(0, 1), Range::new(2, 2)), + (1, Range::new(0, 0), Range::new(0, 3)), + (1, Range::new(0, 3), Range::new(3, 6)), + (1, Range::new(0, 1), Range::new(0, 3)), ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(0, 0), Range::new(17, 19)), + (3, Range::new(0, 0), Range::new(17, 20)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(0, 0), Range::new(32, 40)), + (999, Range::new(0, 0), Range::new(32, 41)), ]), ("", // Edge case of moving forward in empty string vec![ @@ -683,16 +707,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving forward in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 4)), + (1, Range::new(0, 0), Range::new(5, 5)), ]), ("\n \n \n Jumping through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 0), Range::new(1, 3)), - (1, Range::new(1, 3), Range::new(5, 7)), + (1, Range::new(0, 0), Range::new(1, 4)), + (1, Range::new(1, 4), Range::new(5, 8)), ]), ("ヒー..リクス multibyte characters behave as normal characters, including their interaction with punctuation", vec![ - (1, Range::new(0, 0), Range::new(0, 7)), + (1, Range::new(0, 0), Range::new(0, 8)), ]), ]); @@ -708,44 +732,47 @@ mod test { fn test_behaviour_when_moving_to_start_of_previous_words() { let tests = array::IntoIter::new([ ("Basic backward motion from the middle of a word", - vec![(1, Range::new(3, 3), Range::new(3, 0))]), - ("Starting from after boundary retreats the anchor", - vec![(1, Range::new(0, 8), Range::new(7, 0))]), + vec![(1, Range::new(3, 3), Range::new(4, 0))]), + + // // Why do we want this behavior? The current behavior fails this + // // test, but seems better and more consistent. + // ("Starting from after boundary retreats the anchor", + // vec![(1, Range::new(0, 9), Range::new(8, 0))]), + (" Jump to start of a word preceded by whitespace", - vec![(1, Range::new(5, 5), Range::new(5, 4))]), + vec![(1, Range::new(5, 5), Range::new(6, 4))]), (" Jump to start of line from start of word preceded by whitespace", - vec![(1, Range::new(4, 4), Range::new(3, 0))]), + vec![(1, Range::new(4, 4), Range::new(4, 0))]), ("Previous anchor is irrelevant for backward motions", - vec![(1, Range::new(12, 5), Range::new(5, 0))]), + vec![(1, Range::new(12, 5), Range::new(6, 0))]), (" Starting from whitespace moves to first space in sequence", - vec![(1, Range::new(0, 3), Range::new(3, 0))]), + vec![(1, Range::new(0, 4), Range::new(4, 0))]), ("Identifiers_with_underscores are considered a single word", vec![(1, Range::new(0, 20), Range::new(20, 0))]), ("Jumping\n \nback through a newline selects whitespace", - vec![(1, Range::new(0, 13), Range::new(11, 8))]), + vec![(1, Range::new(0, 13), Range::new(12, 8))]), ("Jumping to start of word from the end selects the word", - vec![(1, Range::new(6, 6), Range::new(6, 0))]), + vec![(1, Range::new(6, 7), Range::new(7, 0))]), ("alphanumeric.!,and.?=punctuation are considered 'words' for the purposes of word motion", vec![ - (1, Range::new(30, 30), Range::new(30, 21)), - (1, Range::new(30, 21), Range::new(20, 18)), - (1, Range::new(20, 18), Range::new(17, 15)) + (1, Range::new(29, 30), Range::new(30, 21)), + (1, Range::new(30, 21), Range::new(21, 18)), + (1, Range::new(21, 18), Range::new(18, 15)) ]), - ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 10), Range::new(9, 6)), - (1, Range::new(9, 6), Range::new(5, 0)), + (1, Range::new(0, 10), Range::new(10, 6)), + (1, Range::new(10, 6), Range::new(6, 0)), ]), (".._.._ punctuation is not joined by underscores into a single block", - vec![(1, Range::new(0, 5), Range::new(4, 3))]), + vec![(1, Range::new(0, 6), Range::new(5, 3))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 10), Range::new(7, 0)), + (1, Range::new(0, 10), Range::new(8, 0)), ]), ("Jumping \n\n\n\n\nback from within a newline group selects previous block", vec![ - (1, Range::new(0, 13), Range::new(10, 0)), + (1, Range::new(0, 13), Range::new(11, 0)), ]), ("Failed motions do not modify the range", vec![ @@ -753,11 +780,11 @@ mod test { ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(18, 18), Range::new(8, 0)), + (3, Range::new(18, 18), Range::new(9, 0)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(40, 40), Range::new(9, 0)), + (999, Range::new(40, 40), Range::new(10, 0)), ]), ("", // Edge case of moving backwards in empty string vec![ @@ -765,16 +792,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving backwards in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 0)), + (1, Range::new(5, 5), Range::new(0, 0)), ]), (" \n \nJumping back through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 7), Range::new(6, 4)), - (1, Range::new(6, 4), Range::new(2, 0)), + (1, Range::new(0, 8), Range::new(7, 4)), + (1, Range::new(7, 4), Range::new(3, 0)), ]), ("ヒーリクス multibyte characters behave as normal characters", vec![ - (1, Range::new(0, 5), Range::new(4, 0)), + (1, Range::new(0, 6), Range::new(6, 0)), ]), ]); @@ -789,72 +816,89 @@ mod test { #[test] fn test_behaviour_when_moving_to_start_of_previous_long_words() { let tests = array::IntoIter::new([ - ("Basic backward motion from the middle of a word", - vec![(1, Range::new(3, 3), Range::new(3, 0))]), - ("Starting from after boundary retreats the anchor", - vec![(1, Range::new(0, 8), Range::new(7, 0))]), - (" Jump to start of a word preceded by whitespace", - vec![(1, Range::new(5, 5), Range::new(5, 4))]), - (" Jump to start of line from start of word preceded by whitespace", - vec![(1, Range::new(4, 4), Range::new(3, 0))]), + ( + "Basic backward motion from the middle of a word", + vec![(1, Range::new(3, 3), Range::new(4, 0))], + ), + + // // Why do we want this behavior? The current behavior fails this + // // test, but seems better and more consistent. + // ("Starting from after boundary retreats the anchor", + // vec![(1, Range::new(0, 9), Range::new(8, 0))]), + + ( + " Jump to start of a word preceded by whitespace", + vec![(1, Range::new(5, 5), Range::new(6, 4))], + ), + ( + " Jump to start of line from start of word preceded by whitespace", + vec![(1, Range::new(3, 4), Range::new(4, 0))], + ), ("Previous anchor is irrelevant for backward motions", - vec![(1, Range::new(12, 5), Range::new(5, 0))]), - (" Starting from whitespace moves to first space in sequence", - vec![(1, Range::new(0, 3), Range::new(3, 0))]), + vec![(1, Range::new(12, 5), Range::new(6, 0))]), + ( + " Starting from whitespace moves to first space in sequence", + vec![(1, Range::new(0, 4), Range::new(4, 0))], + ), ("Identifiers_with_underscores are considered a single word", vec![(1, Range::new(0, 20), Range::new(20, 0))]), - ("Jumping\n \nback through a newline selects whitespace", - vec![(1, Range::new(0, 13), Range::new(11, 8))]), - ("Jumping to start of word from the end selects the word", - vec![(1, Range::new(6, 6), Range::new(6, 0))]), - ("alphanumeric.!,and.?=punctuation are treated exactly the same", - vec![ - (1, Range::new(30, 30), Range::new(30, 0)), - ]), - - ("... ... punctuation and spaces behave as expected", + ( + "Jumping\n \nback through a newline selects whitespace", + vec![(1, Range::new(0, 13), Range::new(12, 8))], + ), + ( + "Jumping to start of word from the end selects the word", + vec![(1, Range::new(6, 7), Range::new(7, 0))], + ), + ( + "alphanumeric.!,and.?=punctuation are treated exactly the same", + vec![(1, Range::new(29, 30), Range::new(30, 0))], + ), + ( + "... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 10), Range::new(9, 6)), - (1, Range::new(9, 6), Range::new(5, 0)), - ]), + (1, Range::new(0, 10), Range::new(10, 6)), + (1, Range::new(10, 6), Range::new(6, 0)), + ], + ), (".._.._ punctuation is joined by underscores into a single block", - vec![(1, Range::new(0, 5), Range::new(4, 0))]), - ("Newlines\n\nare bridged seamlessly.", - vec![ - (1, Range::new(0, 10), Range::new(7, 0)), - ]), - ("Jumping \n\n\n\n\nback from within a newline group selects previous block", - vec![ - (1, Range::new(0, 13), Range::new(10, 0)), - ]), - ("Failed motions do not modify the range", - vec![ - (0, Range::new(3, 0), Range::new(3, 0)), - ]), - ("Multiple motions at once resolve correctly", - vec![ - (3, Range::new(18, 18), Range::new(8, 0)), - ]), - ("Excessive motions are performed partially", - vec![ - (999, Range::new(40, 40), Range::new(9, 0)), - ]), - ("", // Edge case of moving backwards in empty string - vec![ - (1, Range::new(0, 0), Range::new(0, 0)), - ]), - ("\n\n\n\n\n", // Edge case of moving backwards in all newlines - vec![ - (1, Range::new(0, 0), Range::new(0, 0)), - ]), + vec![(1, Range::new(0, 6), Range::new(6, 0))]), + ( + "Newlines\n\nare bridged seamlessly.", + vec![(1, Range::new(0, 10), Range::new(8, 0))], + ), + ( + "Jumping \n\n\n\n\nback from within a newline group selects previous block", + vec![(1, Range::new(0, 13), Range::new(11, 0))], + ), + ( + "Failed motions do not modify the range", + vec![(0, Range::new(3, 0), Range::new(3, 0))], + ), + ( + "Multiple motions at once resolve correctly", + vec![(3, Range::new(19, 19), Range::new(9, 0))], + ), + ( + "Excessive motions are performed partially", + vec![(999, Range::new(40, 40), Range::new(10, 0))], + ), + ( + "", // Edge case of moving backwards in empty string + vec![(1, Range::new(0, 0), Range::new(0, 0))], + ), + ( + "\n\n\n\n\n", // Edge case of moving backwards in all newlines + vec![(1, Range::new(5, 5), Range::new(0, 0))], + ), (" \n \nJumping back through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 7), Range::new(6, 4)), - (1, Range::new(6, 4), Range::new(2, 0)), + (1, Range::new(0, 8), Range::new(7, 4)), + (1, Range::new(7, 4), Range::new(3, 0)), ]), ("ヒーリ..クス multibyte characters behave as normal characters, including when interacting with punctuation", vec![ - (1, Range::new(0, 7), Range::new(6, 0)), + (1, Range::new(0, 8), Range::new(8, 0)), ]), ]); @@ -870,42 +914,46 @@ mod test { fn test_behaviour_when_moving_to_end_of_next_words() { let tests = array::IntoIter::new([ ("Basic forward motion from the start of a word to the end of it", - vec![(1, Range::new(0, 0), Range::new(0, 4))]), + vec![(1, Range::new(0, 0), Range::new(0, 5))]), ("Basic forward motion from the end of a word to the end of the next", - vec![(1, Range::new(0, 4), Range::new(5, 12))]), + vec![(1, Range::new(0, 5), Range::new(5, 13))]), ("Basic forward motion from the middle of a word to the end of it", - vec![(1, Range::new(2, 2), Range::new(2, 4))]), + vec![(1, Range::new(2, 2), Range::new(2, 5))]), (" Jumping to end of a word preceded by whitespace", - vec![(1, Range::new(0, 0), Range::new(0, 10))]), - (" Starting from a boundary advances the anchor", - vec![(1, Range::new(0, 0), Range::new(1, 8))]), + vec![(1, Range::new(0, 0), Range::new(0, 11))]), + + // // Why do we want this behavior? The current behavior fails this + // // test, but seems better and more consistent. + // (" Starting from a boundary advances the anchor", + // vec![(1, Range::new(0, 0), Range::new(1, 9))]), + ("Previous anchor is irrelevant for end of word motion", - vec![(1, Range::new(12, 2), Range::new(2, 7))]), + vec![(1, Range::new(12, 2), Range::new(2, 8))]), ("Identifiers_with_underscores are considered a single word", - vec![(1, Range::new(0, 0), Range::new(0, 27))]), + vec![(1, Range::new(0, 0), Range::new(0, 28))]), ("Jumping\n into starting whitespace selects up to the end of next word", - vec![(1, Range::new(0, 6), Range::new(8, 15))]), + vec![(1, Range::new(0, 7), Range::new(8, 16))]), ("alphanumeric.!,and.?=punctuation are considered 'words' for the purposes of word motion", vec![ - (1, Range::new(0, 0), Range::new(0, 11)), - (1, Range::new(0, 11), Range::new(12, 14)), - (1, Range::new(12, 14), Range::new(15, 17)) + (1, Range::new(0, 0), Range::new(0, 12)), + (1, Range::new(0, 12), Range::new(12, 15)), + (1, Range::new(12, 15), Range::new(15, 18)) ]), ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 0), Range::new(0, 2)), - (1, Range::new(0, 2), Range::new(3, 8)), + (1, Range::new(0, 0), Range::new(0, 3)), + (1, Range::new(0, 3), Range::new(3, 9)), ]), (".._.._ punctuation is not joined by underscores into a single block", - vec![(1, Range::new(0, 0), Range::new(0, 1))]), + vec![(1, Range::new(0, 0), Range::new(0, 2))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 0), Range::new(0, 7)), - (1, Range::new(0, 7), Range::new(10, 12)), + (1, Range::new(0, 0), Range::new(0, 8)), + (1, Range::new(0, 8), Range::new(10, 13)), ]), ("Jumping\n\n\n\n\n\n from newlines to whitespace selects to end of next word.", vec![ - (1, Range::new(0, 8), Range::new(13, 19)), + (1, Range::new(0, 8), Range::new(13, 20)), ]), ("A failed motion does not modify the range", vec![ @@ -913,11 +961,11 @@ mod test { ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(0, 0), Range::new(16, 18)), + (3, Range::new(0, 0), Range::new(16, 19)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(0, 0), Range::new(31, 40)), + (999, Range::new(0, 0), Range::new(31, 41)), ]), ("", // Edge case of moving forward in empty string vec![ @@ -925,16 +973,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving forward in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 4)), + (1, Range::new(0, 0), Range::new(5, 5)), ]), ("\n \n \n Jumping through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 0), Range::new(1, 3)), - (1, Range::new(1, 3), Range::new(5, 7)), + (1, Range::new(0, 0), Range::new(1, 4)), + (1, Range::new(1, 4), Range::new(5, 8)), ]), ("ヒーリクス multibyte characters behave as normal characters", vec![ - (1, Range::new(0, 0), Range::new(0, 4)), + (1, Range::new(0, 0), Range::new(0, 5)), ]), ]); @@ -950,44 +998,44 @@ mod test { fn test_behaviour_when_moving_to_end_of_previous_words() { let tests = array::IntoIter::new([ ("Basic backward motion from the middle of a word", - vec![(1, Range::new(9, 9), Range::new(9, 5))]), + vec![(1, Range::new(9, 9), Range::new(10, 5))]), ("Starting from after boundary retreats the anchor", - vec![(1, Range::new(0, 13), Range::new(12, 8))]), + vec![(1, Range::new(0, 14), Range::new(13, 8))]), ("Jump to end of a word succeeded by whitespace", - vec![(1, Range::new(10, 10), Range::new(10, 4))]), + vec![(1, Range::new(11, 11), Range::new(11, 4))]), (" Jump to start of line from end of word preceded by whitespace", - vec![(1, Range::new(7, 7), Range::new(7, 0))]), + vec![(1, Range::new(8, 8), Range::new(8, 0))]), ("Previous anchor is irrelevant for backward motions", - vec![(1, Range::new(26, 12), Range::new(12, 8))]), + vec![(1, Range::new(26, 12), Range::new(13, 8))]), (" Starting from whitespace moves to first space in sequence", - vec![(1, Range::new(0, 3), Range::new(3, 0))]), + vec![(1, Range::new(0, 4), Range::new(4, 0))]), ("Test identifiers_with_underscores are considered a single word", vec![(1, Range::new(0, 25), Range::new(25, 4))]), ("Jumping\n \nback through a newline selects whitespace", - vec![(1, Range::new(0, 13), Range::new(11, 8))]), + vec![(1, Range::new(0, 13), Range::new(12, 8))]), ("Jumping to start of word from the end selects the whole word", - vec![(1, Range::new(15, 15), Range::new(15, 10))]), + vec![(1, Range::new(16, 16), Range::new(16, 10))]), ("alphanumeric.!,and.?=punctuation are considered 'words' for the purposes of word motion", vec![ - (1, Range::new(30, 30), Range::new(30, 21)), - (1, Range::new(30, 21), Range::new(20, 18)), - (1, Range::new(20, 18), Range::new(17, 15)) + (1, Range::new(30, 30), Range::new(31, 21)), + (1, Range::new(31, 21), Range::new(21, 18)), + (1, Range::new(21, 18), Range::new(18, 15)) ]), ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 10), Range::new(9, 9)), - (1, Range::new(9, 6), Range::new(5, 3)), + (1, Range::new(0, 10), Range::new(9, 3)), + (1, Range::new(9, 3), Range::new(3, 0)), ]), (".._.._ punctuation is not joined by underscores into a single block", - vec![(1, Range::new(0, 5), Range::new(4, 3))]), + vec![(1, Range::new(0, 5), Range::new(5, 3))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 10), Range::new(7, 0)), + (1, Range::new(0, 10), Range::new(8, 0)), ]), ("Jumping \n\n\n\n\nback from within a newline group selects previous block", vec![ - (1, Range::new(0, 13), Range::new(10, 7)), + (1, Range::new(0, 13), Range::new(11, 7)), ]), ("Failed motions do not modify the range", vec![ @@ -995,11 +1043,11 @@ mod test { ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(23, 23), Range::new(15, 8)), + (3, Range::new(24, 24), Range::new(16, 8)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(40, 40), Range::new(8, 0)), + (999, Range::new(40, 40), Range::new(9, 0)), ]), ("", // Edge case of moving backwards in empty string vec![ @@ -1007,16 +1055,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving backwards in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 0)), + (1, Range::new(5, 5), Range::new(0, 0)), ]), (" \n \nJumping back through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 7), Range::new(6, 4)), - (1, Range::new(6, 4), Range::new(2, 0)), + (1, Range::new(0, 8), Range::new(7, 4)), + (1, Range::new(7, 4), Range::new(3, 0)), ]), ("Test ヒーリクス multibyte characters behave as normal characters", vec![ - (1, Range::new(0, 9), Range::new(9, 4)), + (1, Range::new(0, 10), Range::new(10, 4)), ]), ]); @@ -1032,40 +1080,44 @@ mod test { fn test_behaviour_when_moving_to_end_of_next_long_words() { let tests = array::IntoIter::new([ ("Basic forward motion from the start of a word to the end of it", - vec![(1, Range::new(0, 0), Range::new(0, 4))]), + vec![(1, Range::new(0, 0), Range::new(0, 5))]), ("Basic forward motion from the end of a word to the end of the next", - vec![(1, Range::new(0, 4), Range::new(5, 12))]), + vec![(1, Range::new(0, 5), Range::new(5, 13))]), ("Basic forward motion from the middle of a word to the end of it", - vec![(1, Range::new(2, 2), Range::new(2, 4))]), + vec![(1, Range::new(2, 2), Range::new(2, 5))]), (" Jumping to end of a word preceded by whitespace", - vec![(1, Range::new(0, 0), Range::new(0, 10))]), - (" Starting from a boundary advances the anchor", - vec![(1, Range::new(0, 0), Range::new(1, 8))]), + vec![(1, Range::new(0, 0), Range::new(0, 11))]), + + // // Why do we want this behavior? The current behavior fails this + // // test, but seems better and more consistent. + // (" Starting from a boundary advances the anchor", + // vec![(1, Range::new(0, 0), Range::new(1, 9))]), + ("Previous anchor is irrelevant for end of word motion", - vec![(1, Range::new(12, 2), Range::new(2, 7))]), + vec![(1, Range::new(12, 2), Range::new(2, 8))]), ("Identifiers_with_underscores are considered a single word", - vec![(1, Range::new(0, 0), Range::new(0, 27))]), + vec![(1, Range::new(0, 0), Range::new(0, 28))]), ("Jumping\n into starting whitespace selects up to the end of next word", - vec![(1, Range::new(0, 6), Range::new(8, 15))]), + vec![(1, Range::new(0, 7), Range::new(8, 16))]), ("alphanumeric.!,and.?=punctuation are treated the same way", vec![ - (1, Range::new(0, 0), Range::new(0, 31)), + (1, Range::new(0, 0), Range::new(0, 32)), ]), ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 0), Range::new(0, 2)), - (1, Range::new(0, 2), Range::new(3, 8)), + (1, Range::new(0, 0), Range::new(0, 3)), + (1, Range::new(0, 3), Range::new(3, 9)), ]), (".._.._ punctuation is joined by underscores into a single block", - vec![(1, Range::new(0, 0), Range::new(0, 5))]), + vec![(1, Range::new(0, 0), Range::new(0, 6))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 0), Range::new(0, 7)), - (1, Range::new(0, 7), Range::new(10, 12)), + (1, Range::new(0, 0), Range::new(0, 8)), + (1, Range::new(0, 8), Range::new(10, 13)), ]), ("Jumping\n\n\n\n\n\n from newlines to whitespace selects to end of next word.", vec![ - (1, Range::new(0, 8), Range::new(13, 19)), + (1, Range::new(0, 9), Range::new(13, 20)), ]), ("A failed motion does not modify the range", vec![ @@ -1073,11 +1125,11 @@ mod test { ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(0, 0), Range::new(16, 18)), + (3, Range::new(0, 0), Range::new(16, 19)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(0, 0), Range::new(31, 40)), + (999, Range::new(0, 0), Range::new(31, 41)), ]), ("", // Edge case of moving forward in empty string vec![ @@ -1085,16 +1137,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving forward in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 4)), + (1, Range::new(0, 0), Range::new(5, 5)), ]), ("\n \n \n Jumping through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 0), Range::new(1, 3)), - (1, Range::new(1, 3), Range::new(5, 7)), + (1, Range::new(0, 0), Range::new(1, 4)), + (1, Range::new(1, 4), Range::new(5, 8)), ]), ("ヒーリ..クス multibyte characters behave as normal characters, including when they interact with punctuation", vec![ - (1, Range::new(0, 0), Range::new(0, 6)), + (1, Range::new(0, 0), Range::new(0, 7)), ]), ]); diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 41b342b2..1f84db6b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -498,6 +498,7 @@ fn move_next_word_start(cx: &mut Context) { view.id, doc.selection(view.id) .clone() + .min_width_1(doc.text().slice(..)) .transform(|range| movement::move_next_word_start(doc.text().slice(..), range, count)), ); } @@ -509,6 +510,7 @@ fn move_prev_word_start(cx: &mut Context) { view.id, doc.selection(view.id) .clone() + .min_width_1(doc.text().slice(..)) .transform(|range| movement::move_prev_word_start(doc.text().slice(..), range, count)), ); } @@ -520,6 +522,7 @@ fn move_next_word_end(cx: &mut Context) { view.id, doc.selection(view.id) .clone() + .min_width_1(doc.text().slice(..)) .transform(|range| movement::move_next_word_end(doc.text().slice(..), range, count)), ); } @@ -529,9 +532,12 @@ fn move_next_long_word_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); doc.set_selection( view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_next_long_word_start(doc.text().slice(..), range, count) - }), + doc.selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)) + .transform(|range| { + movement::move_next_long_word_start(doc.text().slice(..), range, count) + }), ); } @@ -540,9 +546,12 @@ fn move_prev_long_word_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); doc.set_selection( view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_prev_long_word_start(doc.text().slice(..), range, count) - }), + doc.selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)) + .transform(|range| { + movement::move_prev_long_word_start(doc.text().slice(..), range, count) + }), ); } @@ -551,9 +560,12 @@ fn move_next_long_word_end(cx: &mut Context) { let (view, doc) = current!(cx.editor); doc.set_selection( view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_next_long_word_end(doc.text().slice(..), range, count) - }), + doc.selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)) + .transform(|range| { + movement::move_next_long_word_end(doc.text().slice(..), range, count) + }), ); } @@ -574,12 +586,15 @@ fn extend_next_word_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); doc.set_selection( view.id, - doc.selection(view.id).clone().transform(|range| { - let text = doc.text().slice(..); - let word = movement::move_next_word_start(text, range, count); - let pos = word.head; - range.put(text, pos, true) - }), + doc.selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)) + .transform(|range| { + let text = doc.text().slice(..); + let word = movement::move_next_word_start(text, range, count); + let pos = word.head; + range.put(text, pos, true) + }), ); } @@ -588,12 +603,15 @@ fn extend_prev_word_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); doc.set_selection( view.id, - doc.selection(view.id).clone().transform(|range| { - let text = doc.text().slice(..); - let word = movement::move_prev_word_start(text, range, count); - let pos = word.head; - range.put(text, pos, true) - }), + doc.selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)) + .transform(|range| { + let text = doc.text().slice(..); + let word = movement::move_prev_word_start(text, range, count); + let pos = word.head; + range.put(text, pos, true) + }), ); } @@ -602,12 +620,15 @@ fn extend_next_word_end(cx: &mut Context) { let (view, doc) = current!(cx.editor); doc.set_selection( view.id, - doc.selection(view.id).clone().transform(|range| { - let text = doc.text().slice(..); - let word = movement::move_next_word_end(text, range, count); - let pos = word.head; - range.put(text, pos, true) - }), + doc.selection(view.id) + .clone() + .min_width_1(doc.text().slice(..)) + .transform(|range| { + let text = doc.text().slice(..); + let word = movement::move_next_word_end(text, range, count); + let pos = word.head; + range.put(text, pos, true) + }), ); } -- cgit v1.2.3-70-g09d2 From b0311f4fc246f370bb317ab0b1fcb89823347caa Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Mon, 19 Jul 2021 09:25:10 -0700 Subject: Fixed primary cursor position calculation to use 1-width semantics. This had a bunch of knock-on effects that were buggy, such as bracket match highlighting. --- helix-core/src/selection.rs | 11 +++++++-- helix-term/src/commands.rs | 50 ++++++++++++++++++++++++++++------------- helix-term/src/ui/completion.rs | 8 +++---- helix-term/src/ui/editor.rs | 9 +++++--- helix-view/src/editor.rs | 7 +++--- helix-view/src/view.rs | 2 +- 6 files changed, 58 insertions(+), 29 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 21a6c108..7d2526b0 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -267,8 +267,15 @@ impl Selection { } #[must_use] - pub fn cursor(&self) -> usize { - self.primary().head + pub fn cursor(&self, text: RopeSlice) -> usize { + let range = self.primary(); + + // For 1-width cursor semantics. + if range.anchor < range.head { + prev_grapheme_boundary(text, range.head) + } else { + range.head + } } /// Ensure selection containing only the primary selection. diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index cea5a24e..add83b41 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -121,7 +121,7 @@ enum Align { } fn align_view(doc: &Document, view: &mut View, align: Align) { - let pos = doc.selection(view.id).cursor(); + let pos = doc.selection(view.id).cursor(doc.text().slice(..)); let line = doc.text().char_to_line(pos); let relative = match align { @@ -868,7 +868,10 @@ fn switch_to_lowercase(cx: &mut Context) { fn scroll(cx: &mut Context, offset: usize, direction: Direction) { use Direction::*; let (view, doc) = current!(cx.editor); - let cursor = coords_at_pos(doc.text().slice(..), doc.selection(view.id).cursor()); + let cursor = coords_at_pos( + doc.text().slice(..), + doc.selection(view.id).cursor(doc.text().slice(..)), + ); let doc_last_line = doc.text().len_lines() - 1; let last_line = view.last_line(doc); @@ -1038,7 +1041,7 @@ fn split_selection_on_newline(cx: &mut Context) { fn search_impl(doc: &mut Document, view: &mut View, contents: &str, regex: &Regex, extend: bool) { let text = doc.text(); let selection = doc.selection(view.id); - let start = text.char_to_byte(selection.cursor()); + let start = text.char_to_byte(selection.cursor(text.slice(..))); // use find_at to find the next match after the cursor, loop around the end // Careful, `Regex` uses `bytes` as offsets, not character indices! @@ -2446,7 +2449,11 @@ fn goto_definition(cx: &mut Context) { let offset_encoding = language_server.offset_encoding(); - let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding); + let pos = pos_to_lsp_pos( + doc.text(), + doc.selection(view.id).cursor(doc.text().slice(..)), + offset_encoding, + ); // TODO: handle fails let future = language_server.goto_definition(doc.identifier(), pos, None); @@ -2483,7 +2490,11 @@ fn goto_type_definition(cx: &mut Context) { let offset_encoding = language_server.offset_encoding(); - let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding); + let pos = pos_to_lsp_pos( + doc.text(), + doc.selection(view.id).cursor(doc.text().slice(..)), + offset_encoding, + ); // TODO: handle fails let future = language_server.goto_type_definition(doc.identifier(), pos, None); @@ -2520,7 +2531,11 @@ fn goto_implementation(cx: &mut Context) { let offset_encoding = language_server.offset_encoding(); - let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding); + let pos = pos_to_lsp_pos( + doc.text(), + doc.selection(view.id).cursor(doc.text().slice(..)), + offset_encoding, + ); // TODO: handle fails let future = language_server.goto_implementation(doc.identifier(), pos, None); @@ -2557,7 +2572,11 @@ fn goto_reference(cx: &mut Context) { let offset_encoding = language_server.offset_encoding(); - let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding); + let pos = pos_to_lsp_pos( + doc.text(), + doc.selection(view.id).cursor(doc.text().slice(..)), + offset_encoding, + ); // TODO: handle fails let future = language_server.goto_reference(doc.identifier(), pos, None); @@ -2616,7 +2635,7 @@ fn goto_next_diag(cx: &mut Context) { let editor = &mut cx.editor; let (view, doc) = current!(editor); - let cursor_pos = doc.selection(view.id).cursor(); + let cursor_pos = doc.selection(view.id).cursor(doc.text().slice(..)); let diag = if let Some(diag) = doc .diagnostics() .iter() @@ -2637,7 +2656,7 @@ fn goto_prev_diag(cx: &mut Context) { let editor = &mut cx.editor; let (view, doc) = current!(editor); - let cursor_pos = doc.selection(view.id).cursor(); + let cursor_pos = doc.selection(view.id).cursor(doc.text().slice(..)); let diag = if let Some(diag) = doc .diagnostics() .iter() @@ -2665,7 +2684,7 @@ fn signature_help(cx: &mut Context) { let pos = pos_to_lsp_pos( doc.text(), - doc.selection(view.id).cursor(), + doc.selection(view.id).cursor(doc.text().slice(..)), language_server.offset_encoding(), ); @@ -3403,13 +3422,14 @@ fn completion(cx: &mut Context) { }; let offset_encoding = language_server.offset_encoding(); + let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); - let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding); + let pos = pos_to_lsp_pos(doc.text(), cursor, offset_encoding); // TODO: handle fails let future = language_server.completion(doc.identifier(), pos, None); - let trigger_offset = doc.selection(view.id).cursor(); + let trigger_offset = cursor; cx.callback( future, @@ -3459,7 +3479,7 @@ fn hover(cx: &mut Context) { let pos = pos_to_lsp_pos( doc.text(), - doc.selection(view.id).cursor(), + doc.selection(view.id).cursor(doc.text().slice(..)), language_server.offset_encoding(), ); @@ -3525,7 +3545,7 @@ fn match_brackets(cx: &mut Context) { let (view, doc) = current!(cx.editor); if let Some(syntax) = doc.syntax() { - let pos = doc.selection(view.id).cursor(); + let pos = doc.selection(view.id).cursor(doc.text().slice(..)); if let Some(pos) = match_brackets::find(syntax, doc.text(), pos) { let selection = Selection::point(pos); doc.set_selection(view.id, selection); @@ -3629,7 +3649,7 @@ fn align_view_bottom(cx: &mut Context) { fn align_view_middle(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let pos = doc.selection(view.id).cursor(); + let pos = doc.selection(view.id).cursor(doc.text().slice(..)); let pos = coords_at_pos(doc.text().slice(..), pos); const OFFSET: usize = 7; // gutters diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index be6db42c..942a2483 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -86,7 +86,7 @@ impl Completion { let item = item.unwrap(); // if more text was entered, remove it - let cursor = doc.selection(view.id).cursor(); + let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); if trigger_offset < cursor { let remove = Transaction::change( doc.text(), @@ -109,7 +109,7 @@ impl Completion { ) } else { let text = item.insert_text.as_ref().unwrap_or(&item.label); - let cursor = doc.selection(view.id).cursor(); + let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); Transaction::change( doc.text(), vec![(cursor, cursor, Some(text.as_str().into()))].into_iter(), @@ -155,7 +155,7 @@ impl Completion { // TODO: hooks should get processed immediately so maybe do it after select!(), before // looping? - let cursor = doc.selection(view.id).cursor(); + let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); if self.trigger_offset <= cursor { let fragment = doc.text().slice(self.trigger_offset..cursor); let text = Cow::from(fragment); @@ -212,7 +212,7 @@ impl Component for Completion { .language() .and_then(|scope| scope.strip_prefix("source.")) .unwrap_or(""); - let cursor_pos = doc.selection(view.id).cursor(); + let cursor_pos = doc.selection(view.id).cursor(doc.text().slice(..)); let cursor_pos = (helix_core::coords_at_pos(doc.text().slice(..), cursor_pos).row - view.first_line) as u16; diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 6a588e12..acf60905 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -398,7 +398,7 @@ impl EditorView { // TODO: set cursor position for IME if let Some(syntax) = doc.syntax() { use helix_core::match_brackets; - let pos = doc.selection(view.id).cursor(); + let pos = doc.selection(view.id).cursor(doc.text().slice(..)); let pos = match_brackets::find(syntax, doc.text(), pos) .and_then(|pos| view.screen_coords_at_pos(doc, text, pos)); @@ -443,7 +443,7 @@ impl EditorView { widgets::{Paragraph, Widget}, }; - let cursor = doc.selection(view.id).cursor(); + let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); let diagnostics = doc.diagnostics().iter().filter(|diagnostic| { diagnostic.range.start <= cursor && diagnostic.range.end >= cursor @@ -555,7 +555,10 @@ impl EditorView { // _ => "indent:ERROR", // }; let position_info = { - let pos = coords_at_pos(doc.text().slice(..), doc.selection(view.id).cursor()); + let pos = coords_at_pos( + doc.text().slice(..), + doc.selection(view.id).cursor(doc.text().slice(..)), + ); format!("{}:{}", pos.row + 1, pos.col + 1) // convert to 1-indexing }; diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index cd9d0a92..1cd0af02 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -141,12 +141,11 @@ impl Editor { let (view, doc) = current!(self); // initialize selection for view - let selection = doc - .selections + doc.selections .entry(view.id) .or_insert_with(|| Selection::point(0)); // TODO: reuse align_view - let pos = selection.cursor(); + let pos = doc.selection(view.id).cursor(doc.text().slice(..)); let line = doc.text().char_to_line(pos); view.first_line = line.saturating_sub(view.area.height as usize / 2); @@ -296,7 +295,7 @@ impl Editor { const OFFSET: u16 = 7; // 1 diagnostic + 5 linenr + 1 gutter let view = view!(self); let doc = &self.documents[view.doc]; - let cursor = doc.selection(view.id).cursor(); + let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); if let Some(mut pos) = view.screen_coords_at_pos(doc, doc.text().slice(..), cursor) { pos.col += view.area.x as usize + OFFSET as usize; pos.row += view.area.y as usize; diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index ccb61646..67585ed3 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -84,7 +84,7 @@ impl View { } pub fn ensure_cursor_in_view(&mut self, doc: &Document) { - let cursor = doc.selection(self.id).cursor(); + let cursor = doc.selection(self.id).cursor(doc.text().slice(..)); let pos = coords_at_pos(doc.text().slice(..), cursor); let line = pos.row; let col = pos.col; -- cgit v1.2.3-70-g09d2 From 13b0784009cd0c1150ba7fac2bac1465360be6dd Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Mon, 19 Jul 2021 17:44:18 -0700 Subject: Fix extend line behavior. --- helix-term/src/commands.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index add83b41..d1092f20 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1125,17 +1125,17 @@ fn extend_line(cx: &mut Context) { let (view, doc) = current!(cx.editor); let text = doc.text(); - let pos = doc.selection(view.id).primary().min_width_1(text.slice(..)); + let range = doc.selection(view.id).primary().min_width_1(text.slice(..)); - let line_max = text.len_lines(); - let start_line = text.char_to_line(pos.from()).min(line_max); - let end_line = (text.char_to_line(pos.to()) + count).min(line_max); + let start_line = text.char_to_line(range.from()); + let end_line = (text.char_to_line(range.to().saturating_sub(1).max(range.from())) + count) + .min(text.len_lines()); let start = text.line_to_char(start_line); let mut end = text.line_to_char(end_line); - if pos.from() == start && pos.to() == end { - end = text.line_to_char((end_line + 1).min(line_max)); + if range.from() == start && range.to() == end { + end = text.line_to_char((end_line + 1).min(text.len_lines())); } doc.set_selection(view.id, Selection::single(start, end)); @@ -1148,12 +1148,15 @@ fn extend_to_line_bounds(cx: &mut Context) { view.id, doc.selection(view.id).clone().transform(|range| { let text = doc.text(); - let start = text.line_to_char(text.char_to_line(range.from())); - let end = text - .line_to_char(text.char_to_line(range.to()) + 1) - .saturating_sub(1); - if range.anchor < range.head { + let start_line = text.char_to_line(range.from()); + let end_line = (text.char_to_line(range.to().saturating_sub(1).max(range.from())) + 1) + .min(text.len_lines()); + + let start = text.line_to_char(start_line); + let end = text.line_to_char(end_line); + + if range.anchor <= range.head { Range::new(start, end) } else { Range::new(end, start) -- cgit v1.2.3-70-g09d2 From 1792dc6f935f581e7fed3842b1f01e2d5d3d7e05 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Mon, 19 Jul 2021 18:29:26 -0700 Subject: Make search work a little nicer when there are already selections. Specifically, if you have text like "aaaaaaaaa" and you search for "a", the new behavior will actually progress through all of the "a"s, whereas the previous behavior would be stuck on a single one. --- helix-term/src/commands.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index d1092f20..5964e354 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1041,7 +1041,25 @@ fn split_selection_on_newline(cx: &mut Context) { fn search_impl(doc: &mut Document, view: &mut View, contents: &str, regex: &Regex, extend: bool) { let text = doc.text(); let selection = doc.selection(view.id); - let start = text.char_to_byte(selection.cursor(text.slice(..))); + let start = { + let range = selection.primary(); + + // This is a little bit weird. Due to 1-width cursor semantics, we + // would typically want the search to always begin at the visual left-side + // of the head. However, when there's already a selection from e.g. a + // previous search result, we don't want to include any of that selection + // in the subsequent search. The code below makes a compromise between the + // two behaviors that hopefully behaves the way most people expect most of + // the time. + if range.anchor <= range.head { + text.char_to_byte(range.head) + } else { + text.char_to_byte(graphemes::next_grapheme_boundary( + text.slice(..), + range.head, + )) + } + }; // use find_at to find the next match after the cursor, loop around the end // Careful, `Regex` uses `bytes` as offsets, not character indices! -- cgit v1.2.3-70-g09d2 From e8a3980e464a9c98c3f76cada6c46a66498dc2bf Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Tue, 20 Jul 2021 10:56:27 -0700 Subject: Fix line-wise `p` pasting before the current line instead of after. --- helix-term/src/commands.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 5964e354..61c5096d 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3093,7 +3093,10 @@ fn paste_impl( // paste linewise before (Paste::Before, true) => text.line_to_char(text.char_to_line(range.from())), // paste linewise after - (Paste::After, true) => text.line_to_char(text.char_to_line(range.to())), + (Paste::After, true) => { + let idx = range.to().saturating_sub(1).max(range.from()); + text.line_to_char((text.char_to_line(idx) + 1).min(text.len_lines())) + } // paste insert (Paste::Before, false) => range.from(), // paste append -- cgit v1.2.3-70-g09d2 From 1c6b5581f01371a00dc7f6f6e1720ad8af61ec7a Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Tue, 20 Jul 2021 11:58:56 -0700 Subject: Fix various bugs related to goto-end-of-line command. This also fixes a bug with `Selection::normalize()`, that could result in an out-of-bounds primary index. --- helix-core/src/selection.rs | 6 +++--- helix-term/src/commands.rs | 29 +++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 9 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index f1625f99..c08f504d 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -357,14 +357,14 @@ impl Selection { let mut prev_i = 0; for i in 1..self.ranges.len() { if self.ranges[prev_i].overlaps(&self.ranges[i]) { - if i == self.primary_index { - self.primary_index = prev_i; - } self.ranges[prev_i] = self.ranges[prev_i].merge(self.ranges[i]); } else { prev_i += 1; self.ranges[prev_i] = self.ranges[i]; } + if i == self.primary_index { + self.primary_index = prev_i; + } } self.ranges.truncate(prev_i + 1); diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 61c5096d..a155e19e 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -396,11 +396,19 @@ fn goto_line_end(cx: &mut Context) { view.id, doc.selection(view.id).clone().transform(|range| { let text = doc.text().slice(..); - let line = text.char_to_line(range.head); - let pos = line_end_char_index(&text, line); - let pos = graphemes::nth_prev_grapheme_boundary(text, pos, 1); - let pos = range.head.max(pos).max(text.line_to_char(line)); + let head = if range.anchor < range.head { + graphemes::prev_grapheme_boundary(text, range.head) + } else { + range.head + }; + let line = text.char_to_line(head); + + let mut pos = line_end_char_index(&text, line); + if doc.mode != Mode::Select { + pos = graphemes::prev_grapheme_boundary(text, pos); + } + pos = head.max(pos).max(text.line_to_char(line)); range.put(text, pos, doc.mode == Mode::Select) }), @@ -414,9 +422,18 @@ fn goto_line_end_newline(cx: &mut Context) { view.id, doc.selection(view.id).clone().transform(|range| { let text = doc.text().slice(..); - let line = text.char_to_line(range.head); - let pos = line_end_char_index(&text, line); + let head = if range.anchor < range.head { + graphemes::prev_grapheme_boundary(text, range.head) + } else { + range.head + }; + let line = text.char_to_line(head); + + let mut pos = text.line_to_char((line + 1).min(text.len_lines())); + if doc.mode != Mode::Select { + pos = graphemes::prev_grapheme_boundary(text, pos); + } range.put(text, pos, doc.mode == Mode::Select) }), ); -- cgit v1.2.3-70-g09d2 From 1194fc842a7b91045b6b71f839fc9a9b5a5b19ef Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Tue, 20 Jul 2021 12:40:58 -0700 Subject: Use new `Range::line_range()` method in more places, as appropriate. --- helix-term/src/commands.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index a155e19e..72462741 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1162,15 +1162,12 @@ fn extend_line(cx: &mut Context) { let text = doc.text(); let range = doc.selection(view.id).primary().min_width_1(text.slice(..)); - let start_line = text.char_to_line(range.from()); - let end_line = (text.char_to_line(range.to().saturating_sub(1).max(range.from())) + count) - .min(text.len_lines()); - + let (start_line, end_line) = range.line_range(text.slice(..)); let start = text.line_to_char(start_line); - let mut end = text.line_to_char(end_line); + let mut end = text.line_to_char((end_line + count).min(text.len_lines())); if range.from() == start && range.to() == end { - end = text.line_to_char((end_line + 1).min(text.len_lines())); + end = text.line_to_char((end_line + count + 1).min(text.len_lines())); } doc.set_selection(view.id, Selection::single(start, end)); @@ -1184,12 +1181,9 @@ fn extend_to_line_bounds(cx: &mut Context) { doc.selection(view.id).clone().transform(|range| { let text = doc.text(); - let start_line = text.char_to_line(range.from()); - let end_line = (text.char_to_line(range.to().saturating_sub(1).max(range.from())) + 1) - .min(text.len_lines()); - + let (start_line, end_line) = range.line_range(text.slice(..)); let start = text.line_to_char(start_line); - let end = text.line_to_char(end_line); + let end = text.line_to_char((end_line + 1).min(text.len_lines())); if range.anchor <= range.head { Range::new(start, end) @@ -3111,8 +3105,8 @@ fn paste_impl( (Paste::Before, true) => text.line_to_char(text.char_to_line(range.from())), // paste linewise after (Paste::After, true) => { - let idx = range.to().saturating_sub(1).max(range.from()); - text.line_to_char((text.char_to_line(idx) + 1).min(text.len_lines())) + let line = range.line_range(text.slice(..)).1; + text.line_to_char((line + 1).min(text.len_lines())) } // paste insert (Paste::Before, false) => range.from(), -- cgit v1.2.3-70-g09d2 From c848ed7abc7605c1bb1c6c98dfac23cadcb9e439 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Tue, 20 Jul 2021 18:15:34 -0700 Subject: Fixes for misc bugs with view movement. --- helix-core/src/movement.rs | 39 ++++++------ helix-core/src/position.rs | 141 +++++++++++++++++++++++++++++++++----------- helix-core/src/selection.rs | 2 +- helix-term/src/commands.rs | 2 +- helix-view/src/view.rs | 4 +- 5 files changed, 132 insertions(+), 56 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index 21e97ae8..01bec47a 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -7,9 +7,8 @@ use crate::{ coords_at_pos, graphemes::{ next_grapheme_boundary, nth_next_grapheme_boundary, nth_prev_grapheme_boundary, - prev_grapheme_boundary, RopeGraphemes, + prev_grapheme_boundary, }, - line_ending::line_without_line_ending, pos_at_coords, Position, Range, RopeSlice, }; @@ -85,10 +84,8 @@ pub fn move_vertically( } else { (row + count).min(slice.len_lines().saturating_sub(1)) }; - let max_col = RopeGraphemes::new(line_without_line_ending(&slice, new_row)).count(); - let new_col = col.max(horiz as usize).min(max_col); - - pos_at_coords(slice, Position::new(new_row, new_col)) + let new_col = col.max(horiz as usize); + pos_at_coords(slice, Position::new(new_row, new_col), true) }; // Compute the new range according to the type of movement. @@ -365,7 +362,7 @@ mod test { fn test_vertical_move() { let text = Rope::from("abcd\nefg\nwrs"); let slice = text.slice(..); - let pos = pos_at_coords(slice, (0, 4).into()); + let pos = pos_at_coords(slice, (0, 4).into(), true); let range = Range::new(pos, pos); assert_eq!( @@ -381,7 +378,7 @@ mod test { fn horizontal_moves_through_single_line_text() { let text = Rope::from(SINGLE_LINE_SAMPLE); let slice = text.slice(..); - let position = pos_at_coords(slice, (0, 0).into()); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); @@ -404,7 +401,7 @@ mod test { fn horizontal_moves_through_multiline_text() { let text = Rope::from(MULTILINE_SAMPLE); let slice = text.slice(..); - let position = pos_at_coords(slice, (0, 0).into()); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); @@ -431,7 +428,7 @@ mod test { fn selection_extending_moves_in_single_line_text() { let text = Rope::from(SINGLE_LINE_SAMPLE); let slice = text.slice(..); - let position = pos_at_coords(slice, (0, 0).into()); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); let original_anchor = range.anchor; @@ -452,7 +449,7 @@ mod test { fn vertical_moves_in_single_column() { let text = Rope::from(MULTILINE_SAMPLE); let slice = text.slice(..); - let position = pos_at_coords(slice, (0, 0).into()); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); let moves_and_expected_coordinates = IntoIter::new([ ((Direction::Forward, 1usize), (1, 0)), @@ -477,7 +474,7 @@ mod test { fn vertical_moves_jumping_column() { let text = Rope::from(MULTILINE_SAMPLE); let slice = text.slice(..); - let position = pos_at_coords(slice, (0, 0).into()); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); enum Axis { @@ -510,10 +507,10 @@ mod test { } #[test] - fn multibyte_character_column_jumps() { + fn multibyte_character_wide_column_jumps() { let text = Rope::from(MULTIBYTE_CHARACTER_SAMPLE); let slice = text.slice(..); - let position = pos_at_coords(slice, (0, 0).into()); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); // FIXME: The behaviour captured in this test diverges from both Kakoune and Vim. These @@ -524,10 +521,16 @@ mod test { V, } let moves_and_expected_coordinates = IntoIter::new([ - // Places cursor at the fourth kana - ((Axis::H, Direction::Forward, 4), (0, 4)), - // Descent places cursor at the fourth character. - ((Axis::V, Direction::Forward, 1usize), (1, 4)), + // Places cursor at the fourth kana (each of which are double-wide, + // so the visual column is 8). + ((Axis::H, Direction::Forward, 4), (0, 8)), + // Descent places cursor at the 8th character. + ((Axis::V, Direction::Forward, 1usize), (1, 8)), + // Moving back a single-width character. + ((Axis::H, Direction::Backward, 1usize), (1, 7)), + // Jumping back up into the middle of a double-width character shifts + // the column to the start of that character. + ((Axis::V, Direction::Backward, 1usize), (0, 6)), ]); for ((axis, direction, amount), coordinates) in moves_and_expected_coordinates { diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs index c4e8c9d6..15627687 100644 --- a/helix-core/src/position.rs +++ b/helix-core/src/position.rs @@ -1,6 +1,9 @@ +use std::borrow::Cow; + use crate::{ chars::char_is_line_ending, - graphemes::{nth_next_grapheme_boundary, RopeGraphemes}, + graphemes::{ensure_grapheme_boundary_prev, grapheme_width, RopeGraphemes}, + line_ending::line_end_char_index, RopeSlice, }; @@ -52,23 +55,58 @@ impl From for tree_sitter::Point { } } /// Convert a character index to (line, column) coordinates. +/// +/// TODO: this should be split into two methods: one for visual +/// row/column, and one for "objective" row/column (possibly with +/// the column specified in `char`s). The former would be used +/// for cursor movement, and the latter would be used for e.g. the +/// row:column display in the status line. pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position { - // TODO: this isn't correct. This needs to work in terms of - // visual horizontal position, not graphemes. let line = text.char_to_line(pos); + let line_start = text.line_to_char(line); - let col = RopeGraphemes::new(text.slice(line_start..pos)).count(); + let pos = ensure_grapheme_boundary_prev(text, pos); + let col = RopeGraphemes::new(text.slice(line_start..pos)) + .map(|g| { + let g: Cow = g.into(); + grapheme_width(&g) + }) + .sum(); + Position::new(line, col) } /// Convert (line, column) coordinates to a character index. -pub fn pos_at_coords(text: RopeSlice, coords: Position) -> usize { - // TODO: this isn't correct. This needs to work in terms of - // visual horizontal position, not graphemes. +/// +/// `is_1_width` specifies whether the position should be treated +/// as a block cursor or not. This effects how line-ends are handled. +/// `false` corresponds to properly round-tripping with `coords_at_pos()`, +/// whereas `true` will ensure that block cursors don't jump off the +/// end of the line. +pub fn pos_at_coords(text: RopeSlice, coords: Position, is_1_width: bool) -> usize { let Position { row, col } = coords; let line_start = text.line_to_char(row); - // line_start + col - nth_next_grapheme_boundary(text, line_start, col) + let line_end = if is_1_width { + line_end_char_index(&text, row) + } else { + text.line_to_char((row + 1).min(text.len_lines())) + }; + + let mut prev_col = 0; + let mut col_char_offset = 0; + for g in RopeGraphemes::new(text.slice(line_start..line_end)) { + let g: Cow = g.into(); + let next_col = prev_col + grapheme_width(&g); + + if next_col > col { + break; + } + + prev_col = next_col; + col_char_offset += g.chars().count(); + } + + line_start + col_char_offset } #[cfg(test)] @@ -84,13 +122,24 @@ mod test { #[test] fn test_coords_at_pos() { - // let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ"); - // let slice = text.slice(..); - // assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); - // assert_eq!(coords_at_pos(slice, 5), (0, 5).into()); // position on \n - // assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); // position on w - // assert_eq!(coords_at_pos(slice, 7), (1, 1).into()); // position on o - // assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d + let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ"); + let slice = text.slice(..); + assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); + assert_eq!(coords_at_pos(slice, 5), (0, 5).into()); // position on \n + assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); // position on w + assert_eq!(coords_at_pos(slice, 7), (1, 1).into()); // position on o + assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d + + // Test with wide characters. + let text = Rope::from("今日はいい\n"); + let slice = text.slice(..); + assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); + assert_eq!(coords_at_pos(slice, 1), (0, 2).into()); + assert_eq!(coords_at_pos(slice, 2), (0, 4).into()); + assert_eq!(coords_at_pos(slice, 3), (0, 6).into()); + assert_eq!(coords_at_pos(slice, 4), (0, 8).into()); + assert_eq!(coords_at_pos(slice, 5), (0, 10).into()); + assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); // test with grapheme clusters let text = Rope::from("a̐éö̲\r\n"); @@ -99,40 +148,64 @@ mod test { assert_eq!(coords_at_pos(slice, 2), (0, 1).into()); assert_eq!(coords_at_pos(slice, 4), (0, 2).into()); assert_eq!(coords_at_pos(slice, 7), (0, 3).into()); + assert_eq!(coords_at_pos(slice, 9), (1, 0).into()); - let text = Rope::from("किमपि"); + let text = Rope::from("किमपि\n"); let slice = text.slice(..); assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); - assert_eq!(coords_at_pos(slice, 2), (0, 1).into()); - assert_eq!(coords_at_pos(slice, 3), (0, 2).into()); - assert_eq!(coords_at_pos(slice, 5), (0, 3).into()); + assert_eq!(coords_at_pos(slice, 2), (0, 2).into()); + assert_eq!(coords_at_pos(slice, 3), (0, 3).into()); + assert_eq!(coords_at_pos(slice, 5), (0, 5).into()); + assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); } #[test] fn test_pos_at_coords() { let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ"); let slice = text.slice(..); - assert_eq!(pos_at_coords(slice, (0, 0).into()), 0); - assert_eq!(pos_at_coords(slice, (0, 5).into()), 5); // position on \n - assert_eq!(pos_at_coords(slice, (1, 0).into()), 6); // position on w - assert_eq!(pos_at_coords(slice, (1, 1).into()), 7); // position on o - assert_eq!(pos_at_coords(slice, (1, 4).into()), 10); // position on d + assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 5).into(), false), 5); // position on \n + assert_eq!(pos_at_coords(slice, (0, 6).into(), false), 6); // position after \n + assert_eq!(pos_at_coords(slice, (0, 6).into(), true), 5); // position after \n + assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 6); // position on w + assert_eq!(pos_at_coords(slice, (1, 1).into(), false), 7); // position on o + assert_eq!(pos_at_coords(slice, (1, 4).into(), false), 10); // position on d + + // Test with wide characters. + let text = Rope::from("今日はいい\n"); + let slice = text.slice(..); + assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 1); + assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 1); + assert_eq!(pos_at_coords(slice, (0, 4).into(), false), 2); + assert_eq!(pos_at_coords(slice, (0, 6).into(), false), 3); + assert_eq!(pos_at_coords(slice, (0, 8).into(), false), 4); + assert_eq!(pos_at_coords(slice, (0, 10).into(), false), 5); + assert_eq!(pos_at_coords(slice, (0, 11).into(), false), 6); + assert_eq!(pos_at_coords(slice, (0, 11).into(), true), 5); + assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 6); // test with grapheme clusters let text = Rope::from("a̐éö̲\r\n"); let slice = text.slice(..); - assert_eq!(pos_at_coords(slice, (0, 0).into()), 0); - assert_eq!(pos_at_coords(slice, (0, 1).into()), 2); - assert_eq!(pos_at_coords(slice, (0, 2).into()), 4); - assert_eq!(pos_at_coords(slice, (0, 3).into()), 7); // \r\n is one char here - assert_eq!(pos_at_coords(slice, (0, 4).into()), 9); + assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 2); + assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 4); + assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 7); // \r\n is one char here + assert_eq!(pos_at_coords(slice, (0, 4).into(), false), 9); + assert_eq!(pos_at_coords(slice, (0, 4).into(), true), 7); + assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 9); + let text = Rope::from("किमपि"); // 2 - 1 - 2 codepoints // TODO: delete handling as per https://news.ycombinator.com/item?id=20058454 let slice = text.slice(..); - assert_eq!(pos_at_coords(slice, (0, 0).into()), 0); - assert_eq!(pos_at_coords(slice, (0, 1).into()), 2); - assert_eq!(pos_at_coords(slice, (0, 2).into()), 3); - assert_eq!(pos_at_coords(slice, (0, 3).into()), 5); // eol + assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 2); + assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 3); + assert_eq!(pos_at_coords(slice, (0, 4).into(), false), 3); + assert_eq!(pos_at_coords(slice, (0, 5).into(), false), 5); // eol } } diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 074b6199..beade27a 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -286,7 +286,7 @@ impl Selection { // For 1-width cursor semantics. if range.anchor < range.head { - prev_grapheme_boundary(text, range.head) + prev_grapheme_boundary(text, range.head).max(range.anchor) } else { range.head } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 72462741..0ea78f6b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -917,7 +917,7 @@ fn scroll(cx: &mut Context, offset: usize, direction: Direction) { .min(last_line.saturating_sub(scrolloff)); let text = doc.text().slice(..); - let pos = pos_at_coords(text, Position::new(line, cursor.col)); // this func will properly truncate to line end + let pos = pos_at_coords(text, Position::new(line, cursor.col), true); // this func will properly truncate to line end // TODO: only manipulate main selection doc.set_selection(view.id, Selection::point(pos)); diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index 67585ed3..66214691 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -97,9 +97,9 @@ impl View { const OFFSET: usize = 7; // 1 diagnostic + 5 linenr + 1 gutter let last_col = self.first_col + (self.area.width as usize - OFFSET); - if line > last_line.saturating_sub(scrolloff) { + if line > last_line.saturating_sub(scrolloff + 1) { // scroll down - self.first_line += line - (last_line.saturating_sub(scrolloff)); + self.first_line += line - (last_line.saturating_sub(scrolloff + 1)); } else if line < self.first_line + scrolloff { // scroll up self.first_line = line.saturating_sub(scrolloff); -- cgit v1.2.3-70-g09d2 From 063aa9452d7c41e486a143b9f7e14fb5d14ad808 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Wed, 21 Jul 2021 09:32:48 -0700 Subject: Fix yank not working with internally zero-width ranges. --- helix-term/src/commands.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 0ea78f6b..475eccf2 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3000,9 +3000,13 @@ fn redo(cx: &mut Context) { fn yank(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); + let values: Vec = doc .selection(view.id) - .fragments(doc.text().slice(..)) + .clone() + .min_width_1(text) + .fragments(text) .map(Cow::into_owned) .collect(); @@ -3021,10 +3025,13 @@ fn yank(cx: &mut Context) { fn yank_joined_to_clipboard_impl(editor: &mut Editor, separator: &str) -> anyhow::Result<()> { let (view, doc) = current!(editor); + let text = doc.text().slice(..); let values: Vec = doc .selection(view.id) - .fragments(doc.text().slice(..)) + .clone() + .min_width_1(text) + .fragments(text) .map(Cow::into_owned) .collect(); @@ -3052,11 +3059,13 @@ fn yank_joined_to_clipboard(cx: &mut Context) { fn yank_main_selection_to_clipboard_impl(editor: &mut Editor) -> anyhow::Result<()> { let (view, doc) = current!(editor); + let text = doc.text().slice(..); let value = doc .selection(view.id) .primary() - .fragment(doc.text().slice(..)); + .min_width_1(text) + .fragment(text); if let Err(e) = editor.clipboard_provider.set_contents(value.into_owned()) { bail!("Couldn't set system clipboard content: {:?}", e); -- cgit v1.2.3-70-g09d2 From 7d07704e6ff59ed2eee664bb14e9c6a8c42ee4fb Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Wed, 21 Jul 2021 09:56:21 -0700 Subject: Fix append mode not editing correctly. This is currently a bit of a hack, and still doesn't behave quite how we probably want. Left a TODO. --- helix-term/src/commands.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 475eccf2..35a67e36 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1281,18 +1281,19 @@ fn append_mode(cx: &mut Context) { let (view, doc) = current!(cx.editor); enter_insert_mode(doc); doc.restore_cursor = true; + let text = doc.text().slice(..); - let selection = doc.selection(view.id).clone().transform(|range| { - let to = if range.to() == range.from() { - // For 1-width cursor semantics. - graphemes::next_grapheme_boundary(doc.text().slice(..), range.to()) - } else { - range.to() - }; - Range::new(range.from(), to) - }); + // TODO: preserve selections, like in `Insert` mode. Probably we'll want + // an explicit separate `Append` mode or something similar, so that we + // don't change the selection at all, and instead just display and edit + // things differently. + let selection = doc + .selection(view.id) + .clone() + .min_width_1(text) + .transform(|range| Range::new(range.to(), range.to())); - let end = doc.text().len_chars(); + let end = text.len_chars(); if selection.iter().any(|range| range.head == end) { let transaction = Transaction::change( -- cgit v1.2.3-70-g09d2 From 673338bdb6064ff98e8822a251fd31664909610d Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Thu, 22 Jul 2021 10:50:12 -0700 Subject: Use `Range::line_range()` in some more places I missed. --- helix-term/src/commands.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 35a67e36..a17245aa 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3238,8 +3238,7 @@ fn get_lines(doc: &Document, view_id: ViewId) -> Vec { // Get all line numbers for range in doc.selection(view_id) { - let start = doc.text().char_to_line(range.from()); - let end = doc.text().char_to_line(range.to()); + let (start, end) = range.line_range(doc.text().slice(..)); for line in start..=end { lines.push(line) @@ -3367,10 +3366,9 @@ fn join_selections(cx: &mut Context) { let fragment = Tendril::from(" "); for selection in doc.selection(view.id) { - let start = text.char_to_line(selection.from()); - let mut end = text.char_to_line(selection.to()); + let (start, mut end) = selection.line_range(slice); if start == end { - end += 1 + end = (end + 1).min(text.len_lines() - 1); } let lines = start..end; -- cgit v1.2.3-70-g09d2 From 5841954f58702ba2eabc17d96f0bf167cb13af24 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Thu, 22 Jul 2021 11:17:03 -0700 Subject: Calculate the line that the range head is on correctly. --- helix-core/src/selection.rs | 12 ++++++++++++ helix-term/src/commands.rs | 35 ++++++++++++----------------------- 2 files changed, 24 insertions(+), 23 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 3d4ee768..6ca52ebf 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -73,6 +73,18 @@ impl Range { std::cmp::max(self.anchor, self.head) } + /// The line number that the head is on (using 1-width semantics). + #[inline] + #[must_use] + pub fn head_line(&self, text: RopeSlice) -> usize { + let head = if self.anchor < self.head { + prev_grapheme_boundary(text, self.head) + } else { + self.head + }; + text.char_to_line(head) + } + /// The (inclusive) range of lines that the range overlaps. #[inline] #[must_use] diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index a17245aa..c6f592d9 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -396,19 +396,14 @@ fn goto_line_end(cx: &mut Context) { view.id, doc.selection(view.id).clone().transform(|range| { let text = doc.text().slice(..); - - let head = if range.anchor < range.head { - graphemes::prev_grapheme_boundary(text, range.head) - } else { - range.head - }; - let line = text.char_to_line(head); + let line = range.head_line(text); let mut pos = line_end_char_index(&text, line); if doc.mode != Mode::Select { pos = graphemes::prev_grapheme_boundary(text, pos); } - pos = head.max(pos).max(text.line_to_char(line)); + + pos = range.head.max(pos).max(text.line_to_char(line)); range.put(text, pos, doc.mode == Mode::Select) }), @@ -422,13 +417,7 @@ fn goto_line_end_newline(cx: &mut Context) { view.id, doc.selection(view.id).clone().transform(|range| { let text = doc.text().slice(..); - - let head = if range.anchor < range.head { - graphemes::prev_grapheme_boundary(text, range.head) - } else { - range.head - }; - let line = text.char_to_line(head); + let line = range.head_line(text); let mut pos = text.line_to_char((line + 1).min(text.len_lines())); if doc.mode != Mode::Select { @@ -445,7 +434,7 @@ fn goto_line_start(cx: &mut Context) { view.id, doc.selection(view.id).clone().transform(|range| { let text = doc.text().slice(..); - let line = text.char_to_line(range.head); + let line = range.head_line(text); // adjust to start of the line let pos = text.line_to_char(line); @@ -460,10 +449,10 @@ fn goto_first_nonwhitespace(cx: &mut Context) { view.id, doc.selection(view.id).clone().transform(|range| { let text = doc.text().slice(..); - let line_idx = text.char_to_line(range.head); + let line = range.head_line(text); - if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) { - let pos = pos + text.line_to_char(line_idx); + if let Some(pos) = find_first_non_whitespace_char(text.line(line)) { + let pos = pos + text.line_to_char(line); range.put(text, pos, doc.mode == Mode::Select) } else { range @@ -2212,9 +2201,9 @@ fn append_to_line(cx: &mut Context) { doc.set_selection( view.id, doc.selection(view.id).clone().transform(|range| { - let text = &doc.text().slice(..); - let line = text.char_to_line(range.head); - let pos = line_end_char_index(text, line); + let text = doc.text().slice(..); + let line = range.head_line(text); + let pos = line_end_char_index(&text, line); Range::new(pos, pos) }), ); @@ -2274,7 +2263,7 @@ fn open(cx: &mut Context, open: Open) { let mut offs = 0; let mut transaction = Transaction::change_by_selection(contents, selection, |range| { - let line = text.char_to_line(range.head); + let line = range.head_line(text); let line = match open { // adjust position to the end of the line (next line - 1) -- cgit v1.2.3-70-g09d2 From ad814b8c2eba229e99524f412077315dd1d24127 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Fri, 23 Jul 2021 14:27:12 -0700 Subject: Fix append mode, and make insertion always happen at head of range. --- helix-core/src/selection.rs | 14 ++++++++++++++ helix-term/src/commands.rs | 40 +++++++++++++++++++++++----------------- 2 files changed, 37 insertions(+), 17 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 6ca52ebf..e40fceb1 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -447,6 +447,20 @@ impl Selection { self.transform(|r| r.min_width_1(text)) } + /// Transforms the selection into all of the left-side head positions, + /// using 1-width semantics. + pub fn cursors(self, text: RopeSlice) -> Self { + self.transform(|range| { + // For 1-width cursor semantics. + let pos = if range.anchor < range.head { + prev_grapheme_boundary(text, range.head).max(range.anchor) + } else { + range.head + }; + Range::new(pos, pos) + }) + } + pub fn fragments<'a>(&'a self, text: RopeSlice<'a>) -> impl Iterator> + 'a { self.ranges.iter().map(move |range| range.fragment(text)) } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index c6f592d9..5ac83d7b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1272,19 +1272,13 @@ fn append_mode(cx: &mut Context) { doc.restore_cursor = true; let text = doc.text().slice(..); - // TODO: preserve selections, like in `Insert` mode. Probably we'll want - // an explicit separate `Append` mode or something similar, so that we - // don't change the selection at all, and instead just display and edit - // things differently. - let selection = doc - .selection(view.id) - .clone() - .min_width_1(text) - .transform(|range| Range::new(range.to(), range.to())); + let selection = doc.selection(view.id).clone().min_width_1(text); + // Make sure there's room at the end of the document if the last + // selection butts up against it. let end = text.len_chars(); - - if selection.iter().any(|range| range.head == end) { + let last_range = selection.iter().last().unwrap(); + if !last_range.is_empty() && last_range.head == end { let transaction = Transaction::change( doc.text(), std::array::IntoIter::new([(end, end, Some(doc.line_ending.as_str().into()))]), @@ -1292,7 +1286,15 @@ fn append_mode(cx: &mut Context) { doc.apply(&transaction, view.id); } - doc.set_selection(view.id, selection); + doc.set_selection( + view.id, + selection.clone().transform(|range| { + Range::new( + range.from(), + graphemes::next_grapheme_boundary(doc.text().slice(..), range.to()), + ) + }), + ); } mod cmd { @@ -2829,11 +2831,11 @@ pub mod insert { let (view, doc) = current!(cx.editor); let text = doc.text(); - let selection = doc.selection(view.id); + let selection = doc.selection(view.id).clone().cursors(text.slice(..)); // run through insert hooks, stopping on the first one that returns Some(t) for hook in HOOKS { - if let Some(transaction) = hook(text, selection, c) { + if let Some(transaction) = hook(text, &selection, c) { doc.apply(&transaction, view.id); break; } @@ -2853,7 +2855,11 @@ pub mod insert { // indent by one to reach 4 spaces). let indent = Tendril::from(doc.indent_unit()); - let transaction = Transaction::insert(doc.text(), doc.selection(view.id), indent); + let transaction = Transaction::insert( + doc.text(), + &doc.selection(view.id).clone().cursors(doc.text().slice(..)), + indent, + ); doc.apply(&transaction, view.id); } @@ -2862,13 +2868,13 @@ pub mod insert { let text = doc.text().slice(..); let contents = doc.text(); - let selection = doc.selection(view.id); + let selection = doc.selection(view.id).clone().cursors(text); let mut ranges = SmallVec::with_capacity(selection.len()); // TODO: this is annoying, but we need to do it to properly calculate pos after edits let mut offs = 0; - let mut transaction = Transaction::change_by_selection(contents, selection, |range| { + let mut transaction = Transaction::change_by_selection(contents, &selection, |range| { let pos = range.head; let prev = if pos == 0 { -- cgit v1.2.3-70-g09d2 From 427ae6ac6cfdd2a89580692dadd45f1a8dc02d2c Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Fri, 23 Jul 2021 17:06:14 -0700 Subject: Put selection in separate variable in commands code. --- helix-term/src/commands.rs | 571 ++++++++++++++++++++------------------------- 1 file changed, 252 insertions(+), 319 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 5ac83d7b..8bde88a2 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -325,140 +325,111 @@ impl PartialEq for Command { fn move_char_left(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_horizontally( - doc.text().slice(..), - range, - Direction::Backward, - count, - Movement::Move, - ) - }), - ); + let text = doc.text().slice(..); + + let selection = doc.selection(view.id).clone().transform(|range| { + movement::move_horizontally(text, range, Direction::Backward, count, Movement::Move) + }); + doc.set_selection(view.id, selection); } fn move_char_right(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_horizontally( - doc.text().slice(..), - range, - Direction::Forward, - count, - Movement::Move, - ) - }), - ); + let text = doc.text().slice(..); + + let selection = doc.selection(view.id).clone().transform(|range| { + movement::move_horizontally(text, range, Direction::Forward, count, Movement::Move) + }); + doc.set_selection(view.id, selection); } fn move_line_up(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_vertically( - doc.text().slice(..), - range, - Direction::Backward, - count, - Movement::Move, - ) - }), - ); + let text = doc.text().slice(..); + + let selection = doc.selection(view.id).clone().transform(|range| { + movement::move_vertically(text, range, Direction::Backward, count, Movement::Move) + }); + doc.set_selection(view.id, selection); } fn move_line_down(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_vertically( - doc.text().slice(..), - range, - Direction::Forward, - count, - Movement::Move, - ) - }), - ); + let text = doc.text().slice(..); + + let selection = doc.selection(view.id).clone().transform(|range| { + movement::move_vertically(text, range, Direction::Forward, count, Movement::Move) + }); + doc.set_selection(view.id, selection); } fn goto_line_end(cx: &mut Context) { let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - let text = doc.text().slice(..); - let line = range.head_line(text); + let text = doc.text().slice(..); - let mut pos = line_end_char_index(&text, line); - if doc.mode != Mode::Select { - pos = graphemes::prev_grapheme_boundary(text, pos); - } + let selection = doc.selection(view.id).clone().transform(|range| { + let line = range.head_line(text); - pos = range.head.max(pos).max(text.line_to_char(line)); + let mut pos = line_end_char_index(&text, line); + if doc.mode != Mode::Select { + pos = graphemes::prev_grapheme_boundary(text, pos); + } - range.put(text, pos, doc.mode == Mode::Select) - }), - ); + pos = range.head.max(pos).max(text.line_to_char(line)); + + range.put(text, pos, doc.mode == Mode::Select) + }); + doc.set_selection(view.id, selection); } fn goto_line_end_newline(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - let text = doc.text().slice(..); - let line = range.head_line(text); + let selection = doc.selection(view.id).clone().transform(|range| { + let line = range.head_line(text); - let mut pos = text.line_to_char((line + 1).min(text.len_lines())); - if doc.mode != Mode::Select { - pos = graphemes::prev_grapheme_boundary(text, pos); - } - range.put(text, pos, doc.mode == Mode::Select) - }), - ); + let mut pos = text.line_to_char((line + 1).min(text.len_lines())); + if doc.mode != Mode::Select { + pos = graphemes::prev_grapheme_boundary(text, pos); + } + range.put(text, pos, doc.mode == Mode::Select) + }); + doc.set_selection(view.id, selection); } fn goto_line_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - let text = doc.text().slice(..); - let line = range.head_line(text); + let text = doc.text().slice(..); - // adjust to start of the line - let pos = text.line_to_char(line); - range.put(text, pos, doc.mode == Mode::Select) - }), - ); + let selection = doc.selection(view.id).clone().transform(|range| { + let line = range.head_line(text); + + // adjust to start of the line + let pos = text.line_to_char(line); + range.put(text, pos, doc.mode == Mode::Select) + }); + doc.set_selection(view.id, selection); } fn goto_first_nonwhitespace(cx: &mut Context) { let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - let text = doc.text().slice(..); - let line = range.head_line(text); + let text = doc.text().slice(..); - if let Some(pos) = find_first_non_whitespace_char(text.line(line)) { - let pos = pos + text.line_to_char(line); - range.put(text, pos, doc.mode == Mode::Select) - } else { - range - } - }), - ); + let selection = doc.selection(view.id).clone().transform(|range| { + let line = range.head_line(text); + + if let Some(pos) = find_first_non_whitespace_char(text.line(line)) { + let pos = pos + text.line_to_char(line); + range.put(text, pos, doc.mode == Mode::Select) + } else { + range + } + }); + doc.set_selection(view.id, selection); } fn goto_window(cx: &mut Context, align: Align) { @@ -499,80 +470,79 @@ fn goto_window_bottom(cx: &mut Context) { fn move_next_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); - doc.set_selection( - view.id, - doc.selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)) - .transform(|range| movement::move_next_word_start(doc.text().slice(..), range, count)), - ); + let selection = doc + .selection(view.id) + .clone() + .min_width_1(text) + .transform(|range| movement::move_next_word_start(text, range, count)); + doc.set_selection(view.id, selection); } fn move_prev_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)) - .transform(|range| movement::move_prev_word_start(doc.text().slice(..), range, count)), - ); + let text = doc.text().slice(..); + + let selection = doc + .selection(view.id) + .clone() + .min_width_1(text) + .transform(|range| movement::move_prev_word_start(text, range, count)); + doc.set_selection(view.id, selection); } fn move_next_word_end(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)) - .transform(|range| movement::move_next_word_end(doc.text().slice(..), range, count)), - ); + let text = doc.text().slice(..); + + let selection = doc + .selection(view.id) + .clone() + .min_width_1(text) + .transform(|range| movement::move_next_word_end(text, range, count)); + doc.set_selection(view.id, selection); } fn move_next_long_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)) - .transform(|range| { - movement::move_next_long_word_start(doc.text().slice(..), range, count) - }), - ); + let text = doc.text().slice(..); + + let selection = doc + .selection(view.id) + .clone() + .min_width_1(text) + .transform(|range| movement::move_next_long_word_start(text, range, count)); + doc.set_selection(view.id, selection); } fn move_prev_long_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)) - .transform(|range| { - movement::move_prev_long_word_start(doc.text().slice(..), range, count) - }), - ); + let text = doc.text().slice(..); + + let selection = doc + .selection(view.id) + .clone() + .min_width_1(text) + .transform(|range| movement::move_prev_long_word_start(text, range, count)); + doc.set_selection(view.id, selection); } fn move_next_long_word_end(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)) - .transform(|range| { - movement::move_next_long_word_end(doc.text().slice(..), range, count) - }), - ); + let text = doc.text().slice(..); + + let selection = doc + .selection(view.id) + .clone() + .min_width_1(text) + .transform(|range| movement::move_next_long_word_end(text, range, count)); + doc.set_selection(view.id, selection); } fn goto_file_start(cx: &mut Context) { @@ -590,52 +560,52 @@ fn goto_file_end(cx: &mut Context) { fn extend_next_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)) - .transform(|range| { - let text = doc.text().slice(..); - let word = movement::move_next_word_start(text, range, count); - let pos = word.head; - range.put(text, pos, true) - }), - ); + let text = doc.text().slice(..); + + let selection = doc + .selection(view.id) + .clone() + .min_width_1(text) + .transform(|range| { + let word = movement::move_next_word_start(text, range, count); + let pos = word.head; + range.put(text, pos, true) + }); + doc.set_selection(view.id, selection); } fn extend_prev_word_start(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)) - .transform(|range| { - let text = doc.text().slice(..); - let word = movement::move_prev_word_start(text, range, count); - let pos = word.head; - range.put(text, pos, true) - }), - ); + let text = doc.text().slice(..); + + let selection = doc + .selection(view.id) + .clone() + .min_width_1(text) + .transform(|range| { + let word = movement::move_prev_word_start(text, range, count); + let pos = word.head; + range.put(text, pos, true) + }); + doc.set_selection(view.id, selection); } fn extend_next_word_end(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)) - .transform(|range| { - let text = doc.text().slice(..); - let word = movement::move_next_word_end(text, range, count); - let pos = word.head; - range.put(text, pos, true) - }), - ); + let text = doc.text().slice(..); + + let selection = doc + .selection(view.id) + .clone() + .min_width_1(text) + .transform(|range| { + let word = movement::move_next_word_end(text, range, count); + let pos = word.head; + range.put(text, pos, true) + }); + doc.set_selection(view.id, selection); } #[inline] @@ -678,15 +648,13 @@ where }; let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - let text = doc.text().slice(..); - search_fn(text, ch, range.head, count, inclusive) - .map_or(range, |pos| range.put(text, pos, extend)) - }), - ); + let selection = doc.selection(view.id).clone().transform(|range| { + search_fn(text, ch, range.head, count, inclusive) + .map_or(range, |pos| range.put(text, pos, extend)) + }); + doc.set_selection(view.id, selection); }) } @@ -939,69 +907,45 @@ fn half_page_down(cx: &mut Context) { fn extend_char_left(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_horizontally( - doc.text().slice(..), - range, - Direction::Backward, - count, - Movement::Extend, - ) - }), - ); + let text = doc.text().slice(..); + + let selection = doc.selection(view.id).clone().transform(|range| { + movement::move_horizontally(text, range, Direction::Backward, count, Movement::Extend) + }); + doc.set_selection(view.id, selection); } fn extend_char_right(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_horizontally( - doc.text().slice(..), - range, - Direction::Forward, - count, - Movement::Extend, - ) - }), - ); + let text = doc.text().slice(..); + + let selection = doc.selection(view.id).clone().transform(|range| { + movement::move_horizontally(text, range, Direction::Forward, count, Movement::Extend) + }); + doc.set_selection(view.id, selection); } fn extend_line_up(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_vertically( - doc.text().slice(..), - range, - Direction::Backward, - count, - Movement::Extend, - ) - }), - ); + let text = doc.text().slice(..); + + let selection = doc.selection(view.id).clone().transform(|range| { + movement::move_vertically(text, range, Direction::Backward, count, Movement::Extend) + }); + doc.set_selection(view.id, selection); } fn extend_line_down(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_vertically( - doc.text().slice(..), - range, - Direction::Forward, - count, - Movement::Extend, - ) - }), - ); + let text = doc.text().slice(..); + + let selection = doc.selection(view.id).clone().transform(|range| { + movement::move_vertically(text, range, Direction::Forward, count, Movement::Extend) + }); + doc.set_selection(view.id, selection); } fn select_all(cx: &mut Context) { @@ -1222,30 +1166,28 @@ fn change_selection(cx: &mut Context) { fn collapse_selection(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - let pos = if range.head > range.anchor { - // For 1-width cursor semantics. - graphemes::prev_grapheme_boundary(doc.text().slice(..), range.head) - } else { - range.head - }; - Range::new(pos, pos) - }), - ); + let selection = doc.selection(view.id).clone().transform(|range| { + let pos = if range.head > range.anchor { + // For 1-width cursor semantics. + graphemes::prev_grapheme_boundary(text, range.head) + } else { + range.head + }; + Range::new(pos, pos) + }); + doc.set_selection(view.id, selection); } fn flip_selections(cx: &mut Context) { let (view, doc) = current!(cx.editor); - doc.set_selection( - view.id, - doc.selection(view.id) - .clone() - .transform(|range| Range::new(range.head, range.anchor)), - ); + let selection = doc + .selection(view.id) + .clone() + .transform(|range| Range::new(range.head, range.anchor)); + doc.set_selection(view.id, selection); } fn enter_insert_mode(doc: &mut Document) { @@ -1257,12 +1199,11 @@ fn insert_mode(cx: &mut Context) { let (view, doc) = current!(cx.editor); enter_insert_mode(doc); - doc.set_selection( - view.id, - doc.selection(view.id) - .clone() - .transform(|range| Range::new(range.to(), range.from())), - ); + let selection = doc + .selection(view.id) + .clone() + .transform(|range| Range::new(range.to(), range.from())); + doc.set_selection(view.id, selection); } // inserts at the end of each selection @@ -1286,15 +1227,13 @@ fn append_mode(cx: &mut Context) { doc.apply(&transaction, view.id); } - doc.set_selection( - view.id, - selection.clone().transform(|range| { - Range::new( - range.from(), - graphemes::next_grapheme_boundary(doc.text().slice(..), range.to()), - ) - }), - ); + let selection = selection.transform(|range| { + Range::new( + range.from(), + graphemes::next_grapheme_boundary(doc.text().slice(..), range.to()), + ) + }); + doc.set_selection(view.id, selection); } mod cmd { @@ -2200,15 +2139,13 @@ fn append_to_line(cx: &mut Context) { let (view, doc) = current!(cx.editor); enter_insert_mode(doc); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - let text = doc.text().slice(..); - let line = range.head_line(text); - let pos = line_end_char_index(&text, line); - Range::new(pos, pos) - }), - ); + let selection = doc.selection(view.id).clone().transform(|range| { + let text = doc.text().slice(..); + let line = range.head_line(text); + let pos = line_end_char_index(&text, line); + Range::new(pos, pos) + }); + doc.set_selection(view.id, selection); } /// Sometimes when applying formatting changes we want to mark the buffer as unmodified, for @@ -2338,15 +2275,14 @@ fn normal_mode(cx: &mut Context) { // if leaving append mode, move cursor back by 1 if doc.restore_cursor { - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - Range::new( - range.from(), - graphemes::prev_grapheme_boundary(doc.text().slice(..), range.to()), - ) - }), - ); + let text = doc.text().slice(..); + let selection = doc.selection(view.id).clone().transform(|range| { + Range::new( + range.from(), + graphemes::prev_grapheme_boundary(text, range.to()), + ) + }); + doc.set_selection(view.id, selection); doc.restore_cursor = false; } @@ -2370,22 +2306,21 @@ fn goto_last_accessed_file(cx: &mut Context) { fn select_mode(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); // Make sure all selections are at least 1-wide. // (With the exception of being in an empty document, of course.) - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - if range.is_empty() && range.head == doc.text().len_chars() { - Range::new( - graphemes::prev_grapheme_boundary(doc.text().slice(..), range.anchor), - range.head, - ) - } else { - range.min_width_1(doc.text().slice(..)) - } - }), - ); + let selection = doc.selection(view.id).clone().transform(|range| { + if range.is_empty() && range.head == text.len_chars() { + Range::new( + graphemes::prev_grapheme_boundary(text, range.anchor), + range.head, + ) + } else { + range.min_width_1(text) + } + }); + doc.set_selection(view.id, selection); doc_mut!(cx.editor).mode = Mode::Select; } @@ -2964,13 +2899,13 @@ pub mod insert { pub fn delete_word_backward(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - movement::move_prev_word_start(doc.text().slice(..), range, count) - }), - ); + let selection = doc + .selection(view.id) + .clone() + .transform(|range| movement::move_prev_word_start(text, range, count)); + doc.set_selection(view.id, selection); delete_selection(cx) } } @@ -3721,21 +3656,19 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { } = event { let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); - doc.set_selection( - view.id, - doc.selection(view.id).clone().transform(|range| { - let text = doc.text().slice(..); - match ch { - 'w' => textobject::textobject_word(text, range, objtype, count), - // TODO: cancel new ranges if inconsistent surround matches across lines - ch if !ch.is_ascii_alphanumeric() => { - textobject::textobject_surround(text, range, objtype, ch, count) - } - _ => range, + let selection = doc.selection(view.id).clone().transform(|range| { + match ch { + 'w' => textobject::textobject_word(text, range, objtype, count), + // TODO: cancel new ranges if inconsistent surround matches across lines + ch if !ch.is_ascii_alphanumeric() => { + textobject::textobject_surround(text, range, objtype, ch, count) } - }), - ); + _ => range, + } + }); + doc.set_selection(view.id, selection); } }) } -- cgit v1.2.3-70-g09d2 From 8f43dc40398ff8372b1612241081926bac6d2055 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Fri, 23 Jul 2021 17:52:45 -0700 Subject: Fix surround replace command replacing the wrong position on the right. --- helix-term/src/commands.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 57df47a7..ca977f80 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3738,8 +3738,11 @@ fn surround_replace(cx: &mut Context) { let transaction = Transaction::change( doc.text(), change_pos.iter().enumerate().map(|(i, &pos)| { - let ch = if i % 2 == 0 { open } else { close }; - (pos, pos + 1, Some(Tendril::from_char(ch))) + if i % 2 == 0 { + (pos, pos + 1, Some(Tendril::from_char(open))) + } else { + (pos.saturating_sub(1), pos, Some(Tendril::from_char(close))) + } }), ); doc.apply(&transaction, view.id); -- cgit v1.2.3-70-g09d2 From 20723495d3ee82047bc7584e9eca1424d1256f4c Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Fri, 23 Jul 2021 18:03:40 -0700 Subject: Fixed find_till_char and find_char commands. They worked correctly when extending, but not for normal cursor movement. --- helix-term/src/commands.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index ca977f80..93867ee1 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -652,8 +652,13 @@ where let text = doc.text().slice(..); let selection = doc.selection(view.id).clone().transform(|range| { - search_fn(text, ch, range.head, count, inclusive) - .map_or(range, |pos| range.put(text, pos, extend)) + search_fn(text, ch, range.head, count, inclusive).map_or(range, |pos| { + if extend { + range.put(text, pos, true) + } else { + range.put(text, pos.saturating_sub(1), false) + } + }) }); doc.set_selection(view.id, selection); }) -- cgit v1.2.3-70-g09d2 From f96b8b769b3c7457935b5c02db870af97036f7b6 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Sat, 24 Jul 2021 07:44:11 -0700 Subject: Switch to a cleaner range-head moving abstraction. Also fix a bunch of bugs related to it. --- helix-core/src/movement.rs | 35 ++++++++------- helix-core/src/search.rs | 26 ++---------- helix-core/src/selection.rs | 20 +++++---- helix-core/src/surround.rs | 13 +++--- helix-term/src/commands.rs | 101 ++++++++++++++++++++++++++++++++------------ 5 files changed, 111 insertions(+), 84 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index 7f3a5ef4..80d19501 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -42,20 +42,18 @@ pub fn move_horizontally( }; // Compute the new position. - let mut new_pos = if dir == Direction::Backward { + let new_pos = if dir == Direction::Backward { nth_prev_grapheme_boundary(slice, pos, count) } else { nth_next_grapheme_boundary(slice, pos, count) }; - // Shift forward one grapheme if needed, for the - // visual 1-width cursor. - if behaviour == Extend && new_pos >= range.anchor { - new_pos = next_grapheme_boundary(slice, new_pos); - }; - // Compute the final new range. - range.put(slice, new_pos, behaviour == Extend) + if behaviour == Extend { + range.move_head(slice, new_pos, true) + } else { + Range::point(new_pos) + } } pub fn move_vertically( @@ -78,14 +76,17 @@ pub fn move_vertically( let horiz = range.horiz.unwrap_or(col as u32); // Compute the new position. - let new_pos = { + let (new_pos, new_row) = { let new_row = if dir == Direction::Backward { row.saturating_sub(count) } else { (row + count).min(slice.len_lines().saturating_sub(1)) }; let new_col = col.max(horiz as usize); - pos_at_coords(slice, Position::new(new_row, new_col), true) + ( + pos_at_coords(slice, Position::new(new_row, new_col), true), + new_row, + ) }; // Compute the new range according to the type of movement. @@ -97,15 +98,13 @@ pub fn move_vertically( }, Movement::Extend => { - let new_head = if new_pos >= range.anchor { - next_grapheme_boundary(slice, new_pos) + if slice.line(new_row).len_chars() > 0 { + let mut new_range = range.move_head(slice, new_pos, true); + new_range.horiz = Some(horiz); + new_range } else { - new_pos - }; - - let mut new_range = range.put(slice, new_head, true); - new_range.horiz = Some(horiz); - new_range + range + } } } } diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs index d4eb11a9..243ac227 100644 --- a/helix-core/src/search.rs +++ b/helix-core/src/search.rs @@ -1,12 +1,6 @@ use crate::RopeSlice; -pub fn find_nth_next( - text: RopeSlice, - ch: char, - mut pos: usize, - n: usize, - inclusive: bool, -) -> Option { +pub fn find_nth_next(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option { if pos >= text.len_chars() || n == 0 { return None; } @@ -25,20 +19,10 @@ pub fn find_nth_next( } } - if !inclusive { - pos -= 1; - } - - Some(pos) + Some(pos - 1) } -pub fn find_nth_prev( - text: RopeSlice, - ch: char, - mut pos: usize, - n: usize, - inclusive: bool, -) -> Option { +pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option { if pos == 0 || n == 0 { return None; } @@ -57,9 +41,5 @@ pub fn find_nth_prev( } } - if !inclusive { - pos += 1; - } - Some(pos) } diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index e40fceb1..cef68212 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -248,18 +248,18 @@ impl Range { } } - /// Moves the `Range` to `char_idx`. If `extend == true`, then only the head - /// is moved to `char_idx`, and the anchor is adjusted only as needed to - /// preserve 1-width range semantics. + /// Moves the head of the `Range` to `char_idx`, adjusting the anchor + /// as needed to preserve 1-width range semantics. + /// + /// `block_cursor` specifies whether it should treat `char_idx` as a block + /// cursor position or as a range-end position. /// /// This method assumes that the range and `char_idx` are already properly /// grapheme-aligned. #[must_use] #[inline] - pub fn put(self, text: RopeSlice, char_idx: usize, extend: bool) -> Range { - let anchor = if !extend { - char_idx - } else if self.head >= self.anchor && char_idx < self.anchor { + pub fn move_head(self, text: RopeSlice, char_idx: usize, block_cursor: bool) -> Range { + let anchor = if self.head >= self.anchor && char_idx < self.anchor { next_grapheme_boundary(text, self.anchor) } else if self.head < self.anchor && char_idx >= self.anchor { prev_grapheme_boundary(text, self.anchor) @@ -267,7 +267,11 @@ impl Range { self.anchor }; - Range::new(anchor, char_idx) + if block_cursor && anchor <= char_idx { + Range::new(anchor, next_grapheme_boundary(text, char_idx)) + } else { + Range::new(anchor, char_idx) + } } // groupAt diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index af357c96..4d3ed5f5 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -49,19 +49,18 @@ pub fn find_nth_pairs_pos( if Some(open) == text.get_char(pos) { // Special case: cursor is directly on a matching char. match pos { - 0 => Some((pos, search::find_nth_next(text, close, pos + 1, n, true)?)), - _ if (pos + 1) == text.len_chars() => Some(( - search::find_nth_prev(text, open, pos, n, true)?, - text.len_chars(), - )), + 0 => Some((pos, search::find_nth_next(text, close, pos + 1, n)? + 1)), + _ if (pos + 1) == text.len_chars() => { + Some((search::find_nth_prev(text, open, pos, n)?, text.len_chars())) + } // We return no match because there's no way to know which // side of the char we should be searching on. _ => None, } } else { Some(( - search::find_nth_prev(text, open, pos, n, true)?, - search::find_nth_next(text, close, pos, n, true)?, + search::find_nth_prev(text, open, pos, n)?, + search::find_nth_next(text, close, pos, n)? + 1, )) } } else { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 93867ee1..173d0b7b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -373,15 +373,16 @@ fn goto_line_end(cx: &mut Context) { let selection = doc.selection(view.id).clone().transform(|range| { let line = range.head_line(text); + let line_start = text.line_to_char(line); - let mut pos = line_end_char_index(&text, line); - if doc.mode != Mode::Select { - pos = graphemes::prev_grapheme_boundary(text, pos); - } - - pos = range.head.max(pos).max(text.line_to_char(line)); + let pos = graphemes::prev_grapheme_boundary(text, line_end_char_index(&text, line)) + .max(line_start); - range.put(text, pos, doc.mode == Mode::Select) + if doc.mode == Mode::Select { + range.move_head(text, pos, true) + } else { + Range::point(pos) + } }); doc.set_selection(view.id, selection); } @@ -392,12 +393,13 @@ fn goto_line_end_newline(cx: &mut Context) { let selection = doc.selection(view.id).clone().transform(|range| { let line = range.head_line(text); + let pos = line_end_char_index(&text, line); - let mut pos = text.line_to_char((line + 1).min(text.len_lines())); - if doc.mode != Mode::Select { - pos = graphemes::prev_grapheme_boundary(text, pos); + if doc.mode == Mode::Select { + range.move_head(text, pos, true) + } else { + Range::point(pos) } - range.put(text, pos, doc.mode == Mode::Select) }); doc.set_selection(view.id, selection); } @@ -411,7 +413,11 @@ fn goto_line_start(cx: &mut Context) { // adjust to start of the line let pos = text.line_to_char(line); - range.put(text, pos, doc.mode == Mode::Select) + if doc.mode == Mode::Select { + range.move_head(text, pos, true) + } else { + Range::point(pos) + } }); doc.set_selection(view.id, selection); } @@ -425,7 +431,11 @@ fn goto_first_nonwhitespace(cx: &mut Context) { if let Some(pos) = find_first_non_whitespace_char(text.line(line)) { let pos = pos + text.line_to_char(line); - range.put(text, pos, doc.mode == Mode::Select) + if doc.mode == Mode::Select { + range.move_head(text, pos, true) + } else { + Range::point(pos) + } } else { range } @@ -569,8 +579,8 @@ fn extend_next_word_start(cx: &mut Context) { .min_width_1(text) .transform(|range| { let word = movement::move_next_word_start(text, range, count); - let pos = word.head; - range.put(text, pos, true) + let pos = graphemes::prev_grapheme_boundary(text, word.head); + range.move_head(text, pos, true) }); doc.set_selection(view.id, selection); } @@ -587,7 +597,7 @@ fn extend_prev_word_start(cx: &mut Context) { .transform(|range| { let word = movement::move_prev_word_start(text, range, count); let pos = word.head; - range.put(text, pos, true) + range.move_head(text, pos, true) }); doc.set_selection(view.id, selection); } @@ -603,8 +613,8 @@ fn extend_next_word_end(cx: &mut Context) { .min_width_1(text) .transform(|range| { let word = movement::move_next_word_end(text, range, count); - let pos = word.head; - range.put(text, pos, true) + let pos = graphemes::prev_grapheme_boundary(text, word.head); + range.move_head(text, pos, true) }); doc.set_selection(view.id, selection); } @@ -652,11 +662,17 @@ where let text = doc.text().slice(..); let selection = doc.selection(view.id).clone().transform(|range| { + let range = if range.anchor < range.head { + // For 1-width cursor semantics. + Range::new(range.anchor, range.head - 1) + } else { + range + }; search_fn(text, ch, range.head, count, inclusive).map_or(range, |pos| { if extend { - range.put(text, pos, true) + range.move_head(text, pos, true) } else { - range.put(text, pos.saturating_sub(1), false) + Range::point(pos) } }) }); @@ -664,10 +680,39 @@ where }) } +fn find_next_char_impl( + text: RopeSlice, + ch: char, + pos: usize, + n: usize, + inclusive: bool, +) -> Option { + let pos = (pos + 1).min(text.len_chars()); + if inclusive { + search::find_nth_next(text, ch, pos, n) + } else { + search::find_nth_next(text, ch, pos, n).map(|n| n.saturating_sub(1)) + } +} + +fn find_prev_char_impl( + text: RopeSlice, + ch: char, + pos: usize, + n: usize, + inclusive: bool, +) -> Option { + if inclusive { + search::find_nth_prev(text, ch, pos, n) + } else { + search::find_nth_prev(text, ch, pos, n).map(|n| (n + 1).min(text.len_chars())) + } +} + fn find_till_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_next, + find_next_char_impl, false, /* inclusive */ false, /* extend */ ) @@ -676,7 +721,7 @@ fn find_till_char(cx: &mut Context) { fn find_next_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_next, + find_next_char_impl, true, /* inclusive */ false, /* extend */ ) @@ -685,7 +730,7 @@ fn find_next_char(cx: &mut Context) { fn extend_till_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_next, + find_next_char_impl, false, /* inclusive */ true, /* extend */ ) @@ -694,7 +739,7 @@ fn extend_till_char(cx: &mut Context) { fn extend_next_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_next, + find_next_char_impl, true, /* inclusive */ true, /* extend */ ) @@ -703,7 +748,7 @@ fn extend_next_char(cx: &mut Context) { fn till_prev_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_prev, + find_prev_char_impl, false, /* inclusive */ false, /* extend */ ) @@ -712,7 +757,7 @@ fn till_prev_char(cx: &mut Context) { fn find_prev_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_prev, + find_prev_char_impl, true, /* inclusive */ false, /* extend */ ) @@ -721,7 +766,7 @@ fn find_prev_char(cx: &mut Context) { fn extend_till_prev_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_prev, + find_prev_char_impl, false, /* inclusive */ true, /* extend */ ) @@ -730,7 +775,7 @@ fn extend_till_prev_char(cx: &mut Context) { fn extend_prev_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_prev, + find_prev_char_impl, true, /* inclusive */ true, /* extend */ ) -- cgit v1.2.3-70-g09d2 From 0883b4fae03343978e61fc377775d7ba93f86b40 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Mon, 26 Jul 2021 08:40:30 -0700 Subject: Collect some common patterns into methods on `Range`. --- helix-core/src/movement.rs | 50 ++++------------ helix-core/src/selection.rs | 80 ++++++++++++-------------- helix-core/src/textobject.rs | 15 ++--- helix-term/src/commands.rs | 124 +++++++++++++++++++++------------------- helix-term/src/ui/completion.rs | 20 +++++-- helix-term/src/ui/editor.rs | 14 ++++- helix-view/src/editor.rs | 10 +++- helix-view/src/view.rs | 5 +- 8 files changed, 155 insertions(+), 163 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index 80d19501..4af82a1d 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -31,15 +31,7 @@ pub fn move_horizontally( count: usize, behaviour: Movement, ) -> Range { - use Movement::Extend; - - // Shift back one grapheme if needed, to account for - // the cursor being visually 1-width. - let pos = if range.head > range.anchor { - prev_grapheme_boundary(slice, range.head) - } else { - range.head - }; + let pos = range.cursor(slice); // Compute the new position. let new_pos = if dir == Direction::Backward { @@ -49,11 +41,7 @@ pub fn move_horizontally( }; // Compute the final new range. - if behaviour == Extend { - range.move_head(slice, new_pos, true) - } else { - Range::point(new_pos) - } + range.put_cursor(slice, new_pos, behaviour == Movement::Extend) } pub fn move_vertically( @@ -63,13 +51,7 @@ pub fn move_vertically( count: usize, behaviour: Movement, ) -> Range { - // Shift back one grapheme if needed, to account for - // the cursor being visually 1-width. - let pos = if range.head > range.anchor { - prev_grapheme_boundary(slice, range.head) - } else { - range.head - }; + let pos = range.cursor(slice); // Compute the current position's 2d coordinates. let Position { row, col } = coords_at_pos(slice, pos); @@ -89,24 +71,14 @@ pub fn move_vertically( ) }; - // Compute the new range according to the type of movement. - match behaviour { - Movement::Move => Range { - anchor: new_pos, - head: new_pos, - horiz: Some(horiz), - }, - - Movement::Extend => { - if slice.line(new_row).len_chars() > 0 { - let mut new_range = range.move_head(slice, new_pos, true); - new_range.horiz = Some(horiz); - new_range - } else { - range - } - } + // Special-case to avoid moving to the end of the last non-empty line. + if behaviour == Movement::Extend && slice.line(new_row).len_chars() == 0 { + return range; } + + let mut new_range = range.put_cursor(slice, new_pos, behaviour == Movement::Extend); + new_range.horiz = Some(horiz); + new_range } pub fn move_next_word_start(slice: RopeSlice, range: Range, count: usize) -> Range { @@ -153,7 +125,7 @@ fn word_move(slice: RopeSlice, range: Range, count: usize, target: WordMotionTar // Prepare the range appropriately based on the target movement // direction. This is addressing two things at once: // - // 1. 1-width range sementics. + // 1. Block-cursor semantics. // 2. The anchor position being irrelevant to the output result. #[allow(clippy::collapsible_else_if)] // Makes the structure clearer in this case. let start_range = if is_prev { diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index cef68212..247a69fe 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -73,16 +73,11 @@ impl Range { std::cmp::max(self.anchor, self.head) } - /// The line number that the head is on (using 1-width semantics). + /// The line number that the block-cursor is on. #[inline] #[must_use] - pub fn head_line(&self, text: RopeSlice) -> usize { - let head = if self.anchor < self.head { - prev_grapheme_boundary(text, self.head) - } else { - self.head - }; - text.char_to_line(head) + pub fn cursor_line(&self, text: RopeSlice) -> usize { + text.char_to_line(self.cursor(text)) } /// The (inclusive) range of lines that the range overlaps. @@ -248,29 +243,44 @@ impl Range { } } - /// Moves the head of the `Range` to `char_idx`, adjusting the anchor - /// as needed to preserve 1-width range semantics. + /// Gets the left-side position of the block cursor. + #[must_use] + #[inline] + pub fn cursor(self, text: RopeSlice) -> usize { + if self.head > self.anchor { + prev_grapheme_boundary(text, self.head) + } else { + self.head + } + } + + /// Puts the left side of the block cursor at `char_idx`, optionally extending. /// - /// `block_cursor` specifies whether it should treat `char_idx` as a block - /// cursor position or as a range-end position. + /// This follows "1-width" semantics, and therefore does a combination of anchor + /// and head moves to behave as if both the front and back of the range are 1-width + /// blocks /// /// This method assumes that the range and `char_idx` are already properly /// grapheme-aligned. #[must_use] #[inline] - pub fn move_head(self, text: RopeSlice, char_idx: usize, block_cursor: bool) -> Range { - let anchor = if self.head >= self.anchor && char_idx < self.anchor { - next_grapheme_boundary(text, self.anchor) - } else if self.head < self.anchor && char_idx >= self.anchor { - prev_grapheme_boundary(text, self.anchor) - } else { - self.anchor - }; + pub fn put_cursor(self, text: RopeSlice, char_idx: usize, extend: bool) -> Range { + if extend { + let anchor = if self.head >= self.anchor && char_idx < self.anchor { + next_grapheme_boundary(text, self.anchor) + } else if self.head < self.anchor && char_idx >= self.anchor { + prev_grapheme_boundary(text, self.anchor) + } else { + self.anchor + }; - if block_cursor && anchor <= char_idx { - Range::new(anchor, next_grapheme_boundary(text, char_idx)) + if anchor <= char_idx { + Range::new(anchor, next_grapheme_boundary(text, char_idx)) + } else { + Range::new(anchor, char_idx) + } } else { - Range::new(anchor, char_idx) + Range::point(char_idx) } } @@ -309,18 +319,6 @@ impl Selection { self.ranges[self.primary_index] } - #[must_use] - pub fn cursor(&self, text: RopeSlice) -> usize { - let range = self.primary(); - - // For 1-width cursor semantics. - if range.anchor < range.head { - prev_grapheme_boundary(text, range.head).max(range.anchor) - } else { - range.head - } - } - /// Ensure selection containing only the primary selection. pub fn into_single(self) -> Self { if self.ranges.len() == 1 { @@ -452,17 +450,9 @@ impl Selection { } /// Transforms the selection into all of the left-side head positions, - /// using 1-width semantics. + /// using block-cursor semantics. pub fn cursors(self, text: RopeSlice) -> Self { - self.transform(|range| { - // For 1-width cursor semantics. - let pos = if range.anchor < range.head { - prev_grapheme_boundary(text, range.head).max(range.anchor) - } else { - range.head - }; - Range::new(pos, pos) - }) + self.transform(|range| Range::point(range.cursor(text))) } pub fn fragments<'a>(&'a self, text: RopeSlice<'a>) -> impl Iterator> + 'a { diff --git a/helix-core/src/textobject.rs b/helix-core/src/textobject.rs index ae18d7cf..e011c912 100644 --- a/helix-core/src/textobject.rs +++ b/helix-core/src/textobject.rs @@ -59,17 +59,12 @@ pub fn textobject_word( textobject: TextObject, _count: usize, ) -> Range { - // For 1-width cursor semantics. - let head = if range.head > range.anchor { - prev_grapheme_boundary(slice, range.head) - } else { - range.head - }; + let pos = range.cursor(slice); - let word_start = find_word_boundary(slice, head, Direction::Backward); - let word_end = match slice.get_char(head).map(categorize_char) { - None | Some(CharCategory::Whitespace | CharCategory::Eol) => head, - _ => find_word_boundary(slice, head + 1, Direction::Forward), + let word_start = find_word_boundary(slice, pos, Direction::Backward); + let word_end = match slice.get_char(pos).map(categorize_char) { + None | Some(CharCategory::Whitespace | CharCategory::Eol) => pos, + _ => find_word_boundary(slice, pos + 1, Direction::Forward), }; // Special case. diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 173d0b7b..86ede415 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -121,7 +121,10 @@ enum Align { } fn align_view(doc: &Document, view: &mut View, align: Align) { - let pos = doc.selection(view.id).cursor(doc.text().slice(..)); + let pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let line = doc.text().char_to_line(pos); let relative = match align { @@ -372,17 +375,13 @@ fn goto_line_end(cx: &mut Context) { let text = doc.text().slice(..); let selection = doc.selection(view.id).clone().transform(|range| { - let line = range.head_line(text); + let line = range.cursor_line(text); let line_start = text.line_to_char(line); let pos = graphemes::prev_grapheme_boundary(text, line_end_char_index(&text, line)) .max(line_start); - if doc.mode == Mode::Select { - range.move_head(text, pos, true) - } else { - Range::point(pos) - } + range.put_cursor(text, pos, doc.mode == Mode::Select) }); doc.set_selection(view.id, selection); } @@ -392,14 +391,10 @@ fn goto_line_end_newline(cx: &mut Context) { let text = doc.text().slice(..); let selection = doc.selection(view.id).clone().transform(|range| { - let line = range.head_line(text); + let line = range.cursor_line(text); let pos = line_end_char_index(&text, line); - if doc.mode == Mode::Select { - range.move_head(text, pos, true) - } else { - Range::point(pos) - } + range.put_cursor(text, pos, doc.mode == Mode::Select) }); doc.set_selection(view.id, selection); } @@ -409,15 +404,11 @@ fn goto_line_start(cx: &mut Context) { let text = doc.text().slice(..); let selection = doc.selection(view.id).clone().transform(|range| { - let line = range.head_line(text); + let line = range.cursor_line(text); // adjust to start of the line let pos = text.line_to_char(line); - if doc.mode == Mode::Select { - range.move_head(text, pos, true) - } else { - Range::point(pos) - } + range.put_cursor(text, pos, doc.mode == Mode::Select) }); doc.set_selection(view.id, selection); } @@ -427,15 +418,11 @@ fn goto_first_nonwhitespace(cx: &mut Context) { let text = doc.text().slice(..); let selection = doc.selection(view.id).clone().transform(|range| { - let line = range.head_line(text); + let line = range.cursor_line(text); if let Some(pos) = find_first_non_whitespace_char(text.line(line)) { let pos = pos + text.line_to_char(line); - if doc.mode == Mode::Select { - range.move_head(text, pos, true) - } else { - Range::point(pos) - } + range.put_cursor(text, pos, doc.mode == Mode::Select) } else { range } @@ -579,8 +566,8 @@ fn extend_next_word_start(cx: &mut Context) { .min_width_1(text) .transform(|range| { let word = movement::move_next_word_start(text, range, count); - let pos = graphemes::prev_grapheme_boundary(text, word.head); - range.move_head(text, pos, true) + let pos = word.cursor(text); + range.put_cursor(text, pos, true) }); doc.set_selection(view.id, selection); } @@ -596,8 +583,8 @@ fn extend_prev_word_start(cx: &mut Context) { .min_width_1(text) .transform(|range| { let word = movement::move_prev_word_start(text, range, count); - let pos = word.head; - range.move_head(text, pos, true) + let pos = word.cursor(text); + range.put_cursor(text, pos, true) }); doc.set_selection(view.id, selection); } @@ -613,8 +600,8 @@ fn extend_next_word_end(cx: &mut Context) { .min_width_1(text) .transform(|range| { let word = movement::move_next_word_end(text, range, count); - let pos = graphemes::prev_grapheme_boundary(text, word.head); - range.move_head(text, pos, true) + let pos = word.cursor(text); + range.put_cursor(text, pos, true) }); doc.set_selection(view.id, selection); } @@ -663,18 +650,13 @@ where let selection = doc.selection(view.id).clone().transform(|range| { let range = if range.anchor < range.head { - // For 1-width cursor semantics. + // For block-cursor semantics. Range::new(range.anchor, range.head - 1) } else { range }; - search_fn(text, ch, range.head, count, inclusive).map_or(range, |pos| { - if extend { - range.move_head(text, pos, true) - } else { - Range::point(pos) - } - }) + search_fn(text, ch, range.head, count, inclusive) + .map_or(range, |pos| range.put_cursor(text, pos, extend)) }); doc.set_selection(view.id, selection); }) @@ -895,7 +877,9 @@ fn scroll(cx: &mut Context, offset: usize, direction: Direction) { let (view, doc) = current!(cx.editor); let cursor = coords_at_pos( doc.text().slice(..), - doc.selection(view.id).cursor(doc.text().slice(..)), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), ); let doc_last_line = doc.text().len_lines() - 1; @@ -1220,12 +1204,7 @@ fn collapse_selection(cx: &mut Context) { let text = doc.text().slice(..); let selection = doc.selection(view.id).clone().transform(|range| { - let pos = if range.head > range.anchor { - // For 1-width cursor semantics. - graphemes::prev_grapheme_boundary(text, range.head) - } else { - range.head - }; + let pos = range.cursor(text); Range::new(pos, pos) }); doc.set_selection(view.id, selection); @@ -2203,7 +2182,7 @@ fn append_to_line(cx: &mut Context) { let selection = doc.selection(view.id).clone().transform(|range| { let text = doc.text().slice(..); - let line = range.head_line(text); + let line = range.cursor_line(text); let pos = line_end_char_index(&text, line); Range::new(pos, pos) }); @@ -2264,7 +2243,7 @@ fn open(cx: &mut Context, open: Open) { let mut offs = 0; let mut transaction = Transaction::change_by_selection(contents, selection, |range| { - let line = range.head_line(text); + let line = range.cursor_line(text); let line = match open { // adjust position to the end of the line (next line - 1) @@ -2472,7 +2451,9 @@ fn goto_definition(cx: &mut Context) { let pos = pos_to_lsp_pos( doc.text(), - doc.selection(view.id).cursor(doc.text().slice(..)), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), offset_encoding, ); @@ -2513,7 +2494,9 @@ fn goto_type_definition(cx: &mut Context) { let pos = pos_to_lsp_pos( doc.text(), - doc.selection(view.id).cursor(doc.text().slice(..)), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), offset_encoding, ); @@ -2554,7 +2537,9 @@ fn goto_implementation(cx: &mut Context) { let pos = pos_to_lsp_pos( doc.text(), - doc.selection(view.id).cursor(doc.text().slice(..)), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), offset_encoding, ); @@ -2595,7 +2580,9 @@ fn goto_reference(cx: &mut Context) { let pos = pos_to_lsp_pos( doc.text(), - doc.selection(view.id).cursor(doc.text().slice(..)), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), offset_encoding, ); @@ -2656,7 +2643,10 @@ fn goto_next_diag(cx: &mut Context) { let editor = &mut cx.editor; let (view, doc) = current!(editor); - let cursor_pos = doc.selection(view.id).cursor(doc.text().slice(..)); + let cursor_pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let diag = if let Some(diag) = doc .diagnostics() .iter() @@ -2677,7 +2667,10 @@ fn goto_prev_diag(cx: &mut Context) { let editor = &mut cx.editor; let (view, doc) = current!(editor); - let cursor_pos = doc.selection(view.id).cursor(doc.text().slice(..)); + let cursor_pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let diag = if let Some(diag) = doc .diagnostics() .iter() @@ -2705,7 +2698,9 @@ fn signature_help(cx: &mut Context) { let pos = pos_to_lsp_pos( doc.text(), - doc.selection(view.id).cursor(doc.text().slice(..)), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), language_server.offset_encoding(), ); @@ -3457,7 +3452,10 @@ fn completion(cx: &mut Context) { }; let offset_encoding = language_server.offset_encoding(); - let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let pos = pos_to_lsp_pos(doc.text(), cursor, offset_encoding); @@ -3514,7 +3512,9 @@ fn hover(cx: &mut Context) { let pos = pos_to_lsp_pos( doc.text(), - doc.selection(view.id).cursor(doc.text().slice(..)), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), language_server.offset_encoding(), ); @@ -3580,7 +3580,10 @@ fn match_brackets(cx: &mut Context) { let (view, doc) = current!(cx.editor); if let Some(syntax) = doc.syntax() { - let pos = doc.selection(view.id).cursor(doc.text().slice(..)); + let pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); if let Some(pos) = match_brackets::find(syntax, doc.text(), pos) { let selection = Selection::point(pos); doc.set_selection(view.id, selection); @@ -3684,7 +3687,10 @@ fn align_view_bottom(cx: &mut Context) { fn align_view_middle(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let pos = doc.selection(view.id).cursor(doc.text().slice(..)); + let pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let pos = coords_at_pos(doc.text().slice(..), pos); const OFFSET: usize = 7; // gutters diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index 942a2483..2725d53d 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -86,7 +86,10 @@ impl Completion { let item = item.unwrap(); // if more text was entered, remove it - let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); if trigger_offset < cursor { let remove = Transaction::change( doc.text(), @@ -109,7 +112,10 @@ impl Completion { ) } else { let text = item.insert_text.as_ref().unwrap_or(&item.label); - let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); Transaction::change( doc.text(), vec![(cursor, cursor, Some(text.as_str().into()))].into_iter(), @@ -155,7 +161,10 @@ impl Completion { // TODO: hooks should get processed immediately so maybe do it after select!(), before // looping? - let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); if self.trigger_offset <= cursor { let fragment = doc.text().slice(self.trigger_offset..cursor); let text = Cow::from(fragment); @@ -212,7 +221,10 @@ impl Component for Completion { .language() .and_then(|scope| scope.strip_prefix("source.")) .unwrap_or(""); - let cursor_pos = doc.selection(view.id).cursor(doc.text().slice(..)); + let cursor_pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let cursor_pos = (helix_core::coords_at_pos(doc.text().slice(..), cursor_pos).row - view.first_line) as u16; diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 8e29be6c..482a4117 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -406,7 +406,10 @@ impl EditorView { // TODO: set cursor position for IME if let Some(syntax) = doc.syntax() { use helix_core::match_brackets; - let pos = doc.selection(view.id).cursor(doc.text().slice(..)); + let pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let pos = match_brackets::find(syntax, doc.text(), pos) .and_then(|pos| view.screen_coords_at_pos(doc, text, pos)); @@ -451,7 +454,10 @@ impl EditorView { widgets::{Paragraph, Widget}, }; - let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let diagnostics = doc.diagnostics().iter().filter(|diagnostic| { diagnostic.range.start <= cursor && diagnostic.range.end >= cursor @@ -565,7 +571,9 @@ impl EditorView { let position_info = { let pos = coords_at_pos( doc.text().slice(..), - doc.selection(view.id).cursor(doc.text().slice(..)), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), ); format!("{}:{}", pos.row + 1, pos.col + 1) // convert to 1-indexing }; diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 1cd0af02..d5a76c11 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -145,7 +145,10 @@ impl Editor { .entry(view.id) .or_insert_with(|| Selection::point(0)); // TODO: reuse align_view - let pos = doc.selection(view.id).cursor(doc.text().slice(..)); + let pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let line = doc.text().char_to_line(pos); view.first_line = line.saturating_sub(view.area.height as usize / 2); @@ -295,7 +298,10 @@ impl Editor { const OFFSET: u16 = 7; // 1 diagnostic + 5 linenr + 1 gutter let view = view!(self); let doc = &self.documents[view.doc]; - let cursor = doc.selection(view.id).cursor(doc.text().slice(..)); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); if let Some(mut pos) = view.screen_coords_at_pos(doc, doc.text().slice(..), cursor) { pos.col += view.area.x as usize + OFFSET as usize; pos.row += view.area.y as usize; diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index 66214691..e90d0eab 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -84,7 +84,10 @@ impl View { } pub fn ensure_cursor_in_view(&mut self, doc: &Document) { - let cursor = doc.selection(self.id).cursor(doc.text().slice(..)); + let cursor = doc + .selection(self.id) + .primary() + .cursor(doc.text().slice(..)); let pos = coords_at_pos(doc.text().slice(..), cursor); let line = pos.row; let col = pos.col; -- cgit v1.2.3-70-g09d2 From 01247acf0cd06fcb3ba3b033e215b9b13b632816 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Mon, 26 Jul 2021 08:50:26 -0700 Subject: Start searches at the right side of the block cursor. --- helix-term/src/commands.rs | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 86ede415..89042acb 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1024,25 +1024,15 @@ fn split_selection_on_newline(cx: &mut Context) { } fn search_impl(doc: &mut Document, view: &mut View, contents: &str, regex: &Regex, extend: bool) { - let text = doc.text(); + let text = doc.text().slice(..); let selection = doc.selection(view.id); let start = { + // Get the right side of the block cursor. let range = selection.primary(); - - // This is a little bit weird. Due to 1-width cursor semantics, we - // would typically want the search to always begin at the visual left-side - // of the head. However, when there's already a selection from e.g. a - // previous search result, we don't want to include any of that selection - // in the subsequent search. The code below makes a compromise between the - // two behaviors that hopefully behaves the way most people expect most of - // the time. - if range.anchor <= range.head { - text.char_to_byte(range.head) + if range.anchor < range.head { + range.head } else { - text.char_to_byte(graphemes::next_grapheme_boundary( - text.slice(..), - range.head, - )) + graphemes::next_grapheme_boundary(text, range.head) } }; -- cgit v1.2.3-70-g09d2 From cd7302ffd35c4972e7b0414a31a08f4e3c672df5 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Wed, 28 Jul 2021 15:57:00 -0700 Subject: Enforce cursor/selection invariants in one place. Rather than per-command like before. --- helix-core/src/selection.rs | 13 ++-- helix-term/src/commands.rs | 169 ++++++++++++++++---------------------------- helix-view/src/document.rs | 10 ++- 3 files changed, 77 insertions(+), 115 deletions(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 9016462c..14c54295 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -455,13 +455,18 @@ impl Selection { for range in self.ranges.iter_mut() { *range = f(*range) } - self.normalize() } - /// A convenience short-cut for `transform(|r| r.min_width_1(text))`. - pub fn min_width_1(self, text: RopeSlice) -> Self { - self.transform(|r| r.min_width_1(text)) + // Ensures the selection adheres to the following invariants: + // 1. All ranges are grapheme aligned. + // 2. All ranges are at least 1 character wide, unless at the + // very end of the document. + // 3. Ranges are non-overlapping. + // 4. Ranges are sorted by their position in the text. + pub fn ensure_invariants(self, text: RopeSlice) -> Self { + self.transform(|r| r.min_width_1(text).grapheme_aligned(text)) + .normalize() } /// Transforms the selection into all of the left-side head positions, diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index acf1c454..e01ee6cf 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -483,7 +483,6 @@ fn move_next_word_start(cx: &mut Context) { let selection = doc .selection(view.id) .clone() - .min_width_1(text) .transform(|range| movement::move_next_word_start(text, range, count)); doc.set_selection(view.id, selection); } @@ -496,7 +495,6 @@ fn move_prev_word_start(cx: &mut Context) { let selection = doc .selection(view.id) .clone() - .min_width_1(text) .transform(|range| movement::move_prev_word_start(text, range, count)); doc.set_selection(view.id, selection); } @@ -509,7 +507,6 @@ fn move_next_word_end(cx: &mut Context) { let selection = doc .selection(view.id) .clone() - .min_width_1(text) .transform(|range| movement::move_next_word_end(text, range, count)); doc.set_selection(view.id, selection); } @@ -522,7 +519,6 @@ fn move_next_long_word_start(cx: &mut Context) { let selection = doc .selection(view.id) .clone() - .min_width_1(text) .transform(|range| movement::move_next_long_word_start(text, range, count)); doc.set_selection(view.id, selection); } @@ -535,7 +531,6 @@ fn move_prev_long_word_start(cx: &mut Context) { let selection = doc .selection(view.id) .clone() - .min_width_1(text) .transform(|range| movement::move_prev_long_word_start(text, range, count)); doc.set_selection(view.id, selection); } @@ -548,7 +543,6 @@ fn move_next_long_word_end(cx: &mut Context) { let selection = doc .selection(view.id) .clone() - .min_width_1(text) .transform(|range| movement::move_next_long_word_end(text, range, count)); doc.set_selection(view.id, selection); } @@ -574,15 +568,11 @@ fn extend_next_word_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc - .selection(view.id) - .clone() - .min_width_1(text) - .transform(|range| { - let word = movement::move_next_word_start(text, range, count); - let pos = word.cursor(text); - range.put_cursor(text, pos, true) - }); + let selection = doc.selection(view.id).clone().transform(|range| { + let word = movement::move_next_word_start(text, range, count); + let pos = word.cursor(text); + range.put_cursor(text, pos, true) + }); doc.set_selection(view.id, selection); } @@ -591,15 +581,11 @@ fn extend_prev_word_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc - .selection(view.id) - .clone() - .min_width_1(text) - .transform(|range| { - let word = movement::move_prev_word_start(text, range, count); - let pos = word.cursor(text); - range.put_cursor(text, pos, true) - }); + let selection = doc.selection(view.id).clone().transform(|range| { + let word = movement::move_prev_word_start(text, range, count); + let pos = word.cursor(text); + range.put_cursor(text, pos, true) + }); doc.set_selection(view.id, selection); } @@ -608,15 +594,11 @@ fn extend_next_word_end(cx: &mut Context) { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc - .selection(view.id) - .clone() - .min_width_1(text) - .transform(|range| { - let word = movement::move_next_word_end(text, range, count); - let pos = word.cursor(text); - range.put_cursor(text, pos, true) - }); + let selection = doc.selection(view.id).clone().transform(|range| { + let word = movement::move_next_word_end(text, range, count); + let pos = word.cursor(text); + range.put_cursor(text, pos, true) + }); doc.set_selection(view.id, selection); } @@ -663,13 +645,16 @@ where let text = doc.text().slice(..); let selection = doc.selection(view.id).clone().transform(|range| { - let range = if range.anchor < range.head { - // For block-cursor semantics. - Range::new(range.anchor, range.head - 1) + // TODO: use `Range::cursor()` here instead. However, that works in terms of + // graphemes, wheras this function does yet. So we're doing the same logic + // here, but just in terms of chars instead. + let search_start_pos = if range.anchor < range.head { + range.head - 1 } else { - range + range.head }; - search_fn(text, ch, range.head, count, inclusive) + + search_fn(text, ch, search_start_pos, count, inclusive) .map_or(range, |pos| range.put_cursor(text, pos, extend)) }); doc.set_selection(view.id, selection); @@ -795,11 +780,10 @@ fn replace(cx: &mut Context) { _ => None, }; - let text = doc.text().slice(..); - let selection = doc.selection(view.id).clone().min_width_1(text); + let selection = doc.selection(view.id); if let Some(ch) = ch { - let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { if !range.is_empty() { let text: String = RopeGraphemes::new(doc.text().slice(range.from()..range.to())) @@ -828,11 +812,8 @@ fn replace(cx: &mut Context) { fn switch_case(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc - .selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)); - let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + let selection = doc.selection(view.id); + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { let text: Tendril = range .fragment(doc.text().slice(..)) .chars() @@ -856,11 +837,8 @@ fn switch_case(cx: &mut Context) { fn switch_to_uppercase(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc - .selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)); - let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + let selection = doc.selection(view.id); + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { let text: Tendril = range.fragment(doc.text().slice(..)).to_uppercase().into(); (range.from(), range.to(), Some(text)) @@ -872,11 +850,8 @@ fn switch_to_uppercase(cx: &mut Context) { fn switch_to_lowercase(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc - .selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)); - let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + let selection = doc.selection(view.id); + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { let text: Tendril = range.fragment(doc.text().slice(..)).to_lowercase().into(); (range.from(), range.to(), Some(text)) @@ -1040,15 +1015,9 @@ fn split_selection_on_newline(cx: &mut Context) { fn search_impl(doc: &mut Document, view: &mut View, contents: &str, regex: &Regex, extend: bool) { let text = doc.text().slice(..); let selection = doc.selection(view.id); - let start = { - // Get the right side of the block cursor. - let range = selection.primary(); - if range.anchor < range.head { - range.head - } else { - graphemes::next_grapheme_boundary(text, range.head) - } - }; + + // Get the right side of the primary block cursor. + let start = graphemes::next_grapheme_boundary(text, selection.primary().cursor(text)); // use find_at to find the next match after the cursor, loop around the end // Careful, `Regex` uses `bytes` as offsets, not character indices! @@ -1132,7 +1101,7 @@ fn extend_line(cx: &mut Context) { let (view, doc) = current!(cx.editor); let text = doc.text(); - let range = doc.selection(view.id).primary().min_width_1(text.slice(..)); + let range = doc.selection(view.id).primary(); let (start_line, end_line) = range.line_range(text.slice(..)); let start = text.line_to_char(start_line); @@ -1168,14 +1137,14 @@ fn extend_to_line_bounds(cx: &mut Context) { fn delete_selection_impl(reg: &mut Register, doc: &mut Document, view_id: ViewId) { let text = doc.text().slice(..); - let selection = doc.selection(view_id).clone().min_width_1(text); + let selection = doc.selection(view_id); // first yank the selection let values: Vec = selection.fragments(text).map(Cow::into_owned).collect(); reg.write(values); // then delete - let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { (range.from(), range.to(), None) }); doc.apply(&transaction, view_id); @@ -1247,12 +1216,10 @@ fn append_mode(cx: &mut Context) { doc.restore_cursor = true; let text = doc.text().slice(..); - let selection = doc.selection(view.id).clone().min_width_1(text); - // Make sure there's room at the end of the document if the last // selection butts up against it. let end = text.len_chars(); - let last_range = selection.iter().last().unwrap(); + let last_range = doc.selection(view.id).iter().last().unwrap(); if !last_range.is_empty() && last_range.head == end { let transaction = Transaction::change( doc.text(), @@ -1261,7 +1228,7 @@ fn append_mode(cx: &mut Context) { doc.apply(&transaction, view.id); } - let selection = selection.transform(|range| { + let selection = doc.selection(view.id).clone().transform(|range| { Range::new( range.from(), graphemes::next_grapheme_boundary(doc.text().slice(..), range.to()), @@ -1689,10 +1656,7 @@ mod cmd { match cx.editor.clipboard_provider.get_contents() { Ok(contents) => { - let selection = doc - .selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)); + let selection = doc.selection(view.id); let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { (range.from(), range.to(), Some(contents.as_str().into())) @@ -2471,7 +2435,7 @@ fn select_mode(cx: &mut Context) { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - // Make sure all selections are at least 1-wide. + // Make sure end-of-document selections are also 1-width. // (With the exception of being in an empty document, of course.) let selection = doc.selection(view.id).clone().transform(|range| { if range.is_empty() && range.head == text.len_chars() { @@ -2480,7 +2444,7 @@ fn select_mode(cx: &mut Context) { range.head, ) } else { - range.min_width_1(text) + range } }); doc.set_selection(view.id, selection); @@ -3037,9 +3001,10 @@ pub mod insert { let text = doc.text().slice(..); let transaction = Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { + let pos = range.cursor(text); ( - graphemes::nth_prev_grapheme_boundary(text, range.head, count), - range.head, + graphemes::nth_prev_grapheme_boundary(text, pos, count), + pos, None, ) }); @@ -3052,9 +3017,10 @@ pub mod insert { let text = doc.text().slice(..); let transaction = Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { + let pos = range.cursor(text); ( - range.head, - graphemes::nth_next_grapheme_boundary(text, range.head, count), + pos, + graphemes::nth_next_grapheme_boundary(text, pos, count), None, ) }); @@ -3100,8 +3066,6 @@ fn yank(cx: &mut Context) { let values: Vec = doc .selection(view.id) - .clone() - .min_width_1(text) .fragments(text) .map(Cow::into_owned) .collect(); @@ -3125,8 +3089,6 @@ fn yank_joined_to_clipboard_impl(editor: &mut Editor, separator: &str) -> anyhow let values: Vec = doc .selection(view.id) - .clone() - .min_width_1(text) .fragments(text) .map(Cow::into_owned) .collect(); @@ -3157,11 +3119,7 @@ fn yank_main_selection_to_clipboard_impl(editor: &mut Editor) -> anyhow::Result< let (view, doc) = current!(editor); let text = doc.text().slice(..); - let value = doc - .selection(view.id) - .primary() - .min_width_1(text) - .fragment(text); + let value = doc.selection(view.id).primary().fragment(text); if let Err(e) = editor.clipboard_provider.set_contents(value.into_owned()) { bail!("Couldn't set system clipboard content: {:?}", e); @@ -3202,9 +3160,9 @@ fn paste_impl( let mut values = values.iter().cloned().map(Tendril::from).chain(repeat); let text = doc.text(); - let selection = doc.selection(view.id).clone().min_width_1(text.slice(..)); + let selection = doc.selection(view.id); - let transaction = Transaction::change_by_selection(text, &selection, |range| { + let transaction = Transaction::change_by_selection(text, selection, |range| { let pos = match (action, linewise) { // paste linewise before (Paste::Before, true) => text.line_to_char(text.char_to_line(range.from())), @@ -3257,11 +3215,8 @@ fn replace_with_yanked(cx: &mut Context) { if let Some(values) = registers.read(reg_name) { if let Some(yank) = values.first() { - let selection = doc - .selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)); - let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + let selection = doc.selection(view.id); + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { if !range.is_empty() { (range.from(), range.to(), Some(yank.as_str().into())) } else { @@ -3280,11 +3235,8 @@ fn replace_selections_with_clipboard_impl(editor: &mut Editor) -> anyhow::Result match editor.clipboard_provider.get_contents() { Ok(contents) => { - let selection = doc - .selection(view.id) - .clone() - .min_width_1(doc.text().slice(..)); - let transaction = Transaction::change_by_selection(doc.text(), &selection, |range| { + let selection = doc.selection(view.id); + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { (range.from(), range.to(), Some(contents.as_str().into())) }); @@ -3857,8 +3809,7 @@ fn surround_add(cx: &mut Context) { } = event { let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); - let selection = doc.selection(view.id).clone().min_width_1(text); + let selection = doc.selection(view.id); let (open, close) = surround::get_pair(ch); let mut changes = Vec::new(); @@ -3890,9 +3841,9 @@ fn surround_replace(cx: &mut Context) { { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).clone().min_width_1(text); + let selection = doc.selection(view.id); - let change_pos = match surround::get_surround_pos(text, &selection, from, count) + let change_pos = match surround::get_surround_pos(text, selection, from, count) { Some(c) => c, None => return, @@ -3927,9 +3878,9 @@ fn surround_delete(cx: &mut Context) { { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).clone().min_width_1(text); + let selection = doc.selection(view.id); - let change_pos = match surround::get_surround_pos(text, &selection, ch, count) { + let change_pos = match surround::get_surround_pos(text, selection, ch, count) { Some(c) => c, None => return, }; diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 84de4c43..c2078060 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -792,7 +792,8 @@ impl Document { pub fn set_selection(&mut self, view_id: ViewId, selection: Selection) { // TODO: use a transaction? - self.selections.insert(view_id, selection); + self.selections + .insert(view_id, selection.ensure_invariants(self.text().slice(..))); } fn apply_impl(&mut self, transaction: &Transaction, view_id: ViewId) -> bool { @@ -807,7 +808,12 @@ impl Document { .selection() .cloned() .unwrap_or_else(|| self.selection(view_id).clone().map(transaction.changes())); - self.set_selection(view_id, selection); + self.selections.insert(view_id, selection); + + // Ensure all selections accross all views still adhere to invariants. + for selection in self.selections.values_mut() { + *selection = selection.clone().ensure_invariants(self.text.slice(..)); + } } if !transaction.changes().is_empty() { -- cgit v1.2.3-70-g09d2 From 285aba2de5af5d7a45b0640e7e09764b611f6a7b Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Wed, 28 Jul 2021 16:03:34 -0700 Subject: Fix bug with `/` searching after non-ascii characters. Forgot to convert from char indices to byte indices before passing to the regex engine. --- helix-term/src/commands.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index e01ee6cf..baa3e897 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1017,7 +1017,10 @@ fn search_impl(doc: &mut Document, view: &mut View, contents: &str, regex: &Rege let selection = doc.selection(view.id); // Get the right side of the primary block cursor. - let start = graphemes::next_grapheme_boundary(text, selection.primary().cursor(text)); + let start = text.char_to_byte(graphemes::next_grapheme_boundary( + text, + selection.primary().cursor(text), + )); // use find_at to find the next match after the cursor, loop around the end // Careful, `Regex` uses `bytes` as offsets, not character indices! -- cgit v1.2.3-70-g09d2 From e4d41d06e3b52863d35ce3703f78cc8e0807c504 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Wed, 28 Jul 2021 19:20:23 -0700 Subject: Fix typo in comment. --- helix-term/src/commands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'helix-term/src/commands.rs') diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index baa3e897..dc28c098 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -646,7 +646,7 @@ where let selection = doc.selection(view.id).clone().transform(|range| { // TODO: use `Range::cursor()` here instead. However, that works in terms of - // graphemes, wheras this function does yet. So we're doing the same logic + // graphemes, whereas this function doesn't yet. So we're doing the same logic // here, but just in terms of chars instead. let search_start_pos = if range.anchor < range.head { range.head - 1 -- cgit v1.2.3-70-g09d2