Make `Selection`'s normalize and transform methods self-consuming only.

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());
 

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<F>(&mut self, f: F) -> &mut Self
+    pub fn transform<F>(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<F>(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<Item = Cow<str>> + 'a {
         self.ranges.iter().map(move |range| range.fragment(text))
     }

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);
+    doc.set_selection(
-    selection.transform(|range| {
+        view.id,
-        movement::move_horizontally(text, range, Direction::Backward, count, Movement::Move)
+        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);
+    doc.set_selection(
-    selection.transform(|range| {
+        view.id,
-        movement::move_horizontally(text, range, Direction::Forward, count, Movement::Move)
+        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);
+    doc.set_selection(
-    selection.transform(|range| {
+        view.id,
-        movement::move_vertically(text, range, Direction::Backward, count, Movement::Move)
+        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);
+    doc.set_selection(
-    selection.transform(|range| {
+        view.id,
-        movement::move_vertically(text, range, Direction::Forward, count, Movement::Move)
+        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);
+            let pos = line_end_char_index(&text.slice(..), line);
-    selection.transform(|range| {
+            let pos = graphemes::nth_prev_grapheme_boundary(text.slice(..), pos, 1);
-        let line = text.char_to_line(range.head);
+            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);
+            // adjust to start of the line
-    selection.transform(|range| {
+            let pos = text.line_to_char(line);
-        let line = text.char_to_line(range.head);
+            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);
+            if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) {
-    selection.transform(|range| {
+                let pos = pos + text.line_to_char(line_idx);
-        let line_idx = text.char_to_line(range.head);
+                Range::new(pos, pos)
-
+            } else {
-        if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) {
+                range
-            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);
+    doc.set_selection(
-    selection.transform(|range| movement::move_next_word_start(text, range, count));
+        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);
-
+    doc.set_selection(
-    let (text, selection) = doc.text_and_mut_selection(view.id);
+        view.id,
-    selection.transform(|range| movement::move_prev_word_start(text, range, count));
+        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);
-
+    doc.set_selection(
-    let (text, selection) = doc.text_and_mut_selection(view.id);
+        view.id,
-    selection.transform(|range| movement::move_next_word_end(text, range, count));
+        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);
-
+    doc.set_selection(
-    let (text, selection) = doc.text_and_mut_selection(view.id);
+        view.id,
-    selection.transform(|range| movement::move_next_long_word_start(text, range, count));
+        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);
-
+    doc.set_selection(
-    let (text, selection) = doc.text_and_mut_selection(view.id);
+        view.id,
-    selection.transform(|range| movement::move_prev_long_word_start(text, range, count));
+        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);
-
+    doc.set_selection(
-    let (text, selection) = doc.text_and_mut_selection(view.id);
+        view.id,
-    selection.transform(|range| movement::move_next_long_word_end(text, range, count));
+        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);
-
+    doc.set_selection(
-    let (text, selection) = doc.text_and_mut_selection(view.id);
+        view.id,
-    selection.transform(|mut range| {
+        doc.selection(view.id).clone().transform(|mut range| {
-        let word = movement::move_next_word_start(text, range, count);
+            let word = movement::move_next_word_start(doc.text().slice(..), range, count);
-        let pos = word.head;
+            let pos = word.head;
-        Range::new(range.anchor, pos)
+            Range::new(range.anchor, pos)
-    });
+        }),
+    );
 }
 
 fn extend_prev_word_start(cx: &mut Context) {
     let count = cx.count();
     let (view, doc) = current!(cx.editor);
-
+    doc.set_selection(
-    let (text, selection) = doc.text_and_mut_selection(view.id);
+        view.id,
-    selection.transform(|mut range| {
+        doc.selection(view.id).clone().transform(|mut range| {
-        let word = movement::move_prev_word_start(text, range, count);
+            let word = movement::move_prev_word_start(doc.text().slice(..), range, count);
-        let pos = word.head;
+            let pos = word.head;
-        Range::new(range.anchor, pos)
+            Range::new(range.anchor, pos)
-    });
+        }),
+    );
 }
 
 fn extend_next_word_end(cx: &mut Context) {
     let count = cx.count();
     let (view, doc) = current!(cx.editor);
-
+    doc.set_selection(
-    let (text, selection) = doc.text_and_mut_selection(view.id);
+        view.id,
-    selection.transform(|mut range| {
+        doc.selection(view.id).clone().transform(|mut range| {
-        let word = movement::move_next_word_end(text, range, count);
+            let word = movement::move_next_word_end(doc.text().slice(..), range, count);
-        let pos = word.head;
+            let pos = word.head;
-        Range::new(range.anchor, pos)
+            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);
+        doc.set_selection(
-        selection.transform(|mut range| {
+            view.id,
-            search_fn(text, ch, range.head, count, inclusive).map_or(range, |pos| {
+            doc.selection(view.id).clone().transform(|mut range| {
-                if extend {
+                search_fn(doc.text().slice(..), ch, range.head, count, inclusive).map_or(
-                    Range::new(range.anchor, pos)
+                    range,
-                } else {
+                    |pos| {
-                    // select
+                        if extend {
-                    Range::new(range.head, pos)
+                            Range::new(range.anchor, pos)
-                }
+                        } else {
-                // or (pos, pos) to move to found val
+                            // 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);
+            if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) {
-    selection.transform(|range| {
+                let pos = pos + text.line_to_char(line_idx);
-        let line_idx = text.char_to_line(range.head);
+                Range::new(range.anchor, pos)
-
+            } else {
-        if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) {
+                range
-            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);
+    doc.set_selection(
-    selection.transform(|range| {
+        view.id,
-        movement::move_horizontally(text, range, Direction::Backward, count, Movement::Extend)
+        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);
+    doc.set_selection(
-    selection.transform(|range| {
+        view.id,
-        movement::move_horizontally(text, range, Direction::Forward, count, Movement::Extend)
+        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);
+    doc.set_selection(
-    selection.transform(|range| {
+        view.id,
-        movement::move_vertically(text, range, Direction::Backward, count, Movement::Extend)
+        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);
+    doc.set_selection(
-    selection.transform(|range| {
+        view.id,
-        movement::move_vertically(text, range, Direction::Forward, count, Movement::Extend)
+        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);
+            let pos = line_end_char_index(&text, line);
-    selection.transform(|range| {
+            let pos = graphemes::nth_prev_grapheme_boundary(text, pos, 1);
-        let line = text.char_to_line(range.head);
+            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(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);
+            // adjust to start of the line
-    selection.transform(|range| {
+            let pos = text.line_to_char(line);
-        let line = text.char_to_line(range.head);
+            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)
+    doc.set_selection(
-        .transform(|range| Range::new(range.head, range.head));
+        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)
+    doc.set_selection(
-        .transform(|range| Range::new(range.head, range.anchor));
+        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)
+    doc.set_selection(
-        .transform(|range| Range::new(range.to(), range.from()));
+        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);
+    let selection = doc.selection(view.id).clone().transform(|range| {
-    selection.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);
+    doc.set_selection(
-    selection.transform(|range| {
+        view.id,
-        let line = text.char_to_line(range.head);
+        doc.selection(view.id).clone().transform(|range| {
-        let pos = line_end_char_index(&text.slice(..), line);
+            let text = &doc.text().slice(..);
-        Range::new(pos, pos)
+            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);
+        doc.set_selection(
-        selection.transform(|range| {
+            view.id,
-            Range::new(
+            doc.selection(view.id).clone().transform(|range| {
-                range.from(),
+                Range::new(
-                graphemes::prev_grapheme_boundary(text, range.to()),
+                    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);
+        doc.set_selection(
-        selection.transform(|range| movement::move_prev_word_start(text, range, count));
+            view.id,
+            doc.selection(view.id).clone().transform(|range| {
+                movement::move_prev_word_start(doc.text().slice(..), range, count)
+            }),
+        );
         delete_selection(cx)
     }
 }

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,
+    ChangeSet, Diagnostic, LineEnding, Rope, RopeBuilder, Selection, State, Syntax, Transaction,
-    Transaction, DEFAULT_LINE_ENDING,
+    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<PathBuf> {
         let cwdir = std::env::current_dir().expect("couldn't determine current directory");