aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--helix-core/src/movement.rs39
-rw-r--r--helix-core/src/position.rs141
-rw-r--r--helix-core/src/selection.rs2
-rw-r--r--helix-term/src/commands.rs2
-rw-r--r--helix-view/src/view.rs4
5 files changed, 132 insertions, 56 deletions
diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs
index 21e97ae8..01bec47a 100644
--- a/helix-core/src/movement.rs
+++ b/helix-core/src/movement.rs
@@ -7,9 +7,8 @@ use crate::{
coords_at_pos,
graphemes::{
next_grapheme_boundary, nth_next_grapheme_boundary, nth_prev_grapheme_boundary,
- prev_grapheme_boundary, RopeGraphemes,
+ prev_grapheme_boundary,
},
- line_ending::line_without_line_ending,
pos_at_coords, Position, Range, RopeSlice,
};
@@ -85,10 +84,8 @@ pub fn move_vertically(
} else {
(row + count).min(slice.len_lines().saturating_sub(1))
};
- let max_col = RopeGraphemes::new(line_without_line_ending(&slice, new_row)).count();
- let new_col = col.max(horiz as usize).min(max_col);
-
- pos_at_coords(slice, Position::new(new_row, new_col))
+ let new_col = col.max(horiz as usize);
+ pos_at_coords(slice, Position::new(new_row, new_col), true)
};
// Compute the new range according to the type of movement.
@@ -365,7 +362,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());
+ let pos = pos_at_coords(slice, (0, 4).into(), true);
let range = Range::new(pos, pos);
assert_eq!(
@@ -381,7 +378,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());
+ let position = pos_at_coords(slice, (0, 0).into(), true);
let mut range = Range::point(position);
@@ -404,7 +401,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());
+ let position = pos_at_coords(slice, (0, 0).into(), true);
let mut range = Range::point(position);
@@ -431,7 +428,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());
+ let position = pos_at_coords(slice, (0, 0).into(), true);
let mut range = Range::point(position);
let original_anchor = range.anchor;
@@ -452,7 +449,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());
+ let position = pos_at_coords(slice, (0, 0).into(), true);
let mut range = Range::point(position);
let moves_and_expected_coordinates = IntoIter::new([
((Direction::Forward, 1usize), (1, 0)),
@@ -477,7 +474,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());
+ let position = pos_at_coords(slice, (0, 0).into(), true);
let mut range = Range::point(position);
enum Axis {
@@ -510,10 +507,10 @@ mod test {
}
#[test]
- fn multibyte_character_column_jumps() {
+ 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());
+ 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
@@ -524,10 +521,16 @@ mod test {
V,
}
let moves_and_expected_coordinates = IntoIter::new([
- // Places cursor at the fourth kana
- ((Axis::H, Direction::Forward, 4), (0, 4)),
- // Descent places cursor at the fourth character.
- ((Axis::V, Direction::Forward, 1usize), (1, 4)),
+ // 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)),
]);
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 c4e8c9d6..15627687 100644
--- a/helix-core/src/position.rs
+++ b/helix-core/src/position.rs
@@ -1,6 +1,9 @@
+use std::borrow::Cow;
+
use crate::{
chars::char_is_line_ending,
- graphemes::{nth_next_grapheme_boundary, RopeGraphemes},
+ graphemes::{ensure_grapheme_boundary_prev, grapheme_width, RopeGraphemes},
+ line_ending::line_end_char_index,
RopeSlice,
};
@@ -52,23 +55,58 @@ 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.
pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position {
- // TODO: this isn't correct. This needs to work in terms of
- // visual horizontal position, not graphemes.
let line = text.char_to_line(pos);
+
let line_start = text.line_to_char(line);
- let col = RopeGraphemes::new(text.slice(line_start..pos)).count();
+ let pos = ensure_grapheme_boundary_prev(text, pos);
+ let col = RopeGraphemes::new(text.slice(line_start..pos))
+ .map(|g| {
+ let g: Cow<str> = g.into();
+ grapheme_width(&g)
+ })
+ .sum();
+
Position::new(line, col)
}
/// Convert (line, column) coordinates to a character index.
-pub fn pos_at_coords(text: RopeSlice, coords: Position) -> usize {
- // TODO: this isn't correct. This needs to work in terms of
- // visual horizontal position, not graphemes.
+///
+/// `is_1_width` specifies whether the position should be treated
+/// as a block cursor or not. This effects how line-ends are handled.
+/// `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.
+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);
- // line_start + col
- nth_next_grapheme_boundary(text, line_start, col)
+ let line_end = if is_1_width {
+ line_end_char_index(&text, row)
+ } else {
+ 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<str> = g.into();
+ let next_col = prev_col + grapheme_width(&g);
+
+ if next_col > col {
+ break;
+ }
+
+ prev_col = next_col;
+ col_char_offset += g.chars().count();
+ }
+
+ line_start + col_char_offset
}
#[cfg(test)]
@@ -84,13 +122,24 @@ mod test {
#[test]
fn test_coords_at_pos() {
- // 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, 5), (0, 5).into()); // position on \n
- // assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); // position on w
- // assert_eq!(coords_at_pos(slice, 7), (1, 1).into()); // position on o
- // assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d
+ 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, 5), (0, 5).into()); // position on \n
+ assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); // position on w
+ assert_eq!(coords_at_pos(slice, 7), (1, 1).into()); // position on o
+ assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d
+
+ // Test with wide characters.
+ 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, 6), (1, 0).into());
// test with grapheme clusters
let text = Rope::from("a̐éö̲\r\n");
@@ -99,40 +148,64 @@ mod test {
assert_eq!(coords_at_pos(slice, 2), (0, 1).into());
assert_eq!(coords_at_pos(slice, 4), (0, 2).into());
assert_eq!(coords_at_pos(slice, 7), (0, 3).into());
+ assert_eq!(coords_at_pos(slice, 9), (1, 0).into());
- let text = Rope::from("किमपि");
+ 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, 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, 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, 6), (1, 0).into());
}
#[test]
fn test_pos_at_coords() {
let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ");
let slice = text.slice(..);
- assert_eq!(pos_at_coords(slice, (0, 0).into()), 0);
- assert_eq!(pos_at_coords(slice, (0, 5).into()), 5); // position on \n
- assert_eq!(pos_at_coords(slice, (1, 0).into()), 6); // position on w
- assert_eq!(pos_at_coords(slice, (1, 1).into()), 7); // position on o
- assert_eq!(pos_at_coords(slice, (1, 4).into()), 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.
+ 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, (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()), 0);
- assert_eq!(pos_at_coords(slice, (0, 1).into()), 2);
- assert_eq!(pos_at_coords(slice, (0, 2).into()), 4);
- assert_eq!(pos_at_coords(slice, (0, 3).into()), 7); // \r\n is one char here
- assert_eq!(pos_at_coords(slice, (0, 4).into()), 9);
+ 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);
+
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()), 0);
- assert_eq!(pos_at_coords(slice, (0, 1).into()), 2);
- assert_eq!(pos_at_coords(slice, (0, 2).into()), 3);
- assert_eq!(pos_at_coords(slice, (0, 3).into()), 5); // eol
+ 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), 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
}
}
diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs
index 074b6199..beade27a 100644
--- a/helix-core/src/selection.rs
+++ b/helix-core/src/selection.rs
@@ -286,7 +286,7 @@ impl Selection {
// For 1-width cursor semantics.
if range.anchor < range.head {
- prev_grapheme_boundary(text, range.head)
+ prev_grapheme_boundary(text, range.head).max(range.anchor)
} else {
range.head
}
diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs
index 72462741..0ea78f6b 100644
--- a/helix-term/src/commands.rs
+++ b/helix-term/src/commands.rs
@@ -917,7 +917,7 @@ fn scroll(cx: &mut Context, offset: usize, direction: Direction) {
.min(last_line.saturating_sub(scrolloff));
let text = doc.text().slice(..);
- let pos = pos_at_coords(text, Position::new(line, cursor.col)); // this func will properly truncate to line end
+ let pos = pos_at_coords(text, Position::new(line, cursor.col), true); // this func will properly truncate to line end
// TODO: only manipulate main selection
doc.set_selection(view.id, Selection::point(pos));
diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs
index 67585ed3..66214691 100644
--- a/helix-view/src/view.rs
+++ b/helix-view/src/view.rs
@@ -97,9 +97,9 @@ impl View {
const OFFSET: usize = 7; // 1 diagnostic + 5 linenr + 1 gutter
let last_col = self.first_col + (self.area.width as usize - OFFSET);
- if line > last_line.saturating_sub(scrolloff) {
+ if line > last_line.saturating_sub(scrolloff + 1) {
// scroll down
- self.first_line += line - (last_line.saturating_sub(scrolloff));
+ self.first_line += line - (last_line.saturating_sub(scrolloff + 1));
} else if line < self.first_line + scrolloff {
// scroll up
self.first_line = line.saturating_sub(scrolloff);