Merge pull request #376 from cessen/great_line_ending_and_cursor_range_cleanup

The Great Line Ending & Cursor Range Cleanup

helix-core/src/comment.rs

@@ -64,8 +64,10 @@ pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&st
 
     let mut min_next_line = 0;
     for selection in selection {
-        let start = text.char_to_line(selection.from()).max(min_next_line);
+        let (start, end) = selection.line_range(text);
-        let end = text.char_to_line(selection.to()) + 1;
+        let start = start.max(min_next_line).min(text.len_lines());
+        let end = (end + 1).min(text.len_lines());
+
         lines.extend(start..end);
         min_next_line = end + 1;
     }

helix-core/src/graphemes.rs

@@ -71,6 +71,8 @@ pub fn nth_prev_grapheme_boundary(slice: RopeSlice, char_idx: usize, n: usize) -
 }
 
 /// Finds the previous grapheme boundary before the given char position.
+#[must_use]
+#[inline(always)]
 pub fn prev_grapheme_boundary(slice: RopeSlice, char_idx: usize) -> usize {
     nth_prev_grapheme_boundary(slice, char_idx, 1)
 }
@@ -117,21 +119,38 @@ pub fn nth_next_grapheme_boundary(slice: RopeSlice, char_idx: usize, n: usize) -
 }
 
 /// Finds the next grapheme boundary after the given char position.
+#[must_use]
+#[inline(always)]
 pub fn next_grapheme_boundary(slice: RopeSlice, char_idx: usize) -> usize {
     nth_next_grapheme_boundary(slice, char_idx, 1)
 }
 
 /// Returns the passed char index if it's already a grapheme boundary,
 /// or the next grapheme boundary char index if not.
-pub fn ensure_grapheme_boundary(slice: RopeSlice, char_idx: usize) -> usize {
+#[must_use]
+#[inline]
+pub fn ensure_grapheme_boundary_next(slice: RopeSlice, char_idx: usize) -> usize {
     if char_idx == 0 {
-        0
+        char_idx
     } else {
         next_grapheme_boundary(slice, char_idx - 1)
     }
 }
 
+/// Returns the passed char index if it's already a grapheme boundary,
+/// or the prev grapheme boundary char index if not.
+#[must_use]
+#[inline]
+pub fn ensure_grapheme_boundary_prev(slice: RopeSlice, char_idx: usize) -> usize {
+    if char_idx == slice.len_chars() {
+        char_idx
+    } else {
+        prev_grapheme_boundary(slice, char_idx + 1)
+    }
+}
+
 /// Returns whether the given char position is a grapheme boundary.
+#[must_use]
 pub fn is_grapheme_boundary(slice: RopeSlice, char_idx: usize) -> bool {
     // Bounds check
     debug_assert!(char_idx <= slice.len_chars());

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 {

helix-core/src/match_brackets.rs

@@ -24,12 +24,13 @@ pub fn find(syntax: &Syntax, doc: &Rope, pos: usize) -> Option<usize> {
         return None;
     }
 
-    let start_byte = node.start_byte();
     let len = doc.len_bytes();
-    if start_byte >= len {
+    let start_byte = node.start_byte();
+    let end_byte = node.end_byte() - 1; // it's end exclusive
+    if start_byte >= len || end_byte >= len {
         return None;
     }
-    let end_byte = node.end_byte() - 1; // it's end exclusive
+
     let start_char = doc.byte_to_char(start_byte);
     let end_char = doc.byte_to_char(end_byte);
 

helix-core/src/object.rs

@@ -5,7 +5,7 @@ use crate::{Range, RopeSlice, Selection, Syntax};
 pub fn expand_selection(syntax: &Syntax, text: RopeSlice, selection: &Selection) -> Selection {
     let tree = syntax.tree();
 
-    selection.transform(|range| {
+    selection.clone().transform(|range| {
         let from = text.char_to_byte(range.from());
         let to = text.char_to_byte(range.to());
 

helix-core/src/position.rs

@@ -1,6 +1,7 @@
 use crate::{
     chars::char_is_line_ending,
-    graphemes::{nth_next_grapheme_boundary, RopeGraphemes},
+    graphemes::{ensure_grapheme_boundary_prev, RopeGraphemes},
+    line_ending::line_end_char_index,
     RopeSlice,
 };
 
@@ -52,19 +53,50 @@ 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 {
     let line = text.char_to_line(pos);
+
     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)).count();
+
     Position::new(line, col)
 }
 
 /// Convert (line, column) coordinates to a character index.
-pub fn pos_at_coords(text: RopeSlice, coords: Position) -> usize {
+///
+/// `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.
+///
+/// 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);
-    // line_start + col
+    let line_end = if is_1_width {
-    nth_next_grapheme_boundary(text, line_start, col)
+        line_end_char_index(&text, row)
+    } else {
+        text.line_to_char((row + 1).min(text.len_lines()))
+    };
+
+    let mut col_char_offset = 0;
+    for (i, g) in RopeGraphemes::new(text.slice(line_start..line_end)).enumerate() {
+        if i == col {
+            break;
+        }
+        col_char_offset += g.chars().count();
+    }
+
+    line_start + col_char_offset
 }
 
 #[cfg(test)]
@@ -80,55 +112,109 @@ mod test {
 
     #[test]
     fn test_coords_at_pos() {
-        // let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ");
+        let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ");
-        // let slice = text.slice(..);
+        let slice = text.slice(..);
-        // assert_eq!(coords_at_pos(slice, 0), (0, 0).into());
+        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, 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, 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, 7), (1, 1).into()); // position on o
-        // assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d
+        assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d
-
+
-        // test with grapheme clusters
+        // 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, 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.
         let text = Rope::from("a̐éö̲\r\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, 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("किमपि");
+        // 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, 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]
     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, 0).into(), false), 0);
-        assert_eq!(pos_at_coords(slice, (0, 5).into()), 5); // position on \n
+        assert_eq!(pos_at_coords(slice, (0, 5).into(), false), 5); // position on \n
-        assert_eq!(pos_at_coords(slice, (1, 0).into()), 6); // position on w
+        assert_eq!(pos_at_coords(slice, (0, 6).into(), false), 6); // position after \n
-        assert_eq!(pos_at_coords(slice, (1, 1).into()), 7); // position on o
+        assert_eq!(pos_at_coords(slice, (0, 6).into(), true), 5); // position after \n
-        assert_eq!(pos_at_coords(slice, (1, 4).into()), 10); // position on d
+        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 grapheme clusters
+        // 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), 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.
         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, 0).into(), false), 0);
-        assert_eq!(pos_at_coords(slice, (0, 1).into()), 2);
+        assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 2);
-        assert_eq!(pos_at_coords(slice, (0, 2).into()), 4);
+        assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 4);
-        assert_eq!(pos_at_coords(slice, (0, 3).into()), 7); // \r\n is one char here
+        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()), 9);
+        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);
+
+        // 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()), 0);
+        assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0);
-        assert_eq!(pos_at_coords(slice, (0, 1).into()), 2);
+        assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 2);
-        assert_eq!(pos_at_coords(slice, (0, 2).into()), 3);
+        assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 3);
-        assert_eq!(pos_at_coords(slice, (0, 3).into()), 5); // eol
+        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);
     }
 }

helix-core/src/search.rs

@@ -1,18 +1,11 @@
 use crate::RopeSlice;
 
