From fd684ef6931d11632274d87c9a42d4fabb51c074 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Thu, 22 Jul 2021 13:21:44 -0700 Subject: [PATCH] Revert display-width-based vertical cursor movement. Still needs to be done, but should be part of a separate PR. --- helix-core/src/movement.rs | 18 ++++---- helix-core/src/position.rs | 89 +++++++++++++++++++++----------------- 2 files changed, 57 insertions(+), 50 deletions(-) diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index 01bec47ac..7f3a5ef4b 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -521,16 +521,14 @@ mod test { V, } let moves_and_expected_coordinates = IntoIter::new([ - // Places cursor at the fourth kana (each of which are double-wide, - // so the visual column is 8). - ((Axis::H, Direction::Forward, 4), (0, 8)), - // Descent places cursor at the 8th character. - ((Axis::V, Direction::Forward, 1usize), (1, 8)), - // Moving back a single-width character. - ((Axis::H, Direction::Backward, 1usize), (1, 7)), - // Jumping back up into the middle of a double-width character shifts - // the column to the start of that character. - ((Axis::V, Direction::Backward, 1usize), (0, 6)), + // Places cursor at the fourth kana. + ((Axis::H, Direction::Forward, 4), (0, 4)), + // Descent places cursor at the 4th character. + ((Axis::V, Direction::Forward, 1usize), (1, 4)), + // Moving back 1 character. + ((Axis::H, Direction::Backward, 1usize), (1, 3)), + // Jumping back up 1 line. + ((Axis::V, Direction::Backward, 1usize), (0, 3)), ]); for ((axis, direction, amount), coordinates) in moves_and_expected_coordinates { diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs index 156276879..611e6b76b 100644 --- a/helix-core/src/position.rs +++ b/helix-core/src/position.rs @@ -1,8 +1,6 @@ -use std::borrow::Cow; - use crate::{ chars::char_is_line_ending, - graphemes::{ensure_grapheme_boundary_prev, grapheme_width, RopeGraphemes}, + graphemes::{ensure_grapheme_boundary_prev, RopeGraphemes}, line_ending::line_end_char_index, RopeSlice, }; @@ -66,12 +64,7 @@ pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position { let line_start = text.line_to_char(line); let pos = ensure_grapheme_boundary_prev(text, pos); - let col = RopeGraphemes::new(text.slice(line_start..pos)) - .map(|g| { - let g: Cow = g.into(); - grapheme_width(&g) - }) - .sum(); + let col = RopeGraphemes::new(text.slice(line_start..pos)).count(); Position::new(line, col) } @@ -83,6 +76,9 @@ pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position { /// `false` corresponds to properly round-tripping with `coords_at_pos()`, /// whereas `true` will ensure that block cursors don't jump off the /// end of the line. +/// +/// TODO: this should be changed to work in terms of visual row/column, not +/// graphemes. pub fn pos_at_coords(text: RopeSlice, coords: Position, is_1_width: bool) -> usize { let Position { row, col } = coords; let line_start = text.line_to_char(row); @@ -92,17 +88,11 @@ pub fn pos_at_coords(text: RopeSlice, coords: Position, is_1_width: bool) -> usi text.line_to_char((row + 1).min(text.len_lines())) }; - let mut prev_col = 0; let mut col_char_offset = 0; - for g in RopeGraphemes::new(text.slice(line_start..line_end)) { - let g: Cow = g.into(); - let next_col = prev_col + grapheme_width(&g); - - if next_col > col { + for (i, g) in RopeGraphemes::new(text.slice(line_start..line_end)).enumerate() { + if i == col { break; } - - prev_col = next_col; col_char_offset += g.chars().count(); } @@ -131,17 +121,18 @@ mod test { assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d // Test with wide characters. + // TODO: account for character width. let text = Rope::from("今日はいい\n"); let slice = text.slice(..); assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); - assert_eq!(coords_at_pos(slice, 1), (0, 2).into()); - assert_eq!(coords_at_pos(slice, 2), (0, 4).into()); - assert_eq!(coords_at_pos(slice, 3), (0, 6).into()); - assert_eq!(coords_at_pos(slice, 4), (0, 8).into()); - assert_eq!(coords_at_pos(slice, 5), (0, 10).into()); + assert_eq!(coords_at_pos(slice, 1), (0, 1).into()); + assert_eq!(coords_at_pos(slice, 2), (0, 2).into()); + assert_eq!(coords_at_pos(slice, 3), (0, 3).into()); + assert_eq!(coords_at_pos(slice, 4), (0, 4).into()); + assert_eq!(coords_at_pos(slice, 5), (0, 5).into()); assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); - // test with grapheme clusters + // Test with grapheme clusters. let text = Rope::from("a̐éö̲\r\n"); let slice = text.slice(..); assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); @@ -150,13 +141,23 @@ mod test { assert_eq!(coords_at_pos(slice, 7), (0, 3).into()); assert_eq!(coords_at_pos(slice, 9), (1, 0).into()); + // Test with wide-character grapheme clusters. + // TODO: account for character width. let text = Rope::from("किमपि\n"); let slice = text.slice(..); assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); - assert_eq!(coords_at_pos(slice, 2), (0, 2).into()); - assert_eq!(coords_at_pos(slice, 3), (0, 3).into()); - assert_eq!(coords_at_pos(slice, 5), (0, 5).into()); + assert_eq!(coords_at_pos(slice, 2), (0, 1).into()); + assert_eq!(coords_at_pos(slice, 3), (0, 2).into()); + assert_eq!(coords_at_pos(slice, 5), (0, 3).into()); assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); + + // Test with tabs. + // Todo: account for tab stops. + let text = Rope::from("\tHello\n"); + let slice = text.slice(..); + assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); + assert_eq!(coords_at_pos(slice, 1), (0, 1).into()); + assert_eq!(coords_at_pos(slice, 2), (0, 2).into()); } #[test] @@ -172,21 +173,20 @@ mod test { assert_eq!(pos_at_coords(slice, (1, 4).into(), false), 10); // position on d // Test with wide characters. + // TODO: account for character width. let text = Rope::from("今日はいい\n"); let slice = text.slice(..); assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); - assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 0); - assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 1); - assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 1); - assert_eq!(pos_at_coords(slice, (0, 4).into(), false), 2); - assert_eq!(pos_at_coords(slice, (0, 6).into(), false), 3); - assert_eq!(pos_at_coords(slice, (0, 8).into(), false), 4); - assert_eq!(pos_at_coords(slice, (0, 10).into(), false), 5); - assert_eq!(pos_at_coords(slice, (0, 11).into(), false), 6); - assert_eq!(pos_at_coords(slice, (0, 11).into(), true), 5); + assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 1); + assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 2); + assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 3); + assert_eq!(pos_at_coords(slice, (0, 4).into(), false), 4); + assert_eq!(pos_at_coords(slice, (0, 5).into(), false), 5); + assert_eq!(pos_at_coords(slice, (0, 6).into(), false), 6); + assert_eq!(pos_at_coords(slice, (0, 6).into(), true), 5); assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 6); - // test with grapheme clusters + // Test with grapheme clusters. let text = Rope::from("a̐éö̲\r\n"); let slice = text.slice(..); assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); @@ -197,15 +197,24 @@ mod test { assert_eq!(pos_at_coords(slice, (0, 4).into(), true), 7); assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 9); + // Test with wide-character grapheme clusters. + // TODO: account for character width. let text = Rope::from("किमपि"); // 2 - 1 - 2 codepoints // TODO: delete handling as per https://news.ycombinator.com/item?id=20058454 let slice = text.slice(..); assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); - assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 2); + assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 3); + assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 5); + assert_eq!(pos_at_coords(slice, (0, 3).into(), true), 5); + + // Test with tabs. + // Todo: account for tab stops. + let text = Rope::from("\tHello\n"); + let slice = text.slice(..); + assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 1); assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 2); - assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 3); - assert_eq!(pos_at_coords(slice, (0, 4).into(), false), 3); - assert_eq!(pos_at_coords(slice, (0, 5).into(), false), 5); // eol } }