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-view/src/document.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'helix-view/src/document.rs') diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 0f1f3a8f..59a1c42c 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, Selection, State, Syntax, Transaction, - DEFAULT_LINE_ENDING, + ChangeSet, Diagnostic, LineEnding, Rope, RopeBuilder, RopeSlice, Selection, State, Syntax, + Transaction, DEFAULT_LINE_ENDING, }; use helix_lsp::util::LspFormatting; @@ -1000,6 +1000,22 @@ 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 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-view/src/document.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 22dca3b111513f4dc0852acf0bfb0229a8661904 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Thu, 1 Jul 2021 23:36:09 -0700 Subject: Allow last line in file to lack a line break character. --- helix-core/src/movement.rs | 16 ++++++----- helix-core/src/syntax.rs | 18 +++++++++--- helix-term/src/ui/editor.rs | 67 +++++++++++++++++++++++++++++---------------- helix-view/src/document.rs | 14 ++-------- 4 files changed, 70 insertions(+), 45 deletions(-) (limited to 'helix-view/src/document.rs') diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index acc95e7e..b810876c 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -65,7 +65,7 @@ pub fn move_vertically( Direction::Backward => row.saturating_sub(count), Direction::Forward => std::cmp::min( row.saturating_add(count), - slice.len_lines().saturating_sub(2), + slice.len_lines().saturating_sub(1), ), }; @@ -402,12 +402,13 @@ mod test { let moves_and_expected_coordinates = IntoIter::new([ ((Direction::Forward, 1usize), (1, 0)), ((Direction::Forward, 2usize), (3, 0)), + ((Direction::Forward, 1usize), (4, 0)), ((Direction::Backward, 999usize), (0, 0)), - ((Direction::Forward, 3usize), (3, 0)), - ((Direction::Forward, 0usize), (3, 0)), - ((Direction::Backward, 0usize), (3, 0)), - ((Direction::Forward, 5), (4, 0)), - ((Direction::Forward, 999usize), (4, 0)), + ((Direction::Forward, 4usize), (4, 0)), + ((Direction::Forward, 0usize), (4, 0)), + ((Direction::Backward, 0usize), (4, 0)), + ((Direction::Forward, 5), (5, 0)), + ((Direction::Forward, 999usize), (5, 0)), ]); for ((direction, amount), coordinates) in moves_and_expected_coordinates { @@ -439,7 +440,8 @@ mod test { ((Axis::V, Direction::Forward, 1usize), (3, 8)), // Behaviour is preserved even through long jumps ((Axis::V, Direction::Backward, 999usize), (0, 8)), - ((Axis::V, Direction::Forward, 999usize), (4, 8)), + ((Axis::V, Direction::Forward, 4usize), (4, 8)), + ((Axis::V, Direction::Forward, 999usize), (5, 0)), ]); for ((axis, direction, amount), coordinates) in moves_and_expected_coordinates { diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 9dbb2c03..d4379a8e 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -1758,10 +1758,20 @@ impl> Iterator for Merge { self.next_event = self.iter.next(); Some(event) } - // can happen if deleting and cursor at EOF, and diagnostic reaches past the end - (None, Some((_, _))) => { - self.next_span = None; - None + // Can happen if cursor at EOF and/or diagnostic reaches past the end. + // We need to actually emit events for the cursor-at-EOF situation, + // even though the range is past the end of the text. This needs to be + // handled appropriately by the drawing code by not assuming that + // all `Source` events point to valid indices in the rope. + (None, Some((span, range))) => { + let event = HighlightStart(Highlight(*span)); + self.queue.push(HighlightEnd); + self.queue.push(Source { + start: range.start, + end: range.end, + }); + self.next_span = self.spans.next(); + Some(event) } (None, None) => None, e => unreachable!("{:?}", e), diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 138456ca..dab654ad 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -8,7 +8,7 @@ use crate::{ use helix_core::{ coords_at_pos, - graphemes::ensure_grapheme_boundary_next, + graphemes::{ensure_grapheme_boundary_next, next_grapheme_boundary, prev_grapheme_boundary}, syntax::{self, HighlightEvent}, LineEnding, Position, Range, }; @@ -165,21 +165,18 @@ impl EditorView { } .unwrap_or(base_cursor_scope); - let primary_selection_scope = theme - .find_scope_index("ui.selection.primary") - .unwrap_or(selection_scope); - let highlights: Box> = if is_focused { - // inject selections as highlight scopes - let mut spans: Vec<(usize, std::ops::Range)> = Vec::new(); - // TODO: primary + insert mode patching: // (ui.cursor.primary).patch(mode).unwrap_or(cursor) - let primary_cursor_scope = theme .find_scope_index("ui.cursor.primary") .unwrap_or(cursor_scope); + let primary_selection_scope = theme + .find_scope_index("ui.selection.primary") + .unwrap_or(selection_scope); + // inject selections as highlight scopes + let mut spans: Vec<(usize, std::ops::Range)> = Vec::new(); for (i, range) in selections.iter().enumerate() { let (cursor_scope, selection_scope) = if i == primary_idx { (primary_cursor_scope, primary_selection_scope) @@ -187,19 +184,23 @@ impl EditorView { (cursor_scope, selection_scope) }; - if range.head == range.anchor { + // Special-case: cursor at end of the rope. + if range.head == range.anchor && range.head == text.len_chars() { spans.push((cursor_scope, range.head..range.head + 1)); continue; } - let reverse = range.head < range.anchor; - - if reverse { - spans.push((cursor_scope, range.head..range.head + 1)); - spans.push((selection_scope, range.head + 1..range.anchor + 1)); + let range = range.min_width_1(text); + if range.head > range.anchor { + // Standard case. + let cursor_start = prev_grapheme_boundary(text, range.head); + spans.push((selection_scope, range.anchor..cursor_start)); + spans.push((cursor_scope, cursor_start..range.head)); } else { - spans.push((selection_scope, range.anchor..range.head)); - spans.push((cursor_scope, range.head..range.head + 1)); + // Reverse case. + let cursor_end = next_grapheme_boundary(text, range.head); + spans.push((cursor_scope, range.head..cursor_end)); + spans.push((selection_scope, cursor_end..range.anchor)); } } @@ -232,7 +233,10 @@ impl EditorView { spans.pop(); } HighlightEvent::Source { start, end } => { - let text = text.slice(start..end); + // `unwrap_or_else` part is for off-the-end indices of + // the rope, to allow cursor highlighting at the end + // of the rope. + let text = text.get_slice(start..end).unwrap_or_else(|| " ".into()); use helix_core::graphemes::{grapheme_width, RopeGraphemes}; @@ -301,7 +305,11 @@ impl EditorView { let info: Style = theme.get("info"); let hint: Style = theme.get("hint"); - for (i, line) in (view.first_line..last_line).enumerate() { + // Whether to draw the line number for the last line of the + // document or not. We only draw it if it's not an empty line. + let draw_last = text.line_to_byte(last_line) < text.len_bytes(); + + for (i, line) in (view.first_line..=last_line).enumerate() { use helix_core::diagnostic::Severity; if let Some(diagnostic) = doc.diagnostics().iter().find(|d| d.line == line) { surface.set_stringn( @@ -318,11 +326,17 @@ impl EditorView { ); } - // line numbers having selections are rendered differently + // Line numbers having selections are rendered + // differently, further below. + let line_number_text = if line == last_line && !draw_last { + " ~".into() + } else { + format!("{:>5}", line + 1) + }; surface.set_stringn( viewport.x + 1 - OFFSET, viewport.y + i as u16, - format!("{:>5}", line + 1), + line_number_text, 5, linenr, ); @@ -336,7 +350,7 @@ impl EditorView { if is_focused { let screen = { let start = text.line_to_char(view.first_line); - let end = text.line_to_char(last_line + 1); + let end = text.line_to_char(last_line + 1) + 1; // +1 for cursor at end of text. Range::new(start, end) }; @@ -345,10 +359,17 @@ impl EditorView { for selection in selection.iter().filter(|range| range.overlaps(&screen)) { let head = view.screen_coords_at_pos(doc, text, selection.head); if let Some(head) = head { + // Draw line number for selected lines. + let line_number = view.first_line + head.row; + let line_number_text = if line_number == last_line && !draw_last { + " ~".into() + } else { + format!("{:>5}", line_number + 1) + }; surface.set_stringn( viewport.x + 1 - OFFSET, viewport.y + head.row as u16, - format!("{:>5}", view.first_line + head.row + 1), + line_number_text, 5, linenr_select, ); diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 0f1f3a8f..9b609429 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -448,15 +448,7 @@ impl Document { } let mut file = std::fs::File::open(&path).context(format!("unable to open {:?}", path))?; - let (mut rope, encoding) = from_reader(&mut file, encoding)?; - - // search for line endings - let line_ending = auto_detect_line_ending(&rope).unwrap_or(DEFAULT_LINE_ENDING); - - // add missing newline at the end of file - if rope.len_bytes() == 0 || !char_is_line_ending(rope.char(rope.len_chars() - 1)) { - rope.insert(rope.len_chars(), line_ending.as_str()); - } + let (rope, encoding) = from_reader(&mut file, encoding)?; let mut doc = Self::from(rope, Some(encoding)); @@ -466,9 +458,9 @@ impl Document { doc.detect_language(theme, loader); } - // Detect indentation style and set line ending. + // Detect indentation style and line ending. doc.detect_indent_style(); - doc.line_ending = line_ending; + doc.line_ending = auto_detect_line_ending(&doc.text).unwrap_or(DEFAULT_LINE_ENDING); Ok(doc) } -- cgit v1.2.3-70-g09d2 From 7961a13007b90840f08390d1ab9822ebbb0740c5 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Thu, 1 Jul 2021 23:39:49 -0700 Subject: Make new documents empty, rather than starting with a line ending. --- helix-view/src/document.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'helix-view/src/document.rs') diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 9b609429..bc15da96 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1036,7 +1036,7 @@ impl Document { impl Default for Document { fn default() -> Self { - let text = Rope::from(DEFAULT_LINE_ENDING.as_str()); + let text = Rope::from(""); Self::from(text, None) } } -- cgit v1.2.3-70-g09d2 From 28627f97e9f0d7bc61368ecd221b1a0fc378f3b7 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Fri, 2 Jul 2021 00:06:53 -0700 Subject: Fix empty document test. --- helix-view/src/document.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'helix-view/src/document.rs') diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index bc15da96..86f3dfb8 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1161,11 +1161,7 @@ mod test { #[test] fn test_line_ending() { - if cfg!(windows) { - assert_eq!(Document::default().text().to_string(), "\r\n"); - } else { - assert_eq!(Document::default().text().to_string(), "\n"); - } + assert_eq!(Document::default().text().to_string(), ""); } macro_rules! test_decode { -- cgit v1.2.3-70-g09d2 From 198fe409513d43bc2c38fbb148131bf8cab58d19 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Tue, 20 Jul 2021 18:40:41 -0700 Subject: Don't insert a final line ending on file load/reload. --- helix-view/src/document.rs | 27 ++++++--------------------- 1 file changed, 6 insertions(+), 21 deletions(-) (limited to 'helix-view/src/document.rs') diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index a04af94d..2f2b7479 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -306,19 +306,6 @@ pub async fn to_writer<'a, W: tokio::io::AsyncWriteExt + Unpin + ?Sized>( Ok(()) } -/// Inserts the final line ending into `rope` if it's missing. [Why?](https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline) -pub fn with_line_ending(rope: &mut Rope) -> LineEnding { - // search for line endings - let line_ending = auto_detect_line_ending(rope).unwrap_or(DEFAULT_LINE_ENDING); - - // add missing newline at the end of file - if rope.len_bytes() == 0 || !char_is_line_ending(rope.char(rope.len_chars() - 1)) { - rope.insert(rope.len_chars(), line_ending.as_str()); - } - - line_ending -} - /// Like std::mem::replace() except it allows the replacement value to be mapped from the /// original value. fn take_with(mut_ref: &mut T, closure: F) @@ -456,7 +443,7 @@ impl Document { theme: Option<&Theme>, config_loader: Option<&syntax::Loader>, ) -> Result { - let (mut rope, encoding) = if path.exists() { + let (rope, encoding) = if path.exists() { let mut file = std::fs::File::open(&path).context(format!("unable to open {:?}", path))?; from_reader(&mut file, encoding)? @@ -465,7 +452,6 @@ impl Document { (Rope::from(DEFAULT_LINE_ENDING.as_str()), encoding) }; - let line_ending = with_line_ending(&mut rope); let mut doc = Self::from(rope, Some(encoding)); // set the path and try detecting the language @@ -474,9 +460,9 @@ impl Document { doc.detect_language(theme, loader); } - // Detect indentation style and set line ending. + // Detect indentation style and line ending. doc.detect_indent_style(); - doc.line_ending = line_ending; + doc.line_ending = auto_detect_line_ending(&doc.text).unwrap_or(DEFAULT_LINE_ENDING); Ok(doc) } @@ -605,16 +591,15 @@ impl Document { } let mut file = std::fs::File::open(path.unwrap())?; - let (mut rope, ..) = from_reader(&mut file, Some(encoding))?; - let line_ending = with_line_ending(&mut rope); + let (rope, ..) = from_reader(&mut file, Some(encoding))?; let transaction = helix_core::diff::compare_ropes(self.text(), &rope); self.apply(&transaction, view_id); self.append_changes_to_history(view_id); - // Detect indentation style and set line ending. + // Detect indentation style and line ending. self.detect_indent_style(); - self.line_ending = line_ending; + self.line_ending = auto_detect_line_ending(&self.text).unwrap_or(DEFAULT_LINE_ENDING); Ok(()) } -- 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-view/src/document.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