-pub fn find_nth_next(
+pub fn find_nth_next(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option<usize> {
-    text: RopeSlice,
+    if pos >= text.len_chars() || n == 0 {
-    ch: char,
-    mut pos: usize,
-    n: usize,
-    inclusive: bool,
-) -> Option<usize> {
-    if pos >= text.len_chars() {
         return None;
     }
 
-    // start searching right after pos
+    let mut chars = text.chars_at(pos);
-    let mut chars = text.chars_at(pos + 1);
 
     for _ in 0..n {
         loop {
@@ -26,28 +19,21 @@ pub fn find_nth_next(
         }
     }
 
-    if !inclusive {
+    Some(pos - 1)
-        pos -= 1;
-    }
-
-    Some(pos)
 }
 
-pub fn find_nth_prev(
+pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option<usize> {
-    text: RopeSlice,
+    if pos == 0 || n == 0 {
-    ch: char,
+        return None;
-    mut pos: usize,
+    }
-    n: usize,
+
-    inclusive: bool,
-) -> Option<usize> {
-    // start searching right before pos
     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;
@@ -55,9 +41,5 @@ pub fn find_nth_prev(
         }
     }
 
-    if !inclusive {
-        pos += 1;
-    }
-
     Some(pos)
 }

helix-core/src/selection.rs

@@ -1,30 +1,59 @@
-//! Selections are the primary editing construct. Even a single cursor is defined as an empty
+//! Selections are the primary editing construct. Even cursors are
-//! single selection range.
+//! defined as a selection range.
 //!
 //! All positioning is done via `char` offsets into the buffer.
-use crate::{Assoc, ChangeSet, RopeSlice};
+use crate::{
+    graphemes::{
+        ensure_grapheme_boundary_next, ensure_grapheme_boundary_prev, next_grapheme_boundary,
+        prev_grapheme_boundary,
+    },
+    Assoc, ChangeSet, RopeSlice,
+};
 use smallvec::{smallvec, SmallVec};
 use std::borrow::Cow;
 
-#[inline]
+/// A single selection range.
-fn abs_difference(x: usize, y: usize) -> usize {
+///
-    if x < y {
+/// A range consists of an "anchor" and "head" position in
-        y - x
+/// the text.  The head is the part that the user moves when
-    } else {
+/// directly extending a selection.  The head and anchor
-        x - y
+/// can be in any order, or even share the same position.
-    }
+///
-}
+/// The anchor and head positions use gap indexing, meaning
-
+/// that their indices represent the the gaps *between* `char`s
-/// A single selection range. Anchor-inclusive, head-exclusive.
+/// rather than the `char`s themselves. For example, 1
+/// represents the position between the first and second `char`.
+///
+/// Below are some example `Range` configurations to better
+/// illustrate.  The anchor and head indices are show as
+/// "(anchor, head)", followed by example text with "[" and "]"
+/// inserted to represent the anchor and head positions:
+///
+/// - (0, 3): [Som]e text.
+/// - (3, 0): ]Som[e text.
+/// - (2, 7): So[me te]xt.
+/// - (1, 1): S[]ome text.
+///
+/// Ranges are considered to be inclusive on the left and
+/// exclusive on the right, regardless of anchor-head ordering.
+/// This means, for example, that non-zero-width ranges that
+/// are directly adjecent, sharing an edge, do not overlap.
+/// However, a zero-width range will overlap with the shared
+/// left-edge of another range.
+///
+/// By convention, user-facing ranges are considered to have
+/// a block cursor on the head-side of the range that spans a
+/// single grapheme inward from the range's edge.  There are a
+/// variety of helper methods on `Range` for working in terms of
+/// that block cursor, all of which have `cursor` in their name.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct Range {
-    // TODO: optimize into u32
     /// The anchor of the range: the side that doesn't move when extending.
     pub anchor: usize,
     /// The head of the range, moved when extending.
     pub head: usize,
     pub horiz: Option<u32>,
-} // TODO: might be cheaper to store normalized as from/to and an inverted flag
+}
 
 impl Range {
     pub fn new(anchor: usize, head: usize) -> Self {
@@ -53,6 +82,20 @@ impl Range {
         std::cmp::max(self.anchor, self.head)
     }
 
+    /// The (inclusive) range of lines that the range overlaps.
+    #[inline]
+    #[must_use]
+    pub fn line_range(&self, text: RopeSlice) -> (usize, usize) {
+        let from = self.from();
+        let to = if self.is_empty() {
+            self.to()
+        } else {
+            prev_grapheme_boundary(text, self.to()).max(from)
+        };
+
+        (text.char_to_line(from), text.char_to_line(to))
+    }
+
     /// `true` when head and anchor are at the same position.
     #[inline]
     pub fn is_empty(&self) -> bool {
@@ -62,37 +105,39 @@ impl Range {
     /// Check two ranges for overlap.
     #[must_use]
     pub fn overlaps(&self, other: &Self) -> bool {
-        // cursor overlap is checked differently
+        // To my eye, it's non-obvious why this works, but I arrived
-        if self.is_empty() {
+        // at it after transforming the slower version that explicitly
-            let pos = self.head;
+        // enumerated more cases.  The unit tests are thorough.
-            pos >= other.from() && other.to() >= pos
+        self.from() == other.from() || (self.to() > other.from() && other.to() > self.from())
-        } else {
-            self.to() > other.from() && other.to() > self.from()
-        }
     }
 
     pub fn contains(&self, pos: usize) -> bool {
-        if self.is_empty() {
+        self.from() <= pos && pos < self.to()
-            return false;
-        }
-
-        if self.anchor < self.head {
-            self.anchor <= pos && pos < self.head
-        } else {
-            self.head < pos && pos <= self.anchor
-        }
     }
 
     /// Map a range through a set of changes. Returns a new range representing the same position
     /// after the changes are applied.
     pub fn map(self, changes: &ChangeSet) -> Self {
-        let anchor = changes.map_pos(self.anchor, Assoc::After);
+        use std::cmp::Ordering;
-        let head = changes.map_pos(self.head, Assoc::After);
+        let (anchor, head) = match self.anchor.cmp(&self.head) {
-
+            Ordering::Equal => (
-        // TODO: possibly unnecessary
+                changes.map_pos(self.anchor, Assoc::After),
-        if self.anchor == anchor && self.head == head {
+                changes.map_pos(self.head, Assoc::After),
-            return self;
+            ),
-        }
+            Ordering::Less => (
+                changes.map_pos(self.anchor, Assoc::After),
+                changes.map_pos(self.head, Assoc::Before),
+            ),
+            Ordering::Greater => (
+                changes.map_pos(self.anchor, Assoc::Before),
+                changes.map_pos(self.head, Assoc::After),
+            ),
+        };
+
+        // We want to return a new `Range` with `horiz == None` every time,
+        // even if the anchor and head haven't changed, because we don't
+        // know if the *visual* position hasn't changed due to
+        // character-width or grapheme changes earlier in the text.
         Self {
             anchor,
             head,
@@ -103,22 +148,41 @@ impl Range {
     /// Extend the range to cover at least `from` `to`.
     #[must_use]
     pub fn extend(&self, from: usize, to: usize) -> Self {
-        if from <= self.anchor && to >= self.anchor {
+        debug_assert!(from <= to);
-            return Self {
+
-                anchor: from,
+        if self.anchor <= self.head {
-                head: to,
+            Self {
+                anchor: self.anchor.min(from),
+                head: self.head.max(to),
                 horiz: None,
-            };
+            }
+        } else {
+            Self {
+                anchor: self.anchor.max(to),
+                head: self.head.min(from),
+                horiz: None,
+            }
         }
+    }
 
-        Self {
+    /// Returns a range that encompasses both input ranges.
-            anchor: self.anchor,
+    ///
-            head: if abs_difference(from, self.anchor) > abs_difference(to, self.anchor) {
+    /// This is like `extend()`, but tries to negotiate the
-                from
+    /// anchor/head ordering between the two input ranges.
-            } else {
+    #[must_use]
-                to
+    pub fn merge(&self, other: Self) -> Self {
-            },
+        if self.anchor > self.head && other.anchor > other.head {
-            horiz: None,
+            Range {
+                anchor: self.anchor.max(other.anchor),
+                head: self.head.min(other.head),
+                horiz: None,
+            }
+        } else {
+            Range {
+                anchor: self.from().min(other.from()),
+                head: self.to().max(other.to()),
+                horiz: None,
+            }
         }
     }
 
@@ -126,7 +190,120 @@ impl Range {
 
     #[inline]
     pub fn fragment<'a, 'b: 'a>(&'a self, text: RopeSlice<'b>) -> Cow<'b, str> {
-        Cow::from(text.slice(self.from()..self.to() + 1))
+        text.slice(self.from()..self.to()).into()
+    }
+
+    //--------------------------------
+    // Alignment methods.
+
+    /// Compute a possibly new range from this range, with its ends
+    /// shifted as needed to align with grapheme boundaries.
+    ///
+    /// Zero-width ranges will always stay zero-width, and non-zero-width
+    /// ranges will never collapse to zero-width.
+    #[must_use]
+    pub fn grapheme_aligned(&self, slice: RopeSlice) -> Self {
+        use std::cmp::Ordering;
+        let (new_anchor, new_head) = match self.anchor.cmp(&self.head) {
+            Ordering::Equal => {
+                let pos = ensure_grapheme_boundary_prev(slice, self.anchor);
+                (pos, pos)
+            }
+            Ordering::Less => (
+                ensure_grapheme_boundary_prev(slice, self.anchor),
+                ensure_grapheme_boundary_next(slice, self.head),
+            ),
+            Ordering::Greater => (
+                ensure_grapheme_boundary_next(slice, self.anchor),
+                ensure_grapheme_boundary_prev(slice, self.head),
+            ),
+        };
+        Range {
+            anchor: new_anchor,
+            head: new_head,
+            horiz: if new_anchor == self.anchor {
+                self.horiz
+            } else {
+                None
+            },
+        }
+    }
+
+    /// Compute a possibly new range from this range, attempting to ensure
+    /// a minimum range width of 1 char by shifting the head in the forward
+    /// direction as needed.
+    ///
+    /// This method will never shift the anchor, and will only shift the
+    /// head in the forward direction.  Therefore, this method can fail
+    /// at ensuring the minimum width if and only if the passed range is
+    /// both zero-width and at the end of the `RopeSlice`.
+    ///
+    /// If the input range is grapheme-boundary aligned, the returned range
+    /// will also be.  Specifically, if the head needs to shift to achieve
+    /// the minimum width, it will shift to the next grapheme boundary.
+    #[must_use]
+    #[inline]
+    pub fn min_width_1(&self, slice: RopeSlice) -> Self {
+        if self.anchor == self.head {
+            Range {
+                anchor: self.anchor,
+                head: next_grapheme_boundary(slice, self.head),
+                horiz: self.horiz,
+            }
+        } else {
+            *self
+        }
+    }
+
+    //--------------------------------
+    // Block-cursor methods.
+
+    /// 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.
+    ///
+    /// 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 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 anchor <= char_idx {
+                Range::new(anchor, next_grapheme_boundary(text, char_idx))
+            } else {
+                Range::new(anchor, char_idx)
+            }
+        } else {
+            Range::point(char_idx)
+        }
+    }
+
+    /// The line number that the block-cursor is on.
+    #[inline]
+    #[must_use]
+    pub fn cursor_line(&self, text: RopeSlice) -> usize {
+        text.char_to_line(self.cursor(text))
     }
 }
 
@@ -157,11 +334,6 @@ impl Selection {
         self.ranges[self.primary_index]
     }
 
-    #[must_use]
-    pub fn cursor(&self) -> usize {
-        self.primary().head
-    }
-
     /// Ensure selection containing only the primary selection.
     pub fn into_single(self) -> Self {
         if self.ranges.len() == 1 {
@@ -174,13 +346,12 @@ impl Selection {
         }
     }
 
+    /// Adds a new range to the selection and makes it the primary range.
     pub fn push(mut self, range: Range) -> Self {
-        let index = self.ranges.len();
         self.ranges.push(range);
-
+        self.set_primary_index(self.ranges().len() - 1);
-        Self::normalize(self.ranges, index)
+        self.normalize()
     }
-    // replace_range
 
     /// Map selections over a set of changes. Useful for adjusting the selection position after
     /// applying changes to a document.
@@ -206,6 +377,11 @@ impl Selection {
         self.primary_index
     }
 
+    pub fn set_primary_index(&mut self, idx: usize) {
+        assert!(idx < self.ranges.len());
+        self.primary_index = idx;
+    }
+
     #[must_use]
     /// Constructs a selection holding a single range.
     pub fn single(anchor: usize, head: usize) -> Self {
@@ -224,80 +400,79 @@ impl Selection {
         Self::single(pos, pos)
     }
 
-    fn normalize(mut ranges: SmallVec<[Range; 1]>, mut primary_index: usize) -> Self {
+    /// Normalizes a `Selection`.
-        let primary = ranges[primary_index];
+    fn normalize(mut self) -> Self {
-        ranges.sort_unstable_by_key(Range::from);
+        let primary = self.ranges[self.primary_index];
-        primary_index = ranges.iter().position(|&range| range == primary).unwrap();
+        self.ranges.sort_unstable_by_key(Range::from);
-
+        self.primary_index = self
-        let mut result = SmallVec::with_capacity(ranges.len()); // approx
+            .ranges
-
+            .iter()
-        // TODO: we could do with one vec by removing elements as we mutate
+            .position(|&range| range == primary)
-
+            .unwrap();
-        let mut i = 0;
+
-
+        let mut prev_i = 0;
-        for range in ranges.into_iter() {
+        for i in 1..self.ranges.len() {
-            // if previous value exists
+            if self.ranges[prev_i].overlaps(&self.ranges[i]) {
-            if let Some(prev) = result.last_mut() {
+                self.ranges[prev_i] = self.ranges[prev_i].merge(self.ranges[i]);
-                // and we overlap it
+            } else {
-
+                prev_i += 1;
-                // TODO: we used to simply check range.from() <(=) prev.to()
+                self.ranges[prev_i] = self.ranges[i];
-                // avoiding two comparisons
+            }
-                if range.overlaps(prev) {
+            if i == self.primary_index {
-                    let from = prev.from();
+                self.primary_index = prev_i;
-                    let to = std::cmp::max(range.to(), prev.to());
-
-                    if i <= primary_index {
-                        primary_index -= 1
-                    }
-
-                    // merge into previous
-                    if range.anchor > range.head {
-                        prev.anchor = to;
-                        prev.head = from;
-                    } else {
-                        prev.anchor = from;
-                        prev.head = to;
-                    }
-                    continue;
-                }
             }
-
-            result.push(range);
-            i += 1
         }
 
-        Self {
+        self.ranges.truncate(prev_i + 1);
-            ranges: result,
+
-            primary_index,
+        self
-        }
     }
 
     // TODO: consume an iterator or a vec to reduce allocations?
     #[must_use]
     pub fn new(ranges: SmallVec<[Range; 1]>, primary_index: usize) -> Self {
         assert!(!ranges.is_empty());
+        debug_assert!(primary_index < ranges.len());
 
-        // fast path for a single selection (cursor)
+        let mut selection = Self {
-        if ranges.len() == 1 {
+            ranges,
-            return Self {
+            primary_index,
-                ranges,
+        };
-                primary_index: 0,
+
-            };
+        if selection.ranges.len() > 1 {
+            // TODO: only normalize if needed (any ranges out of order)
+            selection = selection.normalize();
         }
 
-        // TODO: only normalize if needed (any ranges out of order)
+        selection
-        Self::normalize(ranges, primary_index)
     }
 
-    /// Takes a closure and maps each selection over the closure.
+    /// Takes a closure and maps each `Range` over the closure.
-    pub fn transform<F>(&self, f: F) -> Self
+    pub fn transform<F>(mut self, f: F) -> Self
     where
         F: Fn(Range) -> Range,
     {
-        Self::new(
+        for range in self.ranges.iter_mut() {
-            self.ranges.iter().copied().map(f).collect(),
+            *range = f(*range)
-            self.primary_index,
+        }
-        )
+        self.normalize()
+    }
+
+    // Ensures the selection adheres to the following invariants:
+    // 1. All ranges are grapheme aligned.
+    // 2. All ranges are at least 1 character wide, unless at the
+    //    very end of the document.
+    // 3. Ranges are non-overlapping.
+    // 4. Ranges are sorted by their position in the text.
+    pub fn ensure_invariants(self, text: RopeSlice) -> Self {
+        self.transform(|r| r.min_width_1(text).grapheme_aligned(text))
+            .normalize()
+    }
+
+    /// Transforms the selection into all of the left-side head positions,
+    /// using block-cursor semantics.
+    pub fn cursors(self, text: RopeSlice) -> Self {
+        self.transform(|range| Range::point(range.cursor(text)))
     }
 
     pub fn fragments<'a>(&'a self, text: RopeSlice<'a>) -> impl Iterator<Item = Cow<str>> + 'a {
@@ -363,7 +538,7 @@ pub fn select_on_matches(
 
             let start = text.byte_to_char(start_byte + mat.start());
             let end = text.byte_to_char(start_byte + mat.end());
-            result.push(Range::new(start, end.saturating_sub(1)));
+            result.push(Range::new(start, end));
         }
     }
 
@@ -384,6 +559,12 @@ pub fn split_on_matches(
     let mut result = SmallVec::with_capacity(selection.len());
 
     for sel in selection {
+        // Special case: zero-width selection.
+        if sel.from() == sel.to() {
+            result.push(*sel);
+            continue;
+        }
+
         // TODO: can't avoid occasional allocations since Regex can't operate on chunks yet
         let fragment = sel.fragment(text);
 
@@ -396,13 +577,12 @@ pub fn split_on_matches(
 
         for mat in regex.find_iter(&fragment) {
             // TODO: retain range direction
-
             let end = text.byte_to_char(start_byte + mat.start());
-            result.push(Range::new(start, end.saturating_sub(1)));
+            result.push(Range::new(start, end));
             start = text.byte_to_char(start_byte + mat.end());
         }
 
-        if start <= sel_end {
+        if start < sel_end {
             result.push(Range::new(start, sel_end));
         }
     }
@@ -484,7 +664,7 @@ mod test {
             .collect::<Vec<String>>()
             .join(",");
 
-        assert_eq!(res, "8/10,10/12");
+        assert_eq!(res, "8/10,10/12,12/12");
     }
 
     #[test]
@@ -498,35 +678,251 @@ mod test {
         assert_eq!(range.contains(13), false);
 
         let range = Range::new(9, 6);
-        assert_eq!(range.contains(9), true);
+        assert_eq!(range.contains(9), false);
         assert_eq!(range.contains(7), true);
-        assert_eq!(range.contains(6), false);
+        assert_eq!(range.contains(6), true);
+    }
+
+    #[test]
+    fn test_overlaps() {
+        fn overlaps(a: (usize, usize), b: (usize, usize)) -> bool {
+            Range::new(a.0, a.1).overlaps(&Range::new(b.0, b.1))
+        }
+
+        // Two non-zero-width ranges, no overlap.
+        assert!(!overlaps((0, 3), (3, 6)));
+        assert!(!overlaps((0, 3), (6, 3)));
+        assert!(!overlaps((3, 0), (3, 6)));
+        assert!(!overlaps((3, 0), (6, 3)));
+        assert!(!overlaps((3, 6), (0, 3)));
+        assert!(!overlaps((3, 6), (3, 0)));
+        assert!(!overlaps((6, 3), (0, 3)));
+        assert!(!overlaps((6, 3), (3, 0)));
+
+        // Two non-zero-width ranges, overlap.
+        assert!(overlaps((0, 4), (3, 6)));
+        assert!(overlaps((0, 4), (6, 3)));
+        assert!(overlaps((4, 0), (3, 6)));
+        assert!(overlaps((4, 0), (6, 3)));
+        assert!(overlaps((3, 6), (0, 4)));
+        assert!(overlaps((3, 6), (4, 0)));
+        assert!(overlaps((6, 3), (0, 4)));
+        assert!(overlaps((6, 3), (4, 0)));
+
+        // Zero-width and non-zero-width range, no overlap.
+        assert!(!overlaps((0, 3), (3, 3)));
+        assert!(!overlaps((3, 0), (3, 3)));
+        assert!(!overlaps((3, 3), (0, 3)));
+        assert!(!overlaps((3, 3), (3, 0)));
+
+        // Zero-width and non-zero-width range, overlap.
+        assert!(overlaps((1, 4), (1, 1)));
+        assert!(overlaps((4, 1), (1, 1)));
+        assert!(overlaps((1, 1), (1, 4)));
+        assert!(overlaps((1, 1), (4, 1)));
+
+        assert!(overlaps((1, 4), (3, 3)));
+        assert!(overlaps((4, 1), (3, 3)));
+        assert!(overlaps((3, 3), (1, 4)));
+        assert!(overlaps((3, 3), (4, 1)));
+
+        // Two zero-width ranges, no overlap.
+        assert!(!overlaps((0, 0), (1, 1)));
+        assert!(!overlaps((1, 1), (0, 0)));
+
+        // Two zero-width ranges, overlap.
+        assert!(overlaps((1, 1), (1, 1)));
+    }
+
+    #[test]
+    fn test_graphem_aligned() {
+        let r = Rope::from_str("\r\nHi\r\n");
+        let s = r.slice(..);
+
+        // Zero-width.
+        assert_eq!(Range::new(0, 0).grapheme_aligned(s), Range::new(0, 0));
+        assert_eq!(Range::new(1, 1).grapheme_aligned(s), Range::new(0, 0));
+        assert_eq!(Range::new(2, 2).grapheme_aligned(s), Range::new(2, 2));
+        assert_eq!(Range::new(3, 3).grapheme_aligned(s), Range::new(3, 3));
+        assert_eq!(Range::new(4, 4).grapheme_aligned(s), Range::new(4, 4));
+        assert_eq!(Range::new(5, 5).grapheme_aligned(s), Range::new(4, 4));
+        assert_eq!(Range::new(6, 6).grapheme_aligned(s), Range::new(6, 6));
+
+        // Forward.
+        assert_eq!(Range::new(0, 1).grapheme_aligned(s), Range::new(0, 2));
+        assert_eq!(Range::new(1, 2).grapheme_aligned(s), Range::new(0, 2));
+        assert_eq!(Range::new(2, 3).grapheme_aligned(s), Range::new(2, 3));
+        assert_eq!(Range::new(3, 4).grapheme_aligned(s), Range::new(3, 4));
+        assert_eq!(Range::new(4, 5).grapheme_aligned(s), Range::new(4, 6));
+        assert_eq!(Range::new(5, 6).grapheme_aligned(s), Range::new(4, 6));
+
+        assert_eq!(Range::new(0, 2).grapheme_aligned(s), Range::new(0, 2));
+        assert_eq!(Range::new(1, 3).grapheme_aligned(s), Range::new(0, 3));
+        assert_eq!(Range::new(2, 4).grapheme_aligned(s), Range::new(2, 4));
+        assert_eq!(Range::new(3, 5).grapheme_aligned(s), Range::new(3, 6));
+        assert_eq!(Range::new(4, 6).grapheme_aligned(s), Range::new(4, 6));
+
+        // Reverse.
+        assert_eq!(Range::new(1, 0).grapheme_aligned(s), Range::new(2, 0));
+        assert_eq!(Range::new(2, 1).grapheme_aligned(s), Range::new(2, 0));
+        assert_eq!(Range::new(3, 2).grapheme_aligned(s), Range::new(3, 2));
+        assert_eq!(Range::new(4, 3).grapheme_aligned(s), Range::new(4, 3));
+        assert_eq!(Range::new(5, 4).grapheme_aligned(s), Range::new(6, 4));
+        assert_eq!(Range::new(6, 5).grapheme_aligned(s), Range::new(6, 4));
+
+        assert_eq!(Range::new(2, 0).grapheme_aligned(s), Range::new(2, 0));
+        assert_eq!(Range::new(3, 1).grapheme_aligned(s), Range::new(3, 0));
+        assert_eq!(Range::new(4, 2).grapheme_aligned(s), Range::new(4, 2));
+        assert_eq!(Range::new(5, 3).grapheme_aligned(s), Range::new(6, 3));
+        assert_eq!(Range::new(6, 4).grapheme_aligned(s), Range::new(6, 4));
+    }
+
+    #[test]
+    fn test_min_width_1() {
+        let r = Rope::from_str("\r\nHi\r\n");
+        let s = r.slice(..);
+
+        // Zero-width.
+        assert_eq!(Range::new(0, 0).min_width_1(s), Range::new(0, 2));
+        assert_eq!(Range::new(1, 1).min_width_1(s), Range::new(1, 2));
+        assert_eq!(Range::new(2, 2).min_width_1(s), Range::new(2, 3));
+        assert_eq!(Range::new(3, 3).min_width_1(s), Range::new(3, 4));
+        assert_eq!(Range::new(4, 4).min_width_1(s), Range::new(4, 6));
+        assert_eq!(Range::new(5, 5).min_width_1(s), Range::new(5, 6));
+        assert_eq!(Range::new(6, 6).min_width_1(s), Range::new(6, 6));
+
+        // Forward.
+        assert_eq!(Range::new(0, 1).min_width_1(s), Range::new(0, 1));
+        assert_eq!(Range::new(1, 2).min_width_1(s), Range::new(1, 2));
+        assert_eq!(Range::new(2, 3).min_width_1(s), Range::new(2, 3));
+        assert_eq!(Range::new(3, 4).min_width_1(s), Range::new(3, 4));
+        assert_eq!(Range::new(4, 5).min_width_1(s), Range::new(4, 5));
+        assert_eq!(Range::new(5, 6).min_width_1(s), Range::new(5, 6));
+
+        // Reverse.
+        assert_eq!(Range::new(1, 0).min_width_1(s), Range::new(1, 0));
+        assert_eq!(Range::new(2, 1).min_width_1(s), Range::new(2, 1));
+        assert_eq!(Range::new(3, 2).min_width_1(s), Range::new(3, 2));
+        assert_eq!(Range::new(4, 3).min_width_1(s), Range::new(4, 3));
+        assert_eq!(Range::new(5, 4).min_width_1(s), Range::new(5, 4));
+        assert_eq!(Range::new(6, 5).min_width_1(s), Range::new(6, 5));
+    }
+
+    #[test]
+    fn test_line_range() {
+        let r = Rope::from_str("\r\nHi\r\nthere!");
+        let s = r.slice(..);
+
+        // Zero-width ranges.
+        assert_eq!(Range::new(0, 0).line_range(s), (0, 0));
+        assert_eq!(Range::new(1, 1).line_range(s), (0, 0));
+        assert_eq!(Range::new(2, 2).line_range(s), (1, 1));
+        assert_eq!(Range::new(3, 3).line_range(s), (1, 1));
+
+        // Forward ranges.
+        assert_eq!(Range::new(0, 1).line_range(s), (0, 0));
+        assert_eq!(Range::new(0, 2).line_range(s), (0, 0));
+        assert_eq!(Range::new(0, 3).line_range(s), (0, 1));
+        assert_eq!(Range::new(1, 2).line_range(s), (0, 0));
+        assert_eq!(Range::new(2, 3).line_range(s), (1, 1));
+        assert_eq!(Range::new(3, 8).line_range(s), (1, 2));
+        assert_eq!(Range::new(0, 12).line_range(s), (0, 2));
+
+        // Reverse ranges.
+        assert_eq!(Range::new(1, 0).line_range(s), (0, 0));
+        assert_eq!(Range::new(2, 0).line_range(s), (0, 0));
+        assert_eq!(Range::new(3, 0).line_range(s), (0, 1));
+        assert_eq!(Range::new(2, 1).line_range(s), (0, 0));
+        assert_eq!(Range::new(3, 2).line_range(s), (1, 1));
+        assert_eq!(Range::new(8, 3).line_range(s), (1, 2));
+        assert_eq!(Range::new(12, 0).line_range(s), (0, 2));
+    }
+
+    #[test]
+    fn test_cursor() {
+        let r = Rope::from_str("\r\nHi\r\nthere!");
+        let s = r.slice(..);
+
+        // Zero-width ranges.
+        assert_eq!(Range::new(0, 0).cursor(s), 0);
+        assert_eq!(Range::new(2, 2).cursor(s), 2);
+        assert_eq!(Range::new(3, 3).cursor(s), 3);
+
+        // Forward ranges.
+        assert_eq!(Range::new(0, 2).cursor(s), 0);
+        assert_eq!(Range::new(0, 3).cursor(s), 2);
+        assert_eq!(Range::new(3, 6).cursor(s), 4);
+
+        // Reverse ranges.
+        assert_eq!(Range::new(2, 0).cursor(s), 0);
+        assert_eq!(Range::new(6, 2).cursor(s), 2);
+        assert_eq!(Range::new(6, 3).cursor(s), 3);
+    }
+
+    #[test]
+    fn test_put_cursor() {
+        let r = Rope::from_str("\r\nHi\r\nthere!");
+        let s = r.slice(..);
+
+        // Zero-width ranges.
+        assert_eq!(Range::new(0, 0).put_cursor(s, 0, true), Range::new(0, 2));
+        assert_eq!(Range::new(0, 0).put_cursor(s, 2, true), Range::new(0, 3));
+        assert_eq!(Range::new(2, 3).put_cursor(s, 4, true), Range::new(2, 6));
+        assert_eq!(Range::new(2, 8).put_cursor(s, 4, true), Range::new(2, 6));
+        assert_eq!(Range::new(8, 8).put_cursor(s, 4, true), Range::new(9, 4));
+
+        // Forward ranges.
+        assert_eq!(Range::new(3, 6).put_cursor(s, 0, true), Range::new(4, 0));
+        assert_eq!(Range::new(3, 6).put_cursor(s, 2, true), Range::new(4, 2));
+        assert_eq!(Range::new(3, 6).put_cursor(s, 3, true), Range::new(3, 4));
+        assert_eq!(Range::new(3, 6).put_cursor(s, 4, true), Range::new(3, 6));
+        assert_eq!(Range::new(3, 6).put_cursor(s, 6, true), Range::new(3, 7));
+        assert_eq!(Range::new(3, 6).put_cursor(s, 8, true), Range::new(3, 9));
+
+        // Reverse ranges.
+        assert_eq!(Range::new(6, 3).put_cursor(s, 0, true), Range::new(6, 0));
+        assert_eq!(Range::new(6, 3).put_cursor(s, 2, true), Range::new(6, 2));
+        assert_eq!(Range::new(6, 3).put_cursor(s, 3, true), Range::new(6, 3));
+        assert_eq!(Range::new(6, 3).put_cursor(s, 4, true), Range::new(6, 4));
+        assert_eq!(Range::new(6, 3).put_cursor(s, 6, true), Range::new(4, 7));
+        assert_eq!(Range::new(6, 3).put_cursor(s, 8, true), Range::new(4, 9));
     }
 
     #[test]
     fn test_split_on_matches() {
         use crate::regex::Regex;
 
-        let text = Rope::from("abcd efg wrs   xyz 123 456");
+        let text = Rope::from(" abcd efg wrs   xyz 123 456");
 
-        let selection = Selection::new(smallvec![Range::new(0, 8), Range::new(10, 19),], 0);
+        let selection = Selection::new(smallvec![Range::new(0, 9), Range::new(11, 20),], 0);
 
         let result = split_on_matches(text.slice(..), &selection, &Regex::new(r"\s+").unwrap());
 
         assert_eq!(
             result.ranges(),
             &[
-                Range::new(0, 3),
+                // TODO: rather than this behavior, maybe we want it
-                Range::new(5, 7),
+                // to be based on which side is the anchor?
-                Range::new(10, 11),
+                //
-                Range::new(15, 17),
+                // We get a leading zero-width range when there's
-                Range::new(19, 19),
+                // a leading match because ranges are inclusive on
+                // the left.  Imagine, for example, if the entire
+                // selection range were matched: you'd still want
+                // at least one range to remain after the split.
+                Range::new(0, 0),
+                Range::new(1, 5),
+                Range::new(6, 9),
+                Range::new(11, 13),
+                Range::new(16, 19),
+                // In contrast to the comment above, there is no
+                // _trailing_ zero-width range despite the trailing
+                // match, because ranges are exclusive on the right.
             ]
         );
 
         assert_eq!(
             result.fragments(text.slice(..)).collect::<Vec<_>>(),
-            &["abcd", "efg", "rs", "xyz", "1"]
+            &["", "abcd", "efg", "rs", "xyz"]
         );
     }
 }

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,34 @@ pub fn find_nth_pairs_pos(
 ) -> Option<(usize, usize)> {
     let (open, close) = get_pair(ch);
 
-    let (open_pos, close_pos) = if open == close {
+    if text.len_chars() < 2 || pos >= text.len_chars() {
-        let prev = search::find_nth_prev(text, open, pos, n, true);
+        return None;
-        let next = search::find_nth_next(text, close, pos, n, true);
+    }
-        if text.char(pos) == open {
+
-            // cursor is *on* a pair
+    if open == close {
-            next.map(|n| (pos, n)).or_else(|| prev.map(|p| (p, 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)? + 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 {
-            (prev?, next?)
+            Some((
+                search::find_nth_prev(text, open, pos, n)?,
+                search::find_nth_next(text, close, pos, n)? + 1,
+            ))
         }
     } else {
-        (
+        Some((
             find_nth_open_pair(text, open, close, pos, n)?,
-            find_nth_close_pair(text, open, close, pos, n)?,
+            next_grapheme_boundary(text, find_nth_close_pair(text, open, close, pos, n)?),
-        )
+        ))
-    };
+    }
-
-    Some((open_pos, close_pos))
 }
 
 fn find_nth_open_pair(
@@ -173,12 +185,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, 11)));
-        assert_eq!(find_nth_pairs_pos(slice, ')', 6, 1), Some((5, 10)));
+        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 +200,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((10, 16)));
-        assert_eq!(find_nth_pairs_pos(slice, '(', 13, 2), Some((4, 21)));
+        assert_eq!(find_nth_pairs_pos(slice, '(', 13, 2), Some((4, 22)));
-        assert_eq!(find_nth_pairs_pos(slice, '(', 13, 3), Some((0, 27)));
+        assert_eq!(find_nth_pairs_pos(slice, '(', 13, 3), Some((0, 28)));
     }
 
     #[test]
@@ -198,14 +211,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, 1), Some((10, 16)));
-        assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 2), Some((4, 21)));
+        assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 2), Some((4, 22)));
-        assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 3), Some((0, 27)));
+        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, '\'', 0, 1), Some((0, 5)));
-        assert_eq!(find_nth_pairs_pos(slice, '\'', 27, 1), Some((21, 27)));
+        assert_eq!(find_nth_pairs_pos(slice, '\'', 27, 1), Some((21, 28)));
     }
 
     #[test]
@@ -214,8 +227,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, 1), Some((5, 25)));
-        assert_eq!(find_nth_pairs_pos(slice, '(', 15, 2), Some((0, 31)));
+        assert_eq!(find_nth_pairs_pos(slice, '(', 15, 2), Some((0, 32)));
     }
 
     #[test]
@@ -224,9 +237,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((10, 16)));
-        assert_eq!(find_nth_pairs_pos(slice, '[', 13, 1), Some((4, 21)));
+        assert_eq!(find_nth_pairs_pos(slice, '[', 13, 1), Some((4, 22)));
-        assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((0, 27)));
+        assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((0, 28)));
     }
 
     #[test]
