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 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

(limited to 'helix-core/src/movement.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 {
-- 
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-core/src/movement.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 6e15c9b8745e9708ee5271c8701d41a8393cb038 Mon Sep 17 00:00:00 2001
From: Nathan Vegdahl
Date: Mon, 5 Jul 2021 18:58:33 -0700
Subject: Make vertical selection movement work properly.

---
 helix-core/src/line_ending.rs |  7 +++++
 helix-core/src/movement.rs    | 73 +++++++++++++++++++++++++++++--------------
 helix-core/src/position.rs    |  4 +++
 3 files changed, 60 insertions(+), 24 deletions(-)

(limited to 'helix-core/src/movement.rs')

diff --git a/helix-core/src/line_ending.rs b/helix-core/src/line_ending.rs
index e3ff6478..18ea5f9f 100644
--- a/helix-core/src/line_ending.rs
+++ b/helix-core/src/line_ending.rs
@@ -159,6 +159,13 @@ pub fn line_end_char_index(slice: &RopeSlice, line: usize) -> usize {
             .unwrap_or(0)
 }
 
+/// Fetches line `line_idx` from the passed rope slice, sans any line ending.
+pub fn line_without_line_ending<'a>(slice: &'a RopeSlice, line_idx: usize) -> RopeSlice<'a> {
+    let start = slice.line_to_char(line_idx);
+    let end = line_end_char_index(slice, line_idx);
+    slice.slice(start..end)
+}
+
 /// Returns the char index of the end of the given RopeSlice, not including
 /// any final line ending.
 pub fn rope_end_without_line_ending(slice: &RopeSlice) -> usize {
diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs
index e01786eb..62311ee4 100644
--- a/helix-core/src/movement.rs
+++ b/helix-core/src/movement.rs
@@ -7,9 +7,9 @@ use crate::{
     coords_at_pos,
     graphemes::{
         next_grapheme_boundary, nth_next_grapheme_boundary, nth_prev_grapheme_boundary,
-        prev_grapheme_boundary,
+        prev_grapheme_boundary, RopeGraphemes,
     },
-    line_ending::get_line_ending,
+    line_ending::line_without_line_ending,
     pos_at_coords, Position, Range, RopeSlice,
 };
 
@@ -95,36 +95,61 @@ pub fn move_vertically(
     count: usize,
     behaviour: Movement,
 ) -> Range {
-    let Position { row, col } = coords_at_pos(slice, range.head);
+    // 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
+    };
 
+    // Compute the current position's 2d coordinates.
+    let Position { row, col } = coords_at_pos(slice, pos);
     let horiz = range.horiz.unwrap_or(col as u32);
 
-    let new_line = match dir {
-        Direction::Backward => row.saturating_sub(count),
-        Direction::Forward => std::cmp::min(
-            row.saturating_add(count),
-            slice.len_lines().saturating_sub(1),
-        ),
-    };
+    // Compute the new position.
+    let new_pos = {
+        let new_row = if dir == Direction::Backward {
+            row.saturating_sub(count)
+        } 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);
 
-    // Length of the line sans line-ending.
-    let new_line_len = {
-        let line = slice.line(new_line);
-        line.len_chars() - get_line_ending(&line).map(|le| le.len_chars()).unwrap_or(0)
+        pos_at_coords(slice, Position::new(new_row, new_col))
     };
 
-    let new_col = std::cmp::min(horiz as usize, new_line_len);
-
-    let pos = pos_at_coords(slice, Position::new(new_line, new_col));
+    // 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 => {
+            let new_head = if new_pos >= range.anchor {
+                next_grapheme_boundary(slice, new_pos)
+            } else {
+                new_pos
+            };
 
-    let anchor = match behaviour {
-        Movement::Extend => range.anchor,
-        Movement::Move => pos,
-    };
+            let new_anchor = if range.anchor <= range.head && range.anchor > new_head {
+                next_grapheme_boundary(slice, range.anchor)
+            } else if range.anchor > range.head && range.anchor < new_head {
+                prev_grapheme_boundary(slice, range.anchor)
+            } else {
+                range.anchor
+            };
 
-    let mut range = Range::new(anchor, pos);
-    range.horiz = Some(horiz);
-    range
+            Range {
+                anchor: new_anchor,
+                head: new_head,
+                horiz: Some(horiz),
+            }
+        }
+    }
 }
 
 pub fn move_next_word_start(slice: RopeSlice, range: Range, count: usize) -> Range {
diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs
index 3d114b52..c4e8c9d6 100644
--- a/helix-core/src/position.rs
+++ b/helix-core/src/position.rs
@@ -53,6 +53,8 @@ impl From<Position> for tree_sitter::Point {
 }
 /// Convert a character index to (line, column) coordinates.
 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();
@@ -61,6 +63,8 @@ pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position {
 
 /// 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.
     let Position { row, col } = coords;
     let line_start = text.line_to_char(row);
     // line_start + col
-- 
cgit v1.2.3-70-g09d2


From 753f7f381b96f591be3c0e37a93b90776c5405cb Mon Sep 17 00:00:00 2001
From: Nathan Vegdahl
Date: Wed, 7 Jul 2021 17:24:39 -0700
Subject: Implement `Range::put()` which manages range movements and
 extensions.

In particular, this wraps the annoying logic involved in keeping the
cursor width to 1 grapheme.
---
 helix-core/src/movement.rs  | 91 +++++++++++++--------------------------------
 helix-core/src/selection.rs | 23 ++++++++++++
 2 files changed, 49 insertions(+), 65 deletions(-)

(limited to 'helix-core/src/movement.rs')

diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs
index bc56f9a4..0e2a2a42 100644
--- a/helix-core/src/movement.rs
+++ b/helix-core/src/movement.rs
@@ -32,60 +32,31 @@ pub fn move_horizontally(
     count: usize,
     behaviour: Movement,
 ) -> Range {
-    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)
-        }
-        (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)
-        }
-        (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);
-            }
+    use Movement::Extend;
 
