aboutsummaryrefslogtreecommitdiff
path: root/helix-core/src
diff options
context:
space:
mode:
authorMatthew Toohey2022-06-21 16:36:36 +0000
committerGitHub2022-06-21 16:36:36 +0000
commit6a3f7f2c399f0b92cef97b0c85ebe976fd7cfcac (patch)
treec350427ff1949f6e95464227ca0583bce8400c70 /helix-core/src
parentfa4934cff9aa5b86b907e218313a7b370962ae67 (diff)
feat: make `move_vertically` aware of tabs and wide characters (#2620)
* feat: make `move_vertically` aware of tabs and wide characters * refactor: replace unnecessary checked_sub with comparison * refactor: leave pos_at_coords unchanged and introduce separate pos_at_visual_coords * style: include comment to explain `pos_at_visual_coords` breaking condition * refactor: use `pos_at_visual_coords` in `text_pos_at_screen_coords` * feat: make `copy_selection_on_line` aware of wide characters
Diffstat (limited to 'helix-core/src')
-rw-r--r--helix-core/src/lib.rs4
-rw-r--r--helix-core/src/movement.rs43
-rw-r--r--helix-core/src/position.rs106
3 files changed, 127 insertions, 26 deletions
diff --git a/helix-core/src/lib.rs b/helix-core/src/lib.rs
index 627b73bb..735a62c1 100644
--- a/helix-core/src/lib.rs
+++ b/helix-core/src/lib.rs
@@ -63,7 +63,9 @@ pub type Tendril = SmartString<smartstring::LazyCompact>;
pub use {regex, tree_sitter};
pub use graphemes::RopeGraphemes;
-pub use position::{coords_at_pos, pos_at_coords, visual_coords_at_pos, Position};
+pub use position::{
+ coords_at_pos, pos_at_coords, pos_at_visual_coords, visual_coords_at_pos, Position,
+};
pub use selection::{Range, Selection};
pub use smallvec::{smallvec, SmallVec};
pub use syntax::Syntax;
diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs
index f60b3c83..2155f77a 100644
--- a/helix-core/src/movement.rs
+++ b/helix-core/src/movement.rs
@@ -5,16 +5,15 @@ use tree_sitter::{Node, QueryCursor};
use crate::{
chars::{categorize_char, char_is_line_ending, CharCategory},
- coords_at_pos,
graphemes::{
next_grapheme_boundary, nth_next_grapheme_boundary, nth_prev_grapheme_boundary,
prev_grapheme_boundary,
},
line_ending::rope_is_line_ending,
- pos_at_coords,
+ pos_at_visual_coords,
syntax::LanguageConfiguration,
textobject::TextObject,
- Position, Range, RopeSlice,
+ visual_coords_at_pos, Position, Range, RopeSlice,
};
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
@@ -35,6 +34,7 @@ pub fn move_horizontally(
dir: Direction,
count: usize,
behaviour: Movement,
+ _: usize,
) -> Range {
let pos = range.cursor(slice);
@@ -54,15 +54,12 @@ pub fn move_vertically(
dir: Direction,
count: usize,
behaviour: Movement,
+ tab_width: usize,
) -> Range {
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 Position { row, col } = visual_coords_at_pos(slice, pos, tab_width);
let horiz = range.horiz.unwrap_or(col as u32);
// Compute the new position.
@@ -71,7 +68,7 @@ pub fn move_vertically(
Direction::Backward => row.saturating_sub(count),
};
let new_col = col.max(horiz as usize);
- let new_pos = pos_at_coords(slice, Position::new(new_row, new_col), true);
+ let new_pos = pos_at_visual_coords(slice, Position::new(new_row, new_col), tab_width);
// 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 {
@@ -446,6 +443,8 @@ pub fn goto_treesitter_object(
mod test {
use ropey::Rope;
+ use crate::{coords_at_pos, pos_at_coords};
+
use super::*;
const SINGLE_LINE_SAMPLE: &str = "This is a simple alphabetic line";
@@ -472,7 +471,7 @@ mod test {
assert_eq!(
coords_at_pos(
slice,
- move_vertically(slice, range, Direction::Forward, 1, Movement::Move).head
+ move_vertically(slice, range, Direction::Forward, 1, Movement::Move, 4).head
),
(1, 3).into()
);
@@ -496,7 +495,7 @@ mod test {
];
for ((direction, amount), coordinates) in moves_and_expected_coordinates {
- range = move_horizontally(slice, range, direction, amount, Movement::Move);
+ range = move_horizontally(slice, range, direction, amount, Movement::Move, 0);
assert_eq!(coords_at_pos(slice, range.head), coordinates.into())
}
}
@@ -522,7 +521,7 @@ mod test {
];
for ((direction, amount), coordinates) in moves_and_expected_coordinates {
- range = move_horizontally(slice, range, direction, amount, Movement::Move);
+ range = move_horizontally(slice, range, direction, amount, Movement::Move, 0);
assert_eq!(coords_at_pos(slice, range.head), coordinates.into());
assert_eq!(range.head, range.anchor);
}
@@ -544,7 +543,7 @@ mod test {
];
for (direction, amount) in moves {
- range = move_horizontally(slice, range, direction, amount, Movement::Extend);
+ range = move_horizontally(slice, range, direction, amount, Movement::Extend, 0);
assert_eq!(range.anchor, original_anchor);
}
}
@@ -568,7 +567,7 @@ mod test {
];
for ((direction, amount), coordinates) in moves_and_expected_coordinates {
- range = move_vertically(slice, range, direction, amount, Movement::Move);
+ range = move_vertically(slice, range, direction, amount, Movement::Move, 4);
assert_eq!(coords_at_pos(slice, range.head), coordinates.into());
assert_eq!(range.head, range.anchor);
}
@@ -602,8 +601,8 @@ mod test {
for ((axis, direction, amount), coordinates) in moves_and_expected_coordinates {
range = match axis {
- Axis::H => move_horizontally(slice, range, direction, amount, Movement::Move),
- Axis::V => move_vertically(slice, range, direction, amount, Movement::Move),
+ Axis::H => move_horizontally(slice, range, direction, amount, Movement::Move, 0),
+ Axis::V => move_vertically(slice, range, direction, amount, Movement::Move, 4),
};
assert_eq!(coords_at_pos(slice, range.head), coordinates.into());
assert_eq!(range.head, range.anchor);
@@ -627,18 +626,18 @@ mod test {
let moves_and_expected_coordinates = [
// 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)),
+ // Descent places cursor at the 8th character.
+ ((Axis::V, Direction::Forward, 1usize), (1, 8)),
+ // Moving back 2 characters.
+ ((Axis::H, Direction::Backward, 2usize), (1, 6)),
// Jumping back up 1 line.
((Axis::V, Direction::Backward, 1usize), (0, 3)),
];
for ((axis, direction, amount), coordinates) in moves_and_expected_coordinates {
range = match axis {
- Axis::H => move_horizontally(slice, range, direction, amount, Movement::Move),
- Axis::V => move_vertically(slice, range, direction, amount, Movement::Move),
+ Axis::H => move_horizontally(slice, range, direction, amount, Movement::Move, 0),
+ Axis::V => move_vertically(slice, range, direction, amount, Movement::Move, 4),
};
assert_eq!(coords_at_pos(slice, range.head), coordinates.into());
assert_eq!(range.head, range.anchor);
diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs
index ce37300a..f456eb98 100644
--- a/helix-core/src/position.rs
+++ b/helix-core/src/position.rs
@@ -109,9 +109,6 @@ pub fn visual_coords_at_pos(text: RopeSlice, pos: usize, tab_width: usize) -> Po
/// with left-side block-cursor positions, as this prevents the the block cursor
/// from jumping to the next line. Otherwise you typically want it to be `false`,
/// such as when dealing with raw anchor/head positions.
-///
-/// TODO: this should be changed to work in terms of visual row/column, not
-/// graphemes.
pub fn pos_at_coords(text: RopeSlice, coords: Position, limit_before_line_ending: bool) -> usize {
let Position { mut row, col } = coords;
if limit_before_line_ending {
@@ -135,6 +132,43 @@ pub fn pos_at_coords(text: RopeSlice, coords: Position, limit_before_line_ending
line_start + col_char_offset
}
+/// Convert visual (line, column) coordinates to a character index.
+///
+/// If the `line` coordinate is beyond the end of the file, the EOF
+/// position will be returned.
+///
+/// If the `column` coordinate is past the end of the given line, the
+/// line-end position (in this case, just before the line ending
+/// character) will be returned.
+pub fn pos_at_visual_coords(text: RopeSlice, coords: Position, tab_width: usize) -> usize {
+ let Position { mut row, col } = coords;
+ row = row.min(text.len_lines() - 1);
+ let line_start = text.line_to_char(row);
+ let line_end = line_end_char_index(&text, row);
+
+ let mut col_char_offset = 0;
+ let mut cols_remaining = col;
+ for grapheme in RopeGraphemes::new(text.slice(line_start..line_end)) {
+ let grapheme_width = if grapheme == "\t" {
+ tab_width - ((col - cols_remaining) % tab_width)
+ } else {
+ let grapheme = Cow::from(grapheme);
+ grapheme_width(&grapheme)
+ };
+
+ // If pos is in the middle of a wider grapheme (tab for example)
+ // return the starting offset.
+ if grapheme_width > cols_remaining {
+ break;
+ }
+
+ cols_remaining -= grapheme_width;
+ col_char_offset += grapheme.chars().count();
+ }
+
+ line_start + col_char_offset
+}
+
#[cfg(test)]
mod test {
use super::*;
@@ -305,4 +339,70 @@ mod test {
assert_eq!(pos_at_coords(slice, (0, 10).into(), true), 0);
assert_eq!(pos_at_coords(slice, (10, 10).into(), true), 0);
}
+
+ #[test]
+ fn test_pos_at_visual_coords() {
+ let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ");
+ let slice = text.slice(..);
+ assert_eq!(pos_at_visual_coords(slice, (0, 0).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (0, 5).into(), 4), 5); // position on \n
+ assert_eq!(pos_at_visual_coords(slice, (0, 6).into(), 4), 5); // position after \n
+ assert_eq!(pos_at_visual_coords(slice, (1, 0).into(), 4), 6); // position on w
+ assert_eq!(pos_at_visual_coords(slice, (1, 1).into(), 4), 7); // position on o
+ assert_eq!(pos_at_visual_coords(slice, (1, 4).into(), 4), 10); // position on d
+
+ // Test with wide characters.
+ let text = Rope::from("今日はいい\n");
+ let slice = text.slice(..);
+ assert_eq!(pos_at_visual_coords(slice, (0, 0).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (0, 1).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (0, 2).into(), 4), 1);
+ assert_eq!(pos_at_visual_coords(slice, (0, 3).into(), 4), 1);
+ assert_eq!(pos_at_visual_coords(slice, (0, 4).into(), 4), 2);
+ assert_eq!(pos_at_visual_coords(slice, (0, 5).into(), 4), 2);
+ assert_eq!(pos_at_visual_coords(slice, (0, 6).into(), 4), 3);
+ assert_eq!(pos_at_visual_coords(slice, (0, 7).into(), 4), 3);
+ assert_eq!(pos_at_visual_coords(slice, (0, 8).into(), 4), 4);
+ assert_eq!(pos_at_visual_coords(slice, (0, 9).into(), 4), 4);
+ // assert_eq!(pos_at_visual_coords(slice, (0, 10).into(), 4, false), 5);
+ // assert_eq!(pos_at_visual_coords(slice, (0, 10).into(), 4, true), 5);
+ assert_eq!(pos_at_visual_coords(slice, (1, 0).into(), 4), 6);
+
+ // Test with grapheme clusters.
+ let text = Rope::from("a̐éö̲\r\n");
+ let slice = text.slice(..);
+ assert_eq!(pos_at_visual_coords(slice, (0, 0).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (0, 1).into(), 4), 2);
+ assert_eq!(pos_at_visual_coords(slice, (0, 2).into(), 4), 4);
+ assert_eq!(pos_at_visual_coords(slice, (0, 3).into(), 4), 7); // \r\n is one char here
+ assert_eq!(pos_at_visual_coords(slice, (0, 4).into(), 4), 7);
+ assert_eq!(pos_at_visual_coords(slice, (1, 0).into(), 4), 9);
+
+ // Test with wide-character grapheme clusters.
+ 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_visual_coords(slice, (0, 0).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (0, 1).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (0, 2).into(), 4), 2);
+ assert_eq!(pos_at_visual_coords(slice, (0, 3).into(), 4), 3);
+
+ // Test with tabs.
+ let text = Rope::from("\tHello\n");
+ let slice = text.slice(..);
+ assert_eq!(pos_at_visual_coords(slice, (0, 0).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (0, 1).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (0, 2).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (0, 3).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (0, 4).into(), 4), 1);
+ assert_eq!(pos_at_visual_coords(slice, (0, 5).into(), 4), 2);
+
+ // Test out of bounds.
+ let text = Rope::new();
+ let slice = text.slice(..);
+ assert_eq!(pos_at_visual_coords(slice, (10, 0).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (0, 10).into(), 4), 0);
+ assert_eq!(pos_at_visual_coords(slice, (10, 10).into(), 4), 0);
+ }
}