@@ -243,7 +256,7 @@ mod test {
             get_surround_pos(slice, &selection, '(', 1)
                 .unwrap()
                 .as_slice(),
-            &[0, 5, 7, 13, 15, 23]
+            &[0, 6, 7, 14, 15, 24]
         );
     }
 

helix-core/src/syntax.rs

@@ -1760,10 +1760,20 @@ impl<I: Iterator<Item = HighlightEvent>> Iterator for Merge<I> {
                 self.next_event = self.iter.next();
                 Some(event)
             }
-            // can happen if deleting and cursor at EOF, and diagnostic reaches past the end
+            // Can happen if cursor at EOF and/or diagnostic reaches past the end.
-            (None, Some((_, _))) => {
+            // We need to actually emit events for the cursor-at-EOF situation,
-                self.next_span = None;
+            // even though the range is past the end of the text.  This needs to be
-                None
+            // handled appropriately by the drawing code by not assuming that
+            // all `Source` events point to valid indices in the rope.
+            (None, Some((span, range))) => {
+                let event = HighlightStart(Highlight(*span));
+                self.queue.push(HighlightEnd);
+                self.queue.push(Source {
+                    start: range.start,
+                    end: range.end,
+                });
+                self.next_span = self.spans.next();
+                Some(event)
             }
             (None, None) => None,
             e => unreachable!("{:?}", e),

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::chars::{categorize_char, CharCategory};
-use crate::movement::{self, Direction};
+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 {
+fn find_word_boundary(slice: RopeSlice, mut pos: usize, direction: Direction) -> usize {
-    this_word_bound_pos(slice, pos, Direction::Forward)
+    use CharCategory::{Eol, Whitespace};
-}
-
-fn this_word_start_pos(slice: RopeSlice, pos: usize) -> usize {
-    this_word_bound_pos(slice, pos, Direction::Backward)
-}
 
-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,31 @@ fn this_word_bound_pos(slice: RopeSlice, mut pos: usize, direction: Direction) -
         }
     };
 
