From 079d4ed86df30c78ca00fd4b86f906c3ea9df7db Mon Sep 17 00:00:00 2001
From: Nathan Vegdahl
Date: Mon, 19 Jul 2021 08:39:48 -0700
Subject: Properly fix `last_line` view calculation.

Turned out to be simpler than I thought.  Didn't even need to change the
other use-sites.
---
 helix-view/src/view.rs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

(limited to 'helix-view/src/view.rs')

diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs
index 24df7a4f..ccb61646 100644
--- a/helix-view/src/view.rs
+++ b/helix-view/src/view.rs
@@ -119,8 +119,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,
-            doc.text().len_lines() - 1,
+            // Saturating subs to make it inclusive zero indexing.
+            (self.first_line + height as usize).saturating_sub(1),
+            doc.text().len_lines().saturating_sub(1),
         )
     }
 
-- 
cgit v1.2.3-70-g09d2


From b0311f4fc246f370bb317ab0b1fcb89823347caa Mon Sep 17 00:00:00 2001
From: Nathan Vegdahl
Date: Mon, 19 Jul 2021 09:25:10 -0700
Subject: Fixed primary cursor position calculation to use 1-width semantics.

This had a bunch of knock-on effects that were buggy, such as bracket
match highlighting.
---
 helix-core/src/selection.rs     | 11 +++++++--
 helix-term/src/commands.rs      | 50 ++++++++++++++++++++++++++++-------------
 helix-term/src/ui/completion.rs |  8 +++----
 helix-term/src/ui/editor.rs     |  9 +++++---
 helix-view/src/editor.rs        |  7 +++---
 helix-view/src/view.rs          |  2 +-
 6 files changed, 58 insertions(+), 29 deletions(-)

(limited to 'helix-view/src/view.rs')

diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs
index 21a6c108..7d2526b0 100644
--- a/helix-core/src/selection.rs
+++ b/helix-core/src/selection.rs
@@ -267,8 +267,15 @@ impl Selection {
     }
 
     #[must_use]