-            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);
-            }
+    // 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
+    };
 
-            Range::new(anchor, head)
-        }
-    }
+    // Compute the new position.
+    let mut 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, behaviour == Extend, new_pos)
 }
 
 pub fn move_vertically(
@@ -135,19 +106,9 @@ pub fn move_vertically(
                 new_pos
             };
 
-            let new_anchor = if range.anchor <= range.head && range.anchor > new_head {
-                next_grapheme_boundary(slice, range.anchor)
-            } else if range.anchor > range.head && range.anchor < new_head {
-                prev_grapheme_boundary(slice, range.anchor)
-            } else {
-                range.anchor
-            };
-
-            Range {
-                anchor: new_anchor,
-                head: new_head,
-                horiz: Some(horiz),
-            }
+            let mut new_range = range.put(slice, true, new_head);
+            new_range.horiz = Some(horiz);
+            new_range
         }
     }
 }
diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs
index 64ff51d8..8951899b 100644
--- a/helix-core/src/selection.rs
+++ b/helix-core/src/selection.rs
@@ -5,6 +5,7 @@
 use crate::{
     graphemes::{
         ensure_grapheme_boundary_next, ensure_grapheme_boundary_prev, next_grapheme_boundary,
+        prev_grapheme_boundary,
     },
     Assoc, ChangeSet, RopeSlice,
 };
@@ -208,6 +209,28 @@ 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.
+    ///
+    /// This method assumes that the range and `char_idx` are already properly
+    /// grapheme-aligned.
+    #[must_use]
+    #[inline]
+    pub fn put(self, text: RopeSlice, extend: bool, char_idx: usize) -> Range {
+        let anchor = if !extend {
+            char_idx
+        } else 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
+        };
+
+        Range::new(anchor, char_idx)
+    }
+
     // groupAt
 
     #[inline]