-    match categorize_char(slice.char(pos)) {
+    let mut prev_category = match direction {
-        CharCategory::Eol | CharCategory::Whitespace => pos,
+        Direction::Forward if pos == 0 => Whitespace,
-        category => {
+        Direction::Forward => categorize_char(slice.char(pos - 1)),
-            for peek in iter {
+        Direction::Backward if pos == slice.len_chars() => Whitespace,
-                let curr_category = categorize_char(peek);
+        Direction::Backward => categorize_char(slice.char(pos)),
-                if curr_category != category
+    };
-                    || curr_category == CharCategory::Eol
+
-                    || curr_category == CharCategory::Whitespace
+    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;
-                }
+                } else {
-                pos = match direction {
+                    match direction {
-                    Direction::Forward => pos + 1,
+                        Direction::Forward => pos += 1,
-                    Direction::Backward => pos.saturating_sub(1),
+                        Direction::Backward => pos = pos.saturating_sub(1),
+                    }
+                    prev_category = category;
                 }
             }
-            pos
         }
     }
+
+    pos
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
@@ -55,46 +56,37 @@ 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 pos = range.cursor(slice);
-    let this_word_end = this_word_end_pos(slice, range.head);
+
+    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.
+    if word_start == word_end {
+        return Range::new(word_start, word_end);
+    }
 
