Ensure coords in screen depends on char width (#885)

The issue affected files with lots of tabs at the start as well.

Fix #840

5 files changed, 87 insertions, 18 deletions

diff --git a/helix-core/src/lib.rs b/helix-core/src/lib.rs
index 96f88ee4..d1720df0 100644
--- a/helix-core/src/lib.rs
+++ b/helix-core/src/lib.rs
@@ -194,7 +194,7 @@ pub use tendril::StrTendril as Tendril;
 pub use {regex, tree_sitter};
 
 pub use graphemes::RopeGraphemes;
-pub use position::{coords_at_pos, pos_at_coords, Position};
+pub use position::{coords_at_pos, pos_at_coords, visual_coords_at_pos, Position};
 pub use selection::{Range, Selection};
 pub use smallvec::SmallVec;
 pub use syntax::Syntax;
diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs
index 5d080545..9e85bd21 100644
--- a/helix-core/src/movement.rs
+++ b/helix-core/src/movement.rs
@@ -53,6 +53,10 @@ pub fn move_vertically(
     let pos = range.cursor(slice);
 
     // Compute the current position's 2d coordinates.
+    // TODO: switch this to use `visual_coords_at_pos` rather than
+    // `coords_at_pos` as this will cause a jerky movement when the visual
+    // position does not match, like moving from a line with tabs/CJK to
+    // a line without
     let Position { row, col } = coords_at_pos(slice, pos);
     let horiz = range.horiz.unwrap_or(col as u32);
 
diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs
index 08a8aed5..c6018ce6 100644
--- a/helix-core/src/position.rs
+++ b/helix-core/src/position.rs
@@ -2,6 +2,7 @@ use crate::{
     chars::char_is_line_ending,
     graphemes::{ensure_grapheme_boundary_prev, RopeGraphemes},
     line_ending::line_end_char_index,
+    unicode::width::UnicodeWidthChar,
     RopeSlice,
 };
 
@@ -54,11 +55,8 @@ 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.
+/// column in `char` count which can be used for row:column display in
+/// status line. See [`visual_coords_at_pos`] for a visual one.
 pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position {
     let line = text.char_to_line(pos);
 
@@ -69,6 +67,28 @@ pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position {
     Position::new(line, col)
 }
 
+/// Convert a character index to (line, column) coordinates visually.
+///
+/// Takes \t, double-width characters (CJK) into account as well as text
+/// not in the document in the future.
+/// See [`coords_at_pos`] for an "objective" one.
+pub fn visual_coords_at_pos(text: RopeSlice, pos: usize, tab_width: 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 = text
+        .slice(line_start..pos)
+        .chars()
+        .flat_map(|c| match c {
+            '\t' => Some(tab_width),
+            c => UnicodeWidthChar::width(c),
+        })
+        .sum();
+
+    Position::new(line, col)
+}
+
 /// Convert (line, column) coordinates to a character index.
 ///
 /// If the `line` coordinate is beyond the end of the file, the EOF
@@ -130,7 +150,6 @@ mod test {
         assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d
 
         // Test with wide characters.
-        // TODO: account for character width.
         let text = Rope::from("今日はいい\n");
         let slice = text.slice(..);
         assert_eq!(coords_at_pos(slice, 0), (0, 0).into());
@@ -151,7 +170,6 @@ mod test {
         assert_eq!(coords_at_pos(slice, 9), (1, 0).into());
 
         // Test with wide-character grapheme clusters.
-        // TODO: account for character width.
         let text = Rope::from("किमपि\n");
         let slice = text.slice(..);
         assert_eq!(coords_at_pos(slice, 0), (0, 0).into());
@@ -161,7 +179,6 @@ mod test {
         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());
@@ -170,6 +187,54 @@ mod test {
     }
 
     #[test]
+    fn test_visual_coords_at_pos() {
+        let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ");
+        let slice = text.slice(..);
+        assert_eq!(visual_coords_at_pos(slice, 0, 8), (0, 0).into());
+        assert_eq!(visual_coords_at_pos(slice, 5, 8), (0, 5).into()); // position on \n
+        assert_eq!(visual_coords_at_pos(slice, 6, 8), (1, 0).into()); // position on w
+        assert_eq!(visual_coords_at_pos(slice, 7, 8), (1, 1).into()); // position on o
+        assert_eq!(visual_coords_at_pos(slice, 10, 8), (1, 4).into()); // position on d
+
+        // Test with wide characters.
+        let text = Rope::from("今日はいい\n");
+        let slice = text.slice(..);
+        assert_eq!(visual_coords_at_pos(slice, 0, 8), (0, 0).into());
+        assert_eq!(visual_coords_at_pos(slice, 1, 8), (0, 2).into());
+        assert_eq!(visual_coords_at_pos(slice, 2, 8), (0, 4).into());
+        assert_eq!(visual_coords_at_pos(slice, 3, 8), (0, 6).into());
+        assert_eq!(visual_coords_at_pos(slice, 4, 8), (0, 8).into());
+        assert_eq!(visual_coords_at_pos(slice, 5, 8), (0, 10).into());
+        assert_eq!(visual_coords_at_pos(slice, 6, 8), (1, 0).into());
+
+        // Test with grapheme clusters.
+        let text = Rope::from("a̐éö̲\r\n");
+        let slice = text.slice(..);
+        assert_eq!(visual_coords_at_pos(slice, 0, 8), (0, 0).into());
+        assert_eq!(visual_coords_at_pos(slice, 2, 8), (0, 1).into());
+        assert_eq!(visual_coords_at_pos(slice, 4, 8), (0, 2).into());
+        assert_eq!(visual_coords_at_pos(slice, 7, 8), (0, 3).into());
+        assert_eq!(visual_coords_at_pos(slice, 9, 8), (1, 0).into());
+
+        // Test with wide-character grapheme clusters.
+        // TODO: account for cluster.
+        let text = Rope::from("किमपि\n");
+        let slice = text.slice(..);
+        assert_eq!(visual_coords_at_pos(slice, 0, 8), (0, 0).into());
+        assert_eq!(visual_coords_at_pos(slice, 2, 8), (0, 2).into());
+        assert_eq!(visual_coords_at_pos(slice, 3, 8), (0, 3).into());
+        assert_eq!(visual_coords_at_pos(slice, 5, 8), (0, 5).into());
+        assert_eq!(visual_coords_at_pos(slice, 6, 8), (1, 0).into());
+
+        // Test with tabs.
+        let text = Rope::from("\tHello\n");
+        let slice = text.slice(..);
+        assert_eq!(visual_coords_at_pos(slice, 0, 8), (0, 0).into());
+        assert_eq!(visual_coords_at_pos(slice, 1, 8), (0, 8).into());
+        assert_eq!(visual_coords_at_pos(slice, 2, 8), (0, 9).into());
+    }
+
+    #[test]
     fn test_pos_at_coords() {
         let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ");
         let slice = text.slice(..);
diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs
index dcb2bfd8..dd782d29 100644
--- a/helix-term/src/ui/completion.rs
+++ b/helix-term/src/ui/completion.rs
@@ -264,12 +264,10 @@ impl Component for Completion {
                 .language()
                 .and_then(|scope| scope.strip_prefix("source."))
                 .unwrap_or("");
-            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.offset.row) as u16;
+            let text = doc.text().slice(..);
+            let cursor_pos = doc.selection(view.id).primary().cursor(text);
+            let coords = helix_core::visual_coords_at_pos(text, cursor_pos, doc.tab_width());
+            let cursor_pos = (coords.row - view.offset.row) as u16;
             let mut markdown_doc = match &option.documentation {
                 Some(lsp::Documentation::String(contents))
                 | Some(lsp::Documentation::MarkupContent(lsp::MarkupContent {
diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs
index 01f18c71..11f30155 100644
--- a/helix-view/src/view.rs
+++ b/helix-view/src/view.rs
@@ -2,10 +2,9 @@ use std::borrow::Cow;
 
 use crate::{graphics::Rect, Document, DocumentId, ViewId};
 use helix_core::{
-    coords_at_pos,
     graphemes::{grapheme_width, RopeGraphemes},
     line_ending::line_end_char_index,
-    Position, RopeSlice, Selection,
+    visual_coords_at_pos, Position, RopeSlice, Selection,
 };
 
 type Jump = (DocumentId, Selection);
@@ -91,7 +90,10 @@ impl View {
             .selection(self.id)
             .primary()
             .cursor(doc.text().slice(..));
-        let Position { col, row: line } = coords_at_pos(doc.text().slice(..), cursor);
+
+        let Position { col, row: line } =
+            visual_coords_at_pos(doc.text().slice(..), cursor, doc.tab_width());
+
         let inner_area = self.inner_area();
         let last_line = (self.offset.row + inner_area.height as usize).saturating_sub(1);