-    pub fn cursor(&self) -> usize {
-        self.primary().head
+    pub fn cursor(&self, text: RopeSlice) -> usize {
+        let range = self.primary();
+
+        // For 1-width cursor semantics.
+        if range.anchor < range.head {
+            prev_grapheme_boundary(text, range.head)
+        } else {
+            range.head
+        }
     }
 
     /// Ensure selection containing only the primary selection.
diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs
index cea5a24e..add83b41 100644
--- a/helix-term/src/commands.rs
+++ b/helix-term/src/commands.rs
@@ -121,7 +121,7 @@ enum Align {
 }
 
 fn align_view(doc: &Document, view: &mut View, align: Align) {
-    let pos = doc.selection(view.id).cursor();
+    let pos = doc.selection(view.id).cursor(doc.text().slice(..));
     let line = doc.text().char_to_line(pos);
 
     let relative = match align {
@@ -868,7 +868,10 @@ fn switch_to_lowercase(cx: &mut Context) {
 fn scroll(cx: &mut Context, offset: usize, direction: Direction) {
     use Direction::*;
     let (view, doc) = current!(cx.editor);
-    let cursor = coords_at_pos(doc.text().slice(..), doc.selection(view.id).cursor());
+    let cursor = coords_at_pos(
+        doc.text().slice(..),
+        doc.selection(view.id).cursor(doc.text().slice(..)),
+    );
     let doc_last_line = doc.text().len_lines() - 1;
 
     let last_line = view.last_line(doc);
@@ -1038,7 +1041,7 @@ fn split_selection_on_newline(cx: &mut Context) {
 fn search_impl(doc: &mut Document, view: &mut View, contents: &str, regex: &Regex, extend: bool) {
     let text = doc.text();
     let selection = doc.selection(view.id);
-    let start = text.char_to_byte(selection.cursor());
+    let start = text.char_to_byte(selection.cursor(text.slice(..)));
 
     // use find_at to find the next match after the cursor, loop around the end
     // Careful, `Regex` uses `bytes` as offsets, not character indices!
@@ -2446,7 +2449,11 @@ fn goto_definition(cx: &mut Context) {
 
     let offset_encoding = language_server.offset_encoding();
 
-    let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding);
+    let pos = pos_to_lsp_pos(
+        doc.text(),
+        doc.selection(view.id).cursor(doc.text().slice(..)),
+        offset_encoding,
+    );
 
     // TODO: handle fails
     let future = language_server.goto_definition(doc.identifier(), pos, None);
@@ -2483,7 +2490,11 @@ fn goto_type_definition(cx: &mut Context) {
 
     let offset_encoding = language_server.offset_encoding();
 
-    let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding);
+    let pos = pos_to_lsp_pos(
+        doc.text(),
+        doc.selection(view.id).cursor(doc.text().slice(..)),
+        offset_encoding,
+    );
 
     // TODO: handle fails
     let future = language_server.goto_type_definition(doc.identifier(), pos, None);
@@ -2520,7 +2531,11 @@ fn goto_implementation(cx: &mut Context) {
 
     let offset_encoding = language_server.offset_encoding();
 
-    let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding);
+    let pos = pos_to_lsp_pos(
+        doc.text(),
+        doc.selection(view.id).cursor(doc.text().slice(..)),
+        offset_encoding,
+    );
 
     // TODO: handle fails
     let future = language_server.goto_implementation(doc.identifier(), pos, None);
@@ -2557,7 +2572,11 @@ fn goto_reference(cx: &mut Context) {
 
     let offset_encoding = language_server.offset_encoding();
 
-    let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding);
+    let pos = pos_to_lsp_pos(
+        doc.text(),
+        doc.selection(view.id).cursor(doc.text().slice(..)),
+        offset_encoding,
+    );
 
     // TODO: handle fails
     let future = language_server.goto_reference(doc.identifier(), pos, None);
@@ -2616,7 +2635,7 @@ fn goto_next_diag(cx: &mut Context) {
     let editor = &mut cx.editor;
     let (view, doc) = current!(editor);
 
-    let cursor_pos = doc.selection(view.id).cursor();
+    let cursor_pos = doc.selection(view.id).cursor(doc.text().slice(..));
     let diag = if let Some(diag) = doc
         .diagnostics()
         .iter()
@@ -2637,7 +2656,7 @@ fn goto_prev_diag(cx: &mut Context) {
     let editor = &mut cx.editor;
     let (view, doc) = current!(editor);
 
-    let cursor_pos = doc.selection(view.id).cursor();
+    let cursor_pos = doc.selection(view.id).cursor(doc.text().slice(..));
     let diag = if let Some(diag) = doc
         .diagnostics()
         .iter()
@@ -2665,7 +2684,7 @@ fn signature_help(cx: &mut Context) {
 
     let pos = pos_to_lsp_pos(
         doc.text(),
-        doc.selection(view.id).cursor(),
+        doc.selection(view.id).cursor(doc.text().slice(..)),
         language_server.offset_encoding(),
     );
 
@@ -3403,13 +3422,14 @@ fn completion(cx: &mut Context) {
     };
 
     let offset_encoding = language_server.offset_encoding();
+    let cursor = doc.selection(view.id).cursor(doc.text().slice(..));
 
-    let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding);
+    let pos = pos_to_lsp_pos(doc.text(), cursor, offset_encoding);
 
     // TODO: handle fails
     let future = language_server.completion(doc.identifier(), pos, None);
 
-    let trigger_offset = doc.selection(view.id).cursor();
+    let trigger_offset = cursor;
 
     cx.callback(
         future,
@@ -3459,7 +3479,7 @@ fn hover(cx: &mut Context) {
 
     let pos = pos_to_lsp_pos(
         doc.text(),
-        doc.selection(view.id).cursor(),
+        doc.selection(view.id).cursor(doc.text().slice(..)),
         language_server.offset_encoding(),
     );
 
@@ -3525,7 +3545,7 @@ fn match_brackets(cx: &mut Context) {
     let (view, doc) = current!(cx.editor);
 
     if let Some(syntax) = doc.syntax() {
-        let pos = doc.selection(view.id).cursor();
+        let pos = doc.selection(view.id).cursor(doc.text().slice(..));
         if let Some(pos) = match_brackets::find(syntax, doc.text(), pos) {
             let selection = Selection::point(pos);
             doc.set_selection(view.id, selection);
@@ -3629,7 +3649,7 @@ fn align_view_bottom(cx: &mut Context) {
 
 fn align_view_middle(cx: &mut Context) {
     let (view, doc) = current!(cx.editor);
-    let pos = doc.selection(view.id).cursor();
+    let pos = doc.selection(view.id).cursor(doc.text().slice(..));
     let pos = coords_at_pos(doc.text().slice(..), pos);
 
     const OFFSET: usize = 7; // gutters
diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs
index be6db42c..942a2483 100644
--- a/helix-term/src/ui/completion.rs
+++ b/helix-term/src/ui/completion.rs
@@ -86,7 +86,7 @@ impl Completion {
                     let item = item.unwrap();
 
                     // if more text was entered, remove it
-                    let cursor = doc.selection(view.id).cursor();
+                    let cursor = doc.selection(view.id).cursor(doc.text().slice(..));
                     if trigger_offset < cursor {
                         let remove = Transaction::change(
                             doc.text(),
@@ -109,7 +109,7 @@ impl Completion {
                         )
                     } else {
                         let text = item.insert_text.as_ref().unwrap_or(&item.label);
-                        let cursor = doc.selection(view.id).cursor();
+                        let cursor = doc.selection(view.id).cursor(doc.text().slice(..));
                         Transaction::change(
                             doc.text(),
                             vec![(cursor, cursor, Some(text.as_str().into()))].into_iter(),
@@ -155,7 +155,7 @@ impl Completion {
         // TODO: hooks should get processed immediately so maybe do it after select!(), before
         // looping?
 
-        let cursor = doc.selection(view.id).cursor();
+        let cursor = doc.selection(view.id).cursor(doc.text().slice(..));
         if self.trigger_offset <= cursor {
             let fragment = doc.text().slice(self.trigger_offset..cursor);
             let text = Cow::from(fragment);
@@ -212,7 +212,7 @@ impl Component for Completion {
                 .language()
                 .and_then(|scope| scope.strip_prefix("source."))
                 .unwrap_or("");
-            let cursor_pos = doc.selection(view.id).cursor();
+            let cursor_pos = doc.selection(view.id).cursor(doc.text().slice(..));
             let cursor_pos = (helix_core::coords_at_pos(doc.text().slice(..), cursor_pos).row
                 - view.first_line) as u16;
 
diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs
index 6a588e12..acf60905 100644
--- a/helix-term/src/ui/editor.rs
+++ b/helix-term/src/ui/editor.rs
@@ -398,7 +398,7 @@ impl EditorView {
                     // TODO: set cursor position for IME
                     if let Some(syntax) = doc.syntax() {
                         use helix_core::match_brackets;
-                        let pos = doc.selection(view.id).cursor();
+                        let pos = doc.selection(view.id).cursor(doc.text().slice(..));
                         let pos = match_brackets::find(syntax, doc.text(), pos)
                             .and_then(|pos| view.screen_coords_at_pos(doc, text, pos));
 
@@ -443,7 +443,7 @@ impl EditorView {
             widgets::{Paragraph, Widget},
         };
 
-        let cursor = doc.selection(view.id).cursor();
+        let cursor = doc.selection(view.id).cursor(doc.text().slice(..));
 
         let diagnostics = doc.diagnostics().iter().filter(|diagnostic| {
             diagnostic.range.start <= cursor && diagnostic.range.end >= cursor
@@ -555,7 +555,10 @@ impl EditorView {
         //     _ => "indent:ERROR",
         // };
         let position_info = {
-            let pos = coords_at_pos(doc.text().slice(..), doc.selection(view.id).cursor());
+            let pos = coords_at_pos(
+                doc.text().slice(..),
+                doc.selection(view.id).cursor(doc.text().slice(..)),
+            );
             format!("{}:{}", pos.row + 1, pos.col + 1) // convert to 1-indexing
         };
 
diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs
index cd9d0a92..1cd0af02 100644
--- a/helix-view/src/editor.rs
+++ b/helix-view/src/editor.rs
@@ -141,12 +141,11 @@ impl Editor {
                 let (view, doc) = current!(self);
 
                 // initialize selection for view
-                let selection = doc
-                    .selections
+                doc.selections
                     .entry(view.id)
                     .or_insert_with(|| Selection::point(0));
                 // TODO: reuse align_view
-                let pos = selection.cursor();
+                let pos = doc.selection(view.id).cursor(doc.text().slice(..));
                 let line = doc.text().char_to_line(pos);
                 view.first_line = line.saturating_sub(view.area.height as usize / 2);
 
@@ -296,7 +295,7 @@ impl Editor {
         const OFFSET: u16 = 7; // 1 diagnostic + 5 linenr + 1 gutter
         let view = view!(self);
         let doc = &self.documents[view.doc];
-        let cursor = doc.selection(view.id).cursor();
+        let cursor = doc.selection(view.id).cursor(doc.text().slice(..));
         if let Some(mut pos) = view.screen_coords_at_pos(doc, doc.text().slice(..), cursor) {
             pos.col += view.area.x as usize + OFFSET as usize;
             pos.row += view.area.y as usize;
diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs
index ccb61646..67585ed3 100644
--- a/helix-view/src/view.rs
+++ b/helix-view/src/view.rs
@@ -84,7 +84,7 @@ impl View {
     }
 
     pub fn ensure_cursor_in_view(&mut self, doc: &Document) {
-        let cursor = doc.selection(self.id).cursor();
+        let cursor = doc.selection(self.id).cursor(doc.text().slice(..));
         let pos = coords_at_pos(doc.text().slice(..), cursor);
         let line = pos.row;
         let col = pos.col;
-- 
cgit v1.2.3-70-g09d2


From 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-view/src/view.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 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-view/src/view.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 5ee6ba5b38ebeb86006bb2e42734a2285eb354df Mon Sep 17 00:00:00 2001
From: Nathan Vegdahl
Date: Mon, 26 Jul 2021 10:51:00 -0700
Subject: Address some PR comments.

---
 helix-core/src/textobject.rs | 7 +++----
 helix-view/src/view.rs       | 8 ++++----
 2 files changed, 7 insertions(+), 8 deletions(-)

(limited to 'helix-view/src/view.rs')

diff --git a/helix-core/src/textobject.rs b/helix-core/src/textobject.rs
index e011c912..b06bca5d 100644
--- a/helix-core/src/textobject.rs
+++ b/helix-core/src/textobject.rs
@@ -32,10 +32,9 @@ fn find_word_boundary(slice: RopeSlice, mut pos: usize, direction: Direction) ->
                 if category != prev_category && pos != 0 && pos != slice.len_chars() {
                     return pos;
                 } else {
-                    if direction == Direction::Forward {
-                        pos += 1;
-                    } else {
-                        pos = pos.saturating_sub(1);
+                    match direction {
+                        Direction::Forward => pos += 1,
+                        Direction::Backward => pos = pos.saturating_sub(1),
                     }
                     prev_category = category;
                 }
diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs
index e90d0eab..6b0c3c2a 100644
--- a/helix-view/src/view.rs
+++ b/helix-view/src/view.rs
@@ -92,17 +92,17 @@ impl View {
         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 + 1) {
+        if line > last_line.saturating_sub(scrolloff) {
             // scroll down
-            self.first_line += line - (last_line.saturating_sub(scrolloff + 1));
+            self.first_line += line - (last_line.saturating_sub(scrolloff));
         } else if line < self.first_line + scrolloff {
             // scroll up
             self.first_line = line.saturating_sub(scrolloff);
-- 
cgit v1.2.3-70-g09d2