-- 
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-core/src/movement.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<usize> {
-    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<usize> {
-    // 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 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-core/src/movement.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<dyn Iterator<Item = char>> = 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<usize> = None;
-        let is_boundary =
-            |a: char, b: Option<char>| 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 e98d669c303576802291ef9826a22066e4191808 Mon Sep 17 00:00:00 2001
From: Nathan Vegdahl
Date: Mon, 19 Jul 2021 12:30:08 -0700
Subject: Handle edge case in `range_to_target()` correctly.

---
 helix-core/src/movement.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'helix-core/src/movement.rs')

diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs
index 2d9798bf..21e97ae8 100644
--- a/helix-core/src/movement.rs
+++ b/helix-core/src/movement.rs
@@ -285,7 +285,7 @@ impl CharHelpers for Chars<'_> {
         // 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 prev_ch.is_none() || reached_target(target, prev_ch.unwrap(), next_ch) {
                 if head == head_start {
                     anchor = head;
                 } else {
-- 
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-core/src/movement.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<Position> 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<str> = 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<str> = 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 fd684ef6931d11632274d87c9a42d4fabb51c074 Mon Sep 17 00:00:00 2001
From: Nathan Vegdahl
Date: Thu, 22 Jul 2021 13:21:44 -0700
Subject: Revert display-width-based vertical cursor movement.

Still needs to be done, but should be part of a separate PR.
---
 helix-core/src/movement.rs | 18 +++++-----
 helix-core/src/position.rs | 89 +++++++++++++++++++++++++---------------------
 2 files changed, 57 insertions(+), 50 deletions(-)

(limited to 'helix-core/src/movement.rs')

diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs
index 01bec47a..7f3a5ef4 100644
--- a/helix-core/src/movement.rs
+++ b/helix-core/src/movement.rs
@@ -521,16 +521,14 @@ mod test {
             V,
         }
         let moves_and_expected_coordinates = IntoIter::new([
-            // 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)),
+            // Places cursor at the fourth kana.
+            ((Axis::H, Direction::Forward, 4), (0, 4)),
+            // Descent places cursor at the 4th character.
+            ((Axis::V, Direction::Forward, 1usize), (1, 4)),
+            // Moving back 1 character.
+            ((Axis::H, Direction::Backward, 1usize), (1, 3)),
+            // Jumping back up 1 line.
+            ((Axis::V, Direction::Backward, 1usize), (0, 3)),
         ]);
 
         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 15627687..611e6b76 100644
--- a/helix-core/src/position.rs
+++ b/helix-core/src/position.rs
@@ -1,8 +1,6 @@
-use std::borrow::Cow;
-
 use crate::{
     chars::char_is_line_ending,
-    graphemes::{ensure_grapheme_boundary_prev, grapheme_width, RopeGraphemes},
+    graphemes::{ensure_grapheme_boundary_prev, RopeGraphemes},
     line_ending::line_end_char_index,
     RopeSlice,
 };
@@ -66,12 +64,7 @@ pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position {
 
     let line_start = text.line_to_char(line);
     let pos = ensure_grapheme_boundary_prev(text, pos);
-    let col = RopeGraphemes::new(text.slice(line_start..pos))
-        .map(|g| {
-            let g: Cow<str> = g.into();
-            grapheme_width(&g)
-        })
-        .sum();
+    let col = RopeGraphemes::new(text.slice(line_start..pos)).count();
 
     Position::new(line, col)
 }
@@ -83,6 +76,9 @@ pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position {
 /// `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.
+///
+/// TODO: this should be changed to work in terms of visual row/column, not
+/// graphemes.
 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);
@@ -92,17 +88,11 @@ pub fn pos_at_coords(text: RopeSlice, coords: Position, is_1_width: bool) -> usi
         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<str> = g.into();
-        let next_col = prev_col + grapheme_width(&g);
-
-        if next_col > col {
+    for (i, g) in RopeGraphemes::new(text.slice(line_start..line_end)).enumerate() {
+        if i == col {
             break;
         }
-
-        prev_col = next_col;
         col_char_offset += g.chars().count();
     }
 
@@ -131,17 +121,18 @@ mod test {
         assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d
 
         // Test with wide characters.
+        // TODO: account for character width.
         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, 1), (0, 1).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, 4), (0, 4).into());
+        assert_eq!(coords_at_pos(slice, 5), (0, 5).into());
         assert_eq!(coords_at_pos(slice, 6), (1, 0).into());
 
-        // test with grapheme clusters
+        // Test with grapheme clusters.
         let text = Rope::from("a̐éö̲\r\n");
         let slice = text.slice(..);
         assert_eq!(coords_at_pos(slice, 0), (0, 0).into());
@@ -150,13 +141,23 @@ mod test {
         assert_eq!(coords_at_pos(slice, 7), (0, 3).into());
         assert_eq!(coords_at_pos(slice, 9), (1, 0).into());
 
+        // Test with wide-character grapheme clusters.
+        // TODO: account for character width.
         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, 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, 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, 6), (1, 0).into());
+
+        // Test with tabs.
+        // Todo: account for tab stops.
+        let text = Rope::from("\tHello\n");
+        let slice = text.slice(..);
+        assert_eq!(coords_at_pos(slice, 0), (0, 0).into());
+        assert_eq!(coords_at_pos(slice, 1), (0, 1).into());
+        assert_eq!(coords_at_pos(slice, 2), (0, 2).into());
     }
 
     #[test]
