From 33e8fa765b003016aa2129166b8d72f4431140ac Mon Sep 17 00:00:00 2001 From: Matthew Toohey Date: Sun, 5 Jun 2022 13:08:12 -0400 Subject: [PATCH] refactor: leave pos_at_coords unchanged and introduce separate pos_at_visual_coords --- helix-core/src/lib.rs | 4 +- helix-core/src/movement.rs | 20 ++-- helix-core/src/position.rs | 178 ++++++++++++++++++++++--------- helix-term/src/application.rs | 7 +- helix-term/src/commands.rs | 16 +-- helix-term/src/commands/typed.rs | 7 +- 6 files changed, 148 insertions(+), 84 deletions(-) diff --git a/helix-core/src/lib.rs b/helix-core/src/lib.rs index 7d857b0f03ec..1faa2acd4557 100644 --- a/helix-core/src/lib.rs +++ b/helix-core/src/lib.rs @@ -63,7 +63,9 @@ pub type Tendril = SmartString; 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 6d4da9aa2349..2155f77a42a4 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -10,7 +10,7 @@ use crate::{ prev_grapheme_boundary, }, line_ending::rope_is_line_ending, - pos_at_coords, + pos_at_visual_coords, syntax::LanguageConfiguration, textobject::TextObject, visual_coords_at_pos, Position, Range, RopeSlice, @@ -68,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), tab_width, 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 { @@ -443,7 +443,7 @@ pub fn goto_treesitter_object( mod test { use ropey::Rope; - use crate::coords_at_pos; + use crate::{coords_at_pos, pos_at_coords}; use super::*; @@ -465,7 +465,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(), 4, true); + let pos = pos_at_coords(slice, (0, 4).into(), true); let range = Range::new(pos, pos); assert_eq!( @@ -481,7 +481,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(), 4, true); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); @@ -504,7 +504,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(), 4, true); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); @@ -531,7 +531,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(), 4, true); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); let original_anchor = range.anchor; @@ -552,7 +552,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(), 4, true); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); let moves_and_expected_coordinates = [ ((Direction::Forward, 1usize), (1, 0)), @@ -577,7 +577,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(), 4, true); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); enum Axis { @@ -613,7 +613,7 @@ mod test { 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(), 4, true); + 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 diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs index 1fa96e6893ce..01aeb9d24dd2 100644 --- a/helix-core/src/position.rs +++ b/helix-core/src/position.rs @@ -109,12 +109,7 @@ 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. -pub fn pos_at_coords( - text: RopeSlice, - coords: Position, - tab_width: usize, - limit_before_line_ending: bool, -) -> usize { +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 { row = row.min(text.len_lines() - 1); @@ -126,11 +121,36 @@ pub fn pos_at_coords( 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 +} + +/// 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 + tab_width - ((col - cols_remaining) % tab_width) } else { let grapheme = Cow::from(grapheme); grapheme_width(&grapheme) @@ -257,68 +277,130 @@ mod test { fn test_pos_at_coords() { let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ"); let slice = text.slice(..); - assert_eq!(pos_at_coords(slice, (0, 0).into(), 4, false), 0); - assert_eq!(pos_at_coords(slice, (0, 5).into(), 4, false), 5); // position on \n - assert_eq!(pos_at_coords(slice, (0, 6).into(), 4, false), 6); // position after \n - assert_eq!(pos_at_coords(slice, (0, 6).into(), 4, true), 5); // position after \n - assert_eq!(pos_at_coords(slice, (1, 0).into(), 4, false), 6); // position on w - assert_eq!(pos_at_coords(slice, (1, 1).into(), 4, false), 7); // position on o - assert_eq!(pos_at_coords(slice, (1, 4).into(), 4, false), 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. + // 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(), 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); + + // 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), 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); + + // Test out of bounds. + let text = Rope::new(); + let slice = text.slice(..); + assert_eq!(pos_at_coords(slice, (10, 0).into(), true), 0); + 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_coords(slice, (0, 0).into(), 4, false), 0); - assert_eq!(pos_at_coords(slice, (0, 1).into(), 4, false), 0); - assert_eq!(pos_at_coords(slice, (0, 2).into(), 4, false), 1); - assert_eq!(pos_at_coords(slice, (0, 3).into(), 4, false), 1); - assert_eq!(pos_at_coords(slice, (0, 4).into(), 4, false), 2); - assert_eq!(pos_at_coords(slice, (0, 5).into(), 4, false), 2); - assert_eq!(pos_at_coords(slice, (0, 6).into(), 4, false), 3); - assert_eq!(pos_at_coords(slice, (0, 7).into(), 4, false), 3); - assert_eq!(pos_at_coords(slice, (0, 8).into(), 4, false), 4); - assert_eq!(pos_at_coords(slice, (0, 9).into(), 4, false), 4); - assert_eq!(pos_at_coords(slice, (0, 10).into(), 4, false), 5); - assert_eq!(pos_at_coords(slice, (0, 10).into(), 4, true), 5); - assert_eq!(pos_at_coords(slice, (1, 0).into(), 4, false), 6); + 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_coords(slice, (0, 0).into(), 4, false), 0); - assert_eq!(pos_at_coords(slice, (0, 1).into(), 4, false), 2); - assert_eq!(pos_at_coords(slice, (0, 2).into(), 4, false), 4); - assert_eq!(pos_at_coords(slice, (0, 3).into(), 4, false), 7); // \r\n is one char here - assert_eq!(pos_at_coords(slice, (0, 4).into(), 4, false), 9); - assert_eq!(pos_at_coords(slice, (0, 4).into(), 4, true), 7); - assert_eq!(pos_at_coords(slice, (1, 0).into(), 4, false), 9); + 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_coords(slice, (0, 0).into(), 4, false), 0); - assert_eq!(pos_at_coords(slice, (0, 1).into(), 4, false), 0); - assert_eq!(pos_at_coords(slice, (0, 2).into(), 4, false), 2); - assert_eq!(pos_at_coords(slice, (0, 3).into(), 4, false), 3); - assert_eq!(pos_at_coords(slice, (0, 3).into(), 4, true), 3); + 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_coords(slice, (0, 0).into(), 4, false), 0); - assert_eq!(pos_at_coords(slice, (0, 1).into(), 4, false), 0); - assert_eq!(pos_at_coords(slice, (0, 2).into(), 4, false), 0); - assert_eq!(pos_at_coords(slice, (0, 3).into(), 4, false), 0); - assert_eq!(pos_at_coords(slice, (0, 4).into(), 4, false), 1); - assert_eq!(pos_at_coords(slice, (0, 5).into(), 4, false), 2); + 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_coords(slice, (10, 0).into(), 4, true), 0); - assert_eq!(pos_at_coords(slice, (0, 10).into(), 4, true), 0); - assert_eq!(pos_at_coords(slice, (10, 10).into(), 4, true), 0); + 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); } } diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 5c55fb199de3..2dfccf0435ec 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -158,12 +158,7 @@ impl Application { // with Action::Load all documents have the same view let view_id = editor.tree.focus; let doc = editor.document_mut(doc_id).unwrap(); - let pos = Selection::point(pos_at_coords( - doc.text().slice(..), - pos, - doc.tab_width(), - true, - )); + let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); doc.set_selection(view_id, pos); } } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 694f86912e50..b9bda5628f1d 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1351,7 +1351,7 @@ pub fn scroll(cx: &mut Context, offset: usize, direction: Direction) { // If cursor needs moving, replace primary selection if line != cursor.row { - let head = pos_at_coords(text, Position::new(line, cursor.col), doc.tab_width(), true); // this func will properly truncate to line end + let head = pos_at_coords(text, Position::new(line, cursor.col), true); // this func will properly truncate to line end let anchor = if doc.mode == Mode::Select { range.anchor @@ -1442,18 +1442,8 @@ fn copy_selection_on_line(cx: &mut Context, direction: Direction) { break; } - let anchor = pos_at_coords( - text, - Position::new(anchor_row, anchor_pos.col), - doc.tab_width(), - true, - ); - let head = pos_at_coords( - text, - Position::new(head_row, head_pos.col), - doc.tab_width(), - true, - ); + let anchor = pos_at_coords(text, Position::new(anchor_row, anchor_pos.col), true); + let head = pos_at_coords(text, Position::new(head_row, head_pos.col), true); // skip lines that are too short if coords_at_pos(text, anchor).col == anchor_pos.col diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 404af0801e00..5121eaa183a0 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -48,12 +48,7 @@ fn open( let (path, pos) = args::parse_file(arg); let _ = cx.editor.open(path, Action::Replace)?; let (view, doc) = current!(cx.editor); - let pos = Selection::point(pos_at_coords( - doc.text().slice(..), - pos, - doc.tab_width(), - true, - )); + let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); doc.set_selection(view.id, pos); // does not affect opening a buffer without pos align_view(doc, view, Align::Center);