-    let (anchor, head);
     match textobject {
-        TextObject::Inside => {
+        TextObject::Inside => Range::new(word_start, word_end),
-            anchor = this_word_start;
+        TextObject::Around => Range::new(
-            head = this_word_end;
+            match slice
-        }
+                .get_char(word_start.saturating_sub(1))
-        TextObject::Around => {
+                .map(categorize_char)
-            if slice
-                .get_char(this_word_end + 1)
-                .map_or(true, char_is_line_ending)
             {
-                head = this_word_end;
+                None | Some(CharCategory::Eol) => word_start,
-                if slice
+                _ => prev_grapheme_boundary(slice, word_start),
-                    .get_char(this_word_start.saturating_sub(1))
+            },
-                    .map_or(true, char_is_line_ending)
+            match slice.get_char(word_end).map(categorize_char) {
-                {
+                None | Some(CharCategory::Eol) => word_end,
-                    // single word on a line
+                _ => next_grapheme_boundary(slice, word_end),
-                    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)
 }
 
 pub fn textobject_surround(
@@ -106,7 +98,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 +121,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)),
+                    (13, Inside, (10, 16)),
-                    (10, Inside, (10, 15)),
+                    (10, Inside, (10, 16)),
-                    (15, Inside, (10, 15)),
+                    (15, Inside, (10, 16)),
-                    (13, Around, (10, 16)),
+                    (13, Around, (9, 17)),
-                    (10, Around, (10, 16)),
+                    (10, Around, (9, 17)),
-                    (15, Around, (10, 16)),
+                    (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)),
+                    (22, Inside, (22, 29)),
-                    (28, Inside, (22, 28)),
+                    (28, Inside, (22, 29)),
-                    (25, Inside, (22, 28)),
+                    (25, Inside, (22, 29)),
-                    (22, Around, (21, 28)),
+                    (22, Around, (21, 29)),
-                    (28, Around, (21, 28)),
+                    (28, Around, (21, 29)),
-                    (25, Around, (21, 28)),
+                    (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)),
+                    (29, Inside, (29, 33)),
-                    (30, Inside, (29, 32)),
+                    (30, Inside, (29, 33)),
-                    (32, Inside, (29, 32)),
+                    (32, Inside, (29, 33)),
-                    (29, Around, (29, 33)),
+                    (29, Around, (29, 34)),
-                    (30, Around, (29, 33)),
+                    (30, Around, (29, 34)),
-                    (32, Around, (29, 33)),
+                    (32, Around, (29, 34)),
                 ],
             ),
             (
                 "cursor on #$%:;* punctuation",
                 vec![
-                    (13, Inside, (10, 15)),
+                    (13, Inside, (10, 16)),
-                    (10, Inside, (10, 15)),
+                    (10, Inside, (10, 16)),
-                    (15, Inside, (10, 15)),
+                    (15, Inside, (10, 16)),
-                    (13, Around, (10, 16)),
+                    (13, Around, (9, 17)),
-                    (10, Around, (10, 16)),
+                    (10, Around, (9, 17)),
-                    (15, Around, (10, 16)),
+                    (15, Around, (9, 17)),
                 ],
             ),
             (
                 "cursor on punc%^#$:;.tuation",
                 vec![
-                    (14, Inside, (14, 20)),
+                    (14, Inside, (14, 21)),
-                    (20, Inside, (14, 20)),
+                    (20, Inside, (14, 21)),
-                    (17, Inside, (14, 20)),
+                    (17, Inside, (14, 21)),
-                    (14, Around, (14, 20)),
+                    (14, Around, (13, 22)),
                     // FIXME: edge case
                     // (20, Around, (14, 20)),
-                    (17, Around, (14, 20)),
+                    (17, Around, (13, 22)),
                 ],
             ),
             (
@@ -198,14 +193,14 @@ mod test {
                     (9, Inside, (9, 9)),
                     (10, Inside, (10, 10)),
                     (11, Inside, (11, 11)),
-                    (9, Around, (9, 16)),
+                    (9, Around, (9, 9)),
-                    (10, Around, (9, 16)),
+                    (10, Around, (10, 10)),
-                    (11, Around, (9, 16)),
+                    (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 +229,67 @@ mod test {
                 "simple (single) surround pairs",
                 vec![
                     (3, Inside, (3, 3), '(', 1),
-                    (7, Inside, (8, 13), ')', 1),
+                    (7, Inside, (8, 14), ')', 1),
-                    (10, Inside, (8, 13), '(', 1),
+                    (10, Inside, (8, 14), '(', 1),
-                    (14, Inside, (8, 13), ')', 1),
+                    (14, Inside, (8, 14), ')', 1),
                     (3, Around, (3, 3), '(', 1),
-                    (7, Around, (7, 14), ')', 1),
+                    (7, Around, (7, 15), ')', 1),
-                    (10, Around, (7, 14), '(', 1),
+                    (10, Around, (7, 15), '(', 1),
-                    (14, Around, (7, 14), ')', 1),
+                    (14, Around, (7, 15), ')', 1),
                 ],
             ),
             (
                 "samexx 'single' surround pairs",
                 vec![
                     (3, Inside, (3, 3), '\'', 1),
-                    (7, Inside, (8, 13), '\'', 1),
+                    (7, Inside, (7, 7), '\'', 1),
-                    (10, Inside, (8, 13), '\'', 1),
+                    (10, Inside, (8, 14), '\'', 1),
-                    (14, Inside, (8, 13), '\'', 1),
+                    (14, Inside, (14, 14), '\'', 1),
                     (3, Around, (3, 3), '\'', 1),
-                    (7, Around, (7, 14), '\'', 1),
+                    (7, Around, (7, 7), '\'', 1),
-                    (10, Around, (7, 14), '\'', 1),
+                    (10, Around, (7, 15), '\'', 1),
-                    (14, Around, (7, 14), '\'', 1),
+                    (14, Around, (14, 14), '\'', 1),
                 ],
             ),
             (
                 "(nested (surround (pairs)) 3 levels)",
                 vec![
-                    (0, Inside, (1, 34), '(', 1),
+                    (0, Inside, (1, 35), '(', 1),
-                    (6, Inside, (1, 34), ')', 1),
+                    (6, Inside, (1, 35), ')', 1),
-                    (8, Inside, (9, 24), '(', 1),
+                    (8, Inside, (9, 25), '(', 1),
-                    (8, Inside, (9, 34), ')', 2),
+                    (8, Inside, (9, 35), ')', 2),
-                    (20, Inside, (9, 24), '(', 2),
+                    (20, Inside, (9, 25), '(', 2),
-                    (20, Inside, (1, 34), ')', 3),
+                    (20, Inside, (1, 35), ')', 3),
-                    (0, Around, (0, 35), '(', 1),
+                    (0, Around, (0, 36), '(', 1),
-                    (6, Around, (0, 35), ')', 1),
+                    (6, Around, (0, 36), ')', 1),
-                    (8, Around, (8, 25), '(', 1),
+                    (8, Around, (8, 26), '(', 1),
-                    (8, Around, (8, 35), ')', 2),
+                    (8, Around, (8, 36), ')', 2),
-                    (20, Around, (8, 25), '(', 2),
+                    (20, Around, (8, 26), '(', 2),
-                    (20, Around, (0, 35), ')', 3),
+                    (20, Around, (0, 36), ')', 3),
                 ],
             ),
             (
                 "(mixed {surround [pair] same} line)",
                 vec![
-                    (2, Inside, (1, 33), '(', 1),
+                    (2, Inside, (1, 34), '(', 1),
-                    (9, Inside, (8, 27), '{', 1),
+                    (9, Inside, (8, 28), '{', 1),
-                    (18, Inside, (18, 21), '[', 1),
+                    (18, Inside, (18, 22), '[', 1),
-                    (2, Around, (0, 34), '(', 1),
+                    (2, Around, (0, 35), '(', 1),
-                    (9, Around, (7, 28), '{', 1),
+                    (9, Around, (7, 29), '{', 1),
-                    (18, Around, (17, 22), '[', 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),
+                    (7, Inside, (1, 29), '[', 1),
-                    (15, Inside, (16, 35), '{', 1),
+                    (15, Inside, (16, 36), '{', 1),
-                    (7, Around, (0, 29), '[', 1),
+                    (7, Around, (0, 30), '[', 1),
-                    (15, Around, (15, 36), '{', 1),
+                    (15, Around, (15, 37), '{', 1),
                 ],
             ),
         ];

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

helix-term/src/ui/editor.rs

@@ -8,7 +8,7 @@ use crate::{
 
 use helix_core::{
     coords_at_pos,
-    graphemes::{ensure_grapheme_boundary, next_grapheme_boundary},
+    graphemes::{ensure_grapheme_boundary_next, next_grapheme_boundary, prev_grapheme_boundary},
     syntax::{self, HighlightEvent},
     unicode::segmentation::UnicodeSegmentation,
     unicode::width::UnicodeWidthStr,
@@ -166,8 +166,8 @@ impl EditorView {
         let highlights = highlights.into_iter().map(|event| match event.unwrap() {
             // convert byte offsets to char offset
             HighlightEvent::Source { start, end } => {
-                let start = ensure_grapheme_boundary(text, text.byte_to_char(start));
+                let start = ensure_grapheme_boundary_next(text, text.byte_to_char(start));
-                let end = ensure_grapheme_boundary(text, text.byte_to_char(end));
+                let end = ensure_grapheme_boundary_next(text, text.byte_to_char(end));
                 HighlightEvent::Source { start, end }
             }
             event => event,
@@ -191,21 +191,18 @@ impl EditorView {
         }
         .unwrap_or(base_cursor_scope);
 
-        let primary_selection_scope = theme
-            .find_scope_index("ui.selection.primary")
-            .unwrap_or(selection_scope);
-
         let highlights: Box<dyn Iterator<Item = HighlightEvent>> = if is_focused {
-            // inject selections as highlight scopes
-            let mut spans: Vec<(usize, std::ops::Range<usize>)> = Vec::new();
-
             // TODO: primary + insert mode patching:
             // (ui.cursor.primary).patch(mode).unwrap_or(cursor)
-
             let primary_cursor_scope = theme
                 .find_scope_index("ui.cursor.primary")
                 .unwrap_or(cursor_scope);
+            let primary_selection_scope = theme
+                .find_scope_index("ui.selection.primary")
+                .unwrap_or(selection_scope);
 
+            // inject selections as highlight scopes
+            let mut spans: Vec<(usize, std::ops::Range<usize>)> = Vec::new();
             for (i, range) in selections.iter().enumerate() {
                 let (cursor_scope, selection_scope) = if i == primary_idx {
                     (primary_cursor_scope, primary_selection_scope)
@@ -213,24 +210,23 @@ impl EditorView {
                     (cursor_scope, selection_scope)
                 };
 
-                let cursor_end = next_grapheme_boundary(text, range.head); // Used in every case below.
+                // Special-case: cursor at end of the rope.
-
+                if range.head == range.anchor && range.head == text.len_chars() {
-                if range.head == range.anchor {
+                    spans.push((cursor_scope, range.head..range.head + 1));
-                    spans.push((cursor_scope, range.head..cursor_end));
                     continue;
                 }
 
-                let reverse = range.head < range.anchor;
+                let range = range.min_width_1(text);
-
+                if range.head > range.anchor {
-                if reverse {
+                    // Standard case.
-                    spans.push((cursor_scope, range.head..cursor_end));
+                    let cursor_start = prev_grapheme_boundary(text, range.head);
-                    spans.push((
+                    spans.push((selection_scope, range.anchor..cursor_start));
-                        selection_scope,
+                    spans.push((cursor_scope, cursor_start..range.head));
-                        cursor_end..next_grapheme_boundary(text, range.anchor),
-                    ));
                 } else {
-                    spans.push((selection_scope, range.anchor..range.head));
+                    // Reverse case.
+                    let cursor_end = next_grapheme_boundary(text, range.head);
                     spans.push((cursor_scope, range.head..cursor_end));
+                    spans.push((selection_scope, cursor_end..range.anchor));
                 }
             }
 
@@ -263,7 +259,10 @@ impl EditorView {
                     spans.pop();
                 }
                 HighlightEvent::Source { start, end } => {
-                    let text = text.slice(start..end);
+                    // `unwrap_or_else` part is for off-the-end indices of
+                    // the rope, to allow cursor highlighting at the end
+                    // of the rope.
+                    let text = text.get_slice(start..end).unwrap_or_else(|| " ".into());
 
                     use helix_core::graphemes::{grapheme_width, RopeGraphemes};
 
@@ -332,7 +331,11 @@ impl EditorView {
         let info: Style = theme.get("info");
         let hint: Style = theme.get("hint");
 
-        for (i, line) in (view.first_line..last_line).enumerate() {
+        // Whether to draw the line number for the last line of the
+        // document or not.  We only draw it if it's not an empty line.
+        let draw_last = text.line_to_byte(last_line) < text.len_bytes();
+
+        for (i, line) in (view.first_line..(last_line + 1)).enumerate() {
             use helix_core::diagnostic::Severity;
             if let Some(diagnostic) = doc.diagnostics().iter().find(|d| d.line == line) {
                 surface.set_stringn(
@@ -349,11 +352,17 @@ impl EditorView {
                 );
             }
 
-            // line numbers having selections are rendered differently
+            // Line numbers having selections are rendered
+            // differently, further below.
+            let line_number_text = if line == last_line && !draw_last {
+                "    ~".into()
+            } else {
+                format!("{:>5}", line + 1)
+            };
             surface.set_stringn(
                 viewport.x + 1 - OFFSET,
                 viewport.y + i as u16,
-                format!("{:>5}", line + 1),
+                line_number_text,
                 5,
                 linenr,
             );
@@ -367,19 +376,34 @@ impl EditorView {
         if is_focused {
             let screen = {
                 let start = text.line_to_char(view.first_line);
-                let end = text.line_to_char(last_line + 1);
+                let end = text.line_to_char(last_line + 1) + 1; // +1 for cursor at end of text.
                 Range::new(start, end)
             };
 
             let selection = doc.selection(view.id);
 
             for selection in selection.iter().filter(|range| range.overlaps(&screen)) {
-                let head = view.screen_coords_at_pos(doc, text, selection.head);
+                let head = view.screen_coords_at_pos(
+                    doc,
+                    text,
+                    if selection.head > selection.anchor {
+                        selection.head - 1
+                    } else {
+                        selection.head
+                    },
+                );
                 if let Some(head) = head {
+                    // Draw line number for selected lines.
+                    let line_number = view.first_line + head.row;
+                    let line_number_text = if line_number == last_line && !draw_last {
+                        "    ~".into()
+                    } else {
+                        format!("{:>5}", line_number + 1)
+                    };
                     surface.set_stringn(
                         viewport.x + 1 - OFFSET,
                         viewport.y + head.row as u16,
-                        format!("{:>5}", view.first_line + head.row + 1),
+                        line_number_text,
                         5,
                         linenr_select,
                     );
@@ -387,7 +411,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();
+                        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));
 
@@ -432,7 +459,10 @@ impl EditorView {
             widgets::{Paragraph, Widget},
         };
 
-        let cursor = doc.selection(view.id).cursor();
+        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
@@ -544,7 +574,12 @@ impl EditorView {
         //     _ => "indent:ERROR",
         // };
         let position_info = {
-            let pos = coords_at_pos(doc.text().slice(..), doc.selection(view.id).cursor());
+            let pos = coords_at_pos(
+                doc.text().slice(..),
+                doc.selection(view.id)
+                    .primary()
+                    .cursor(doc.text().slice(..)),
+            );
             format!("{}:{}", pos.row + 1, pos.col + 1) // convert to 1-indexing
         };
 

helix-view/src/document.rs

@@ -306,19 +306,6 @@ pub async fn to_writer<'a, W: tokio::io::AsyncWriteExt + Unpin + ?Sized>(
     Ok(())
 }
 
-/// Inserts the final line ending into `rope` if it's missing. [Why?](https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline)
-pub fn with_line_ending(rope: &mut Rope) -> LineEnding {
-    // search for line endings
-    let line_ending = auto_detect_line_ending(rope).unwrap_or(DEFAULT_LINE_ENDING);
-
-    // add missing newline at the end of file
-    if rope.len_bytes() == 0 || !char_is_line_ending(rope.char(rope.len_chars() - 1)) {
-        rope.insert(rope.len_chars(), line_ending.as_str());
-    }
-
-    line_ending
-}
-
 /// Like std::mem::replace() except it allows the replacement value to be mapped from the
 /// original value.
 fn take_with<T, F>(mut_ref: &mut T, closure: F)
@@ -456,7 +443,7 @@ impl Document {
         theme: Option<&Theme>,
         config_loader: Option<&syntax::Loader>,
     ) -> Result<Self, Error> {
-        let (mut rope, encoding) = if path.exists() {
+        let (rope, encoding) = if path.exists() {
             let mut file =
                 std::fs::File::open(&path).context(format!("unable to open {:?}", path))?;
             from_reader(&mut file, encoding)?
@@ -465,7 +452,6 @@ impl Document {
             (Rope::from(DEFAULT_LINE_ENDING.as_str()), encoding)
         };
 
-        let line_ending = with_line_ending(&mut rope);
         let mut doc = Self::from(rope, Some(encoding));
 
         // set the path and try detecting the language
@@ -474,9 +460,9 @@ impl Document {
             doc.detect_language(theme, loader);
         }
 
-        // Detect indentation style and set line ending.
+        // Detect indentation style and line ending.
         doc.detect_indent_style();
-        doc.line_ending = line_ending;
+        doc.line_ending = auto_detect_line_ending(&doc.text).unwrap_or(DEFAULT_LINE_ENDING);
 
         Ok(doc)
     }
@@ -605,17 +591,16 @@ impl Document {
         }
 
         let mut file = std::fs::File::open(path.unwrap())?;
-        let (mut rope, ..) = from_reader(&mut file, Some(encoding))?;
+        let (rope, ..) = from_reader(&mut file, Some(encoding))?;
-        let line_ending = with_line_ending(&mut rope);
 
         let transaction = helix_core::diff::compare_ropes(self.text(), &rope);
         self.apply(&transaction, view_id);
         self.append_changes_to_history(view_id);
         self.reset_modified();
 
-        // Detect indentation style and set line ending.
+        // Detect indentation style and line ending.
         self.detect_indent_style();
-        self.line_ending = line_ending;
+        self.line_ending = auto_detect_line_ending(&self.text).unwrap_or(DEFAULT_LINE_ENDING);
 
         Ok(())
     }
@@ -807,7 +792,8 @@ impl Document {
 
     pub fn set_selection(&mut self, view_id: ViewId, selection: Selection) {
         // TODO: use a transaction?
-        self.selections.insert(view_id, selection);
+        self.selections
+            .insert(view_id, selection.ensure_invariants(self.text().slice(..)));
     }
 
     fn apply_impl(&mut self, transaction: &Transaction, view_id: ViewId) -> bool {
@@ -822,7 +808,12 @@ impl Document {
                 .selection()
                 .cloned()
                 .unwrap_or_else(|| self.selection(view_id).clone().map(transaction.changes()));
-            self.set_selection(view_id, selection);
+            self.selections.insert(view_id, selection);
+
+            // Ensure all selections accross all views still adhere to invariants.
+            for selection in self.selections.values_mut() {
+                *selection = selection.clone().ensure_invariants(self.text.slice(..));
+            }
         }
 
         if !transaction.changes().is_empty() {
@@ -1089,7 +1080,7 @@ impl Document {
 
 impl Default for Document {
     fn default() -> Self {
-        let text = Rope::from(DEFAULT_LINE_ENDING.as_str());
+        let text = Rope::from("");
         Self::from(text, None)
     }
 }
@@ -1214,11 +1205,7 @@ mod test {
 
     #[test]
     fn test_line_ending() {
-        if cfg!(windows) {
+        assert_eq!(Document::default().text().to_string(), "");
-            assert_eq!(Document::default().text().to_string(), "\r\n");
-        } else {
-            assert_eq!(Document::default().text().to_string(), "\n");
-        }
     }
 
     macro_rules! test_decode {

helix-view/src/editor.rs

@@ -138,12 +138,14 @@ impl Editor {
                 let (view, doc) = current!(self);
 
                 // initialize selection for view
-                let selection = doc
+                doc.selections
-                    .selections
                     .entry(view.id)
                     .or_insert_with(|| Selection::point(0));
                 // TODO: reuse align_view
-                let pos = selection.cursor();
+                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);
 
@@ -293,7 +295,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();
+        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;

helix-view/src/view.rs

@@ -84,18 +84,21 @@ impl View {
     }
 
     pub fn ensure_cursor_in_view(&mut self, doc: &Document) {
-        let cursor = doc.selection(self.id).cursor();
+        let cursor = doc
+            .selection(self.id)
+            .primary()
+            .cursor(doc.text().slice(..));
         let pos = coords_at_pos(doc.text().slice(..), cursor);
         let line = pos.row;
         let col = pos.col;
         let height = self.area.height.saturating_sub(1); // - 1 for statusline
-        let last_line = self.first_line + height as usize;
+        let last_line = (self.first_line + height as usize).saturating_sub(1);
 
         let scrolloff = PADDING.min(self.area.height as usize / 2); // TODO: user pref
 
         // TODO: not ideal
         const OFFSET: usize = 7; // 1 diagnostic + 5 linenr + 1 gutter
-        let last_col = self.first_col + (self.area.width as usize - OFFSET);
+        let last_col = (self.first_col + self.area.width as usize).saturating_sub(OFFSET + 1);
 
         if line > last_line.saturating_sub(scrolloff) {
             // scroll down
@@ -119,8 +122,9 @@ impl View {
     pub fn last_line(&self, doc: &Document) -> usize {
         let height = self.area.height.saturating_sub(1); // - 1 for statusline
         std::cmp::min(
-            self.first_line + height as usize,
+            // Saturating subs to make it inclusive zero indexing.
-            doc.text().len_lines() - 1,
+            (self.first_line + height as usize).saturating_sub(1),
+            doc.text().len_lines().saturating_sub(1),
         )
     }