@@ -172,21 +173,20 @@ mod test {
         assert_eq!(pos_at_coords(slice, (1, 4).into(), false), 10); // position on d
 
         // Test with wide characters.
+        // TODO: account for character width.
         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, (0, 1).into(), false), 1);
+        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), 4);
+        assert_eq!(pos_at_coords(slice, (0, 5).into(), false), 5);
+        assert_eq!(pos_at_coords(slice, (0, 6).into(), false), 6);
+        assert_eq!(pos_at_coords(slice, (0, 6).into(), true), 5);
         assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 6);
 
-        // test with grapheme clusters
+        // Test with grapheme clusters.
         let text = Rope::from("a̐éö̲\r\n");
         let slice = text.slice(..);
         assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0);
@@ -197,15 +197,24 @@ mod test {
         assert_eq!(pos_at_coords(slice, (0, 4).into(), true), 7);
         assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 9);
 
+        // Test with wide-character grapheme clusters.
+        // TODO: account for character width.
         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(), false), 0);
-        assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 0);
+        assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 2);
+        assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 3);
+        assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 5);
+        assert_eq!(pos_at_coords(slice, (0, 3).into(), true), 5);
+
+        // Test with tabs.
+        // Todo: account for tab stops.
+        let text = Rope::from("\tHello\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), 1);
         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
     }
 }
-- 
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-core/src/movement.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<usize> {
+pub fn find_nth_next(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option<usize> {
     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<usize> {
+pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option<usize> {
     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<usize> {
+    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<usize> {
+    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-core/src/movement.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<Item = Cow<str>> + '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 84f8167fd1d54a5ebf924d6d4a43bd24cfe40dd2 Mon Sep 17 00:00:00 2001
From: Nathan Vegdahl
Date: Mon, 26 Jul 2021 23:09:58 -0700
Subject: Use `match` for branching on the `Direction` enum in more places.

---
 helix-core/src/movement.rs | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

(limited to 'helix-core/src/movement.rs')

diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs
index 4af82a1d..38e14594 100644
--- a/helix-core/src/movement.rs
+++ b/helix-core/src/movement.rs
@@ -34,10 +34,9 @@ pub fn move_horizontally(
     let pos = range.cursor(slice);
 
     // Compute the new position.
-    let new_pos = if dir == Direction::Backward {
-        nth_prev_grapheme_boundary(slice, pos, count)
-    } else {
-        nth_next_grapheme_boundary(slice, pos, count)
+    let new_pos = match dir {
+        Direction::Forward => nth_next_grapheme_boundary(slice, pos, count),
+        Direction::Backward => nth_prev_grapheme_boundary(slice, pos, count),
     };
 
     // Compute the final new range.
@@ -59,10 +58,9 @@ pub fn move_vertically(
 
     // Compute the new position.
     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_row = match dir {
+            Direction::Forward => (row + count).min(slice.len_lines().saturating_sub(1)),
+            Direction::Backward => row.saturating_sub(count),
         };
         let new_col = col.max(horiz as usize);
         (
-- 
cgit v1.2.3-70-g09d2


From aead4e69a66ba004aa34e5a1a59e05239117250e Mon Sep 17 00:00:00 2001
From: Nathan Vegdahl
Date: Mon, 26 Jul 2021 23:20:58 -0700
Subject: Minor cleanup of the vertical movement code.

---
 helix-core/src/movement.rs | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

(limited to 'helix-core/src/movement.rs')

diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs
index 38e14594..74307636 100644
--- a/helix-core/src/movement.rs
+++ b/helix-core/src/movement.rs
@@ -57,17 +57,12 @@ pub fn move_vertically(
     let horiz = range.horiz.unwrap_or(col as u32);
 
     // Compute the new position.
-    let (new_pos, new_row) = {
-        let new_row = match dir {
-            Direction::Forward => (row + count).min(slice.len_lines().saturating_sub(1)),
-            Direction::Backward => row.saturating_sub(count),
-        };
-        let new_col = col.max(horiz as usize);
-        (
-            pos_at_coords(slice, Position::new(new_row, new_col), true),
-            new_row,
-        )
+    let new_row = match dir {
+        Direction::Forward => (row + count).min(slice.len_lines().saturating_sub(1)),
+        Direction::Backward => row.saturating_sub(count),
     };
+    let new_col = col.max(horiz as usize);
+    let new_pos = pos_at_coords(slice, Position::new(new_row, new_col), true);
 
     // 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 {
-- 
cgit v1.2.3-70-g09d2