diff options
| -rw-r--r-- | helix-core/src/lib.rs | 4 | ||||
| -rw-r--r-- | helix-core/src/movement.rs | 43 | ||||
| -rw-r--r-- | helix-core/src/position.rs | 106 | ||||
| -rw-r--r-- | helix-term/src/commands.rs | 26 | ||||
| -rw-r--r-- | helix-view/src/view.rs | 53 |
5 files changed, 153 insertions, 79 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); + } } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 046351a3..d7937ff5 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -16,14 +16,14 @@ use helix_core::{ line_ending::{get_line_ending_of_str, line_end_char_index, str_is_line_ending}, match_brackets, movement::{self, Direction}, - object, pos_at_coords, + object, pos_at_coords, pos_at_visual_coords, regex::{self, Regex, RegexBuilder}, search::{self, CharMatcher}, selection, shellwords, surround, textobject, tree_sitter::Node, unicode::width::UnicodeWidthChar, - LineEnding, Position, Range, Rope, RopeGraphemes, RopeSlice, Selection, SmallVec, Tendril, - Transaction, + visual_coords_at_pos, LineEnding, Position, Range, Rope, RopeGraphemes, RopeSlice, Selection, + SmallVec, Tendril, Transaction, }; use helix_view::{ clipboard::ClipboardType, @@ -511,7 +511,7 @@ fn no_op(_cx: &mut Context) {} fn move_impl<F>(cx: &mut Context, move_fn: F, dir: Direction, behaviour: Movement) where - F: Fn(RopeSlice, Range, Direction, usize, Movement) -> Range, + F: Fn(RopeSlice, Range, Direction, usize, Movement, usize) -> Range, { let count = cx.count(); let (view, doc) = current!(cx.editor); @@ -520,7 +520,7 @@ where let selection = doc .selection(view.id) .clone() - .transform(|range| move_fn(text, range, dir, count, behaviour)); + .transform(|range| move_fn(text, range, dir, count, behaviour, doc.tab_width())); doc.set_selection(view.id, selection); } @@ -1412,9 +1412,10 @@ fn copy_selection_on_line(cx: &mut Context, direction: Direction) { range.head }; - // TODO: this should use visual offsets / pos_at_screen_coords - let head_pos = coords_at_pos(text, head); - let anchor_pos = coords_at_pos(text, range.anchor); + let tab_width = doc.tab_width(); + + let head_pos = visual_coords_at_pos(text, head, tab_width); + let anchor_pos = visual_coords_at_pos(text, range.anchor, tab_width); let height = std::cmp::max(head_pos.row, anchor_pos.row) - std::cmp::min(head_pos.row, anchor_pos.row) @@ -1444,12 +1445,13 @@ fn copy_selection_on_line(cx: &mut Context, direction: Direction) { break; } - 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); + let anchor = + pos_at_visual_coords(text, Position::new(anchor_row, anchor_pos.col), tab_width); + let head = pos_at_visual_coords(text, Position::new(head_row, head_pos.col), tab_width); // skip lines that are too short - if coords_at_pos(text, anchor).col == anchor_pos.col - && coords_at_pos(text, head).col == head_pos.col + if visual_coords_at_pos(text, anchor, tab_width).col == anchor_pos.col + && visual_coords_at_pos(text, head, tab_width).col == head_pos.col { if is_primary { primary_index = ranges.len(); diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index a496fe33..bfae12a4 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -1,15 +1,9 @@ -use std::borrow::Cow; - use crate::{ graphics::Rect, gutter::{self, Gutter}, Document, DocumentId, ViewId, }; -use helix_core::{ - graphemes::{grapheme_width, RopeGraphemes}, - line_ending::line_end_char_index, - visual_coords_at_pos, Position, RopeSlice, Selection, -}; +use helix_core::{pos_at_visual_coords, visual_coords_at_pos, Position, RopeSlice, Selection}; use std::fmt; @@ -251,44 +245,21 @@ impl View { return None; } - let line_number = (row - inner.y) as usize + self.offset.row; - - if line_number > text.len_lines() - 1 { + let text_row = (row - inner.y) as usize + self.offset.row; + if text_row > text.len_lines() - 1 { return Some(text.len_chars()); } - let mut pos = text.line_to_char(line_number); - - let current_line = text.line(line_number); - - let target = (column - inner.x) as usize + self.offset.col; - let mut col = 0; - - // TODO: extract this part as pos_at_visual_coords - for grapheme in RopeGraphemes::new(current_line) { - if col >= target { - break; - } - - let width = if grapheme == "\t" { - tab_width - (col % 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 col + width > target { - break; - } + let text_col = (column - inner.x) as usize + self.offset.col; - col += width; - // TODO: use byte pos that converts back to char pos? - pos += grapheme.chars().count(); - } - - Some(pos.min(line_end_char_index(&text.slice(..), line_number))) + Some(pos_at_visual_coords( + *text, + Position { + row: text_row, + col: text_col, + }, + tab_width, + )) } /// Translates a screen position to position in the text document. |