c848ed7 · helix · git.iko.soy

git.iko.soy/helix

Fixes for misc bugs with view movement.

commitc848ed7abc7605c1bb1c6c98dfac23cadcb9e439 authorNathan Vegdahl <cessen@cessen.com> dateTue 20 Jul 2021, 18:15 (5 years ago) parentd5534a6

5 files changed+132−56

compare with master

unifiedsplit

helix-core/src/movement.rs+21−18 helix-core/src/position.rs+107−34 helix-core/src/selection.rs+1−1 helix-term/src/commands.rs+1−1 helix-view/src/view.rs+2−2

helix-core/src/movement.rs+21−18

@@ -7,9 +7,8 @@ use crate::{

77 coords_at_pos,

88 graphemes::{

99 next_grapheme_boundary, nth_next_grapheme_boundary, nth_prev_grapheme_boundary,

10− prev_grapheme_boundary, RopeGraphemes,

10+ prev_grapheme_boundary,

1111 },

12− line_ending::line_without_line_ending,

1312 pos_at_coords, Position, Range, RopeSlice,

1413};

1514

@@ -85,10 +84,8 @@ pub fn move_vertically(

8584 } else {

8685 (row + count).min(slice.len_lines().saturating_sub(1))

8786 };

88− let max_col = RopeGraphemes::new(line_without_line_ending(&slice, new_row)).count();

89− let new_col = col.max(horiz as usize).min(max_col);

90−

91− pos_at_coords(slice, Position::new(new_row, new_col))

87+ let new_col = col.max(horiz as usize);

88+ pos_at_coords(slice, Position::new(new_row, new_col), true)

9289 };

9390

9491 // Compute the new range according to the type of movement.

@@ -365,7 +362,7 @@ mod test {

365362 fn test_vertical_move() {

366363 let text = Rope::from("abcd\nefg\nwrs");

367364 let slice = text.slice(..);

368− let pos = pos_at_coords(slice, (0, 4).into());

365+ let pos = pos_at_coords(slice, (0, 4).into(), true);

369366

370367 let range = Range::new(pos, pos);

371368 assert_eq!(

@@ -381,7 +378,7 @@ mod test {

381378 fn horizontal_moves_through_single_line_text() {

382379 let text = Rope::from(SINGLE_LINE_SAMPLE);

383380 let slice = text.slice(..);

384− let position = pos_at_coords(slice, (0, 0).into());

381+ let position = pos_at_coords(slice, (0, 0).into(), true);

385382

386383 let mut range = Range::point(position);

387384

@@ -404,7 +401,7 @@ mod test {

404401 fn horizontal_moves_through_multiline_text() {

405402 let text = Rope::from(MULTILINE_SAMPLE);

406403 let slice = text.slice(..);

407− let position = pos_at_coords(slice, (0, 0).into());

404+ let position = pos_at_coords(slice, (0, 0).into(), true);

408405

409406 let mut range = Range::point(position);

410407

@@ -431,7 +428,7 @@ mod test {

431428 fn selection_extending_moves_in_single_line_text() {

432429 let text = Rope::from(SINGLE_LINE_SAMPLE);

433430 let slice = text.slice(..);

434− let position = pos_at_coords(slice, (0, 0).into());

431+ let position = pos_at_coords(slice, (0, 0).into(), true);

435432

436433 let mut range = Range::point(position);

437434 let original_anchor = range.anchor;

@@ -452,7 +449,7 @@ mod test {

452449 fn vertical_moves_in_single_column() {

453450 let text = Rope::from(MULTILINE_SAMPLE);

454451 let slice = text.slice(..);

455− let position = pos_at_coords(slice, (0, 0).into());

452+ let position = pos_at_coords(slice, (0, 0).into(), true);

456453 let mut range = Range::point(position);

457454 let moves_and_expected_coordinates = IntoIter::new([

458455 ((Direction::Forward, 1usize), (1, 0)),

@@ -477,7 +474,7 @@ mod test {

477474 fn vertical_moves_jumping_column() {

478475 let text = Rope::from(MULTILINE_SAMPLE);

479476 let slice = text.slice(..);

480− let position = pos_at_coords(slice, (0, 0).into());

477+ let position = pos_at_coords(slice, (0, 0).into(), true);

481478 let mut range = Range::point(position);

482479

483480 enum Axis {

@@ -510,10 +507,10 @@ mod test {

510507 }

511508

512509 #[test]

513− fn multibyte_character_column_jumps() {

510+ fn multibyte_character_wide_column_jumps() {

514511 let text = Rope::from(MULTIBYTE_CHARACTER_SAMPLE);

515512 let slice = text.slice(..);

516− let position = pos_at_coords(slice, (0, 0).into());

513+ let position = pos_at_coords(slice, (0, 0).into(), true);

517514 let mut range = Range::point(position);

518515

519516 // FIXME: The behaviour captured in this test diverges from both Kakoune and Vim. These

@@ -524,10 +521,16 @@ mod test {

524521 V,

525522 }

526523 let moves_and_expected_coordinates = IntoIter::new([

527− // Places cursor at the fourth kana

528− ((Axis::H, Direction::Forward, 4), (0, 4)),

529− // Descent places cursor at the fourth character.

530− ((Axis::V, Direction::Forward, 1usize), (1, 4)),

524+ // Places cursor at the fourth kana (each of which are double-wide,

525+ // so the visual column is 8).

526+ ((Axis::H, Direction::Forward, 4), (0, 8)),

527+ // Descent places cursor at the 8th character.

528+ ((Axis::V, Direction::Forward, 1usize), (1, 8)),

529+ // Moving back a single-width character.

530+ ((Axis::H, Direction::Backward, 1usize), (1, 7)),

531+ // Jumping back up into the middle of a double-width character shifts

532+ // the column to the start of that character.

533+ ((Axis::V, Direction::Backward, 1usize), (0, 6)),

531534 ]);

532535

533536 for ((axis, direction, amount), coordinates) in moves_and_expected_coordinates {

helix-core/src/position.rs+107−34

@@ -1,6 +1,9 @@

1+use std::borrow::Cow;

13use crate::{

24 chars::char_is_line_ending,

3− graphemes::{nth_next_grapheme_boundary, RopeGraphemes},

5+ graphemes::{ensure_grapheme_boundary_prev, grapheme_width, RopeGraphemes},

6+ line_ending::line_end_char_index,

47 RopeSlice,

58};

@@ -52,23 +55,58 @@ impl From<Position> for tree_sitter::Point {

5255 }

5356}

5457/// Convert a character index to (line, column) coordinates.

58+///

59+/// TODO: this should be split into two methods: one for visual

60+/// row/column, and one for "objective" row/column (possibly with

61+/// the column specified in `char`s). The former would be used

62+/// for cursor movement, and the latter would be used for e.g. the

63+/// row:column display in the status line.

5564pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position {

56− // TODO: this isn't correct. This needs to work in terms of

57− // visual horizontal position, not graphemes.

5865 let line = text.char_to_line(pos);

66+

5967 let line_start = text.line_to_char(line);

60− let col = RopeGraphemes::new(text.slice(line_start..pos)).count();

68+ let pos = ensure_grapheme_boundary_prev(text, pos);

69+ let col = RopeGraphemes::new(text.slice(line_start..pos))

70+ .map(|g| {

71+ let g: Cow<str> = g.into();

72+ grapheme_width(&g)

73+ })

74+ .sum();

75+

6176 Position::new(line, col)

6277}

6378

6479/// Convert (line, column) coordinates to a character index.

65−pub fn pos_at_coords(text: RopeSlice, coords: Position) -> usize {

66− // TODO: this isn't correct. This needs to work in terms of

67− // visual horizontal position, not graphemes.

80+///

81+/// `is_1_width` specifies whether the position should be treated

82+/// as a block cursor or not. This effects how line-ends are handled.

83+/// `false` corresponds to properly round-tripping with `coords_at_pos()`,

84+/// whereas `true` will ensure that block cursors don't jump off the

85+/// end of the line.

86+pub fn pos_at_coords(text: RopeSlice, coords: Position, is_1_width: bool) -> usize {

6887 let Position { row, col } = coords;

6988 let line_start = text.line_to_char(row);

70− // line_start + col

71− nth_next_grapheme_boundary(text, line_start, col)

89+ let line_end = if is_1_width {

90+ line_end_char_index(&text, row)

91+ } else {

92+ text.line_to_char((row + 1).min(text.len_lines()))

93+ };

94+

95+ let mut prev_col = 0;

96+ let mut col_char_offset = 0;

97+ for g in RopeGraphemes::new(text.slice(line_start..line_end)) {

98+ let g: Cow<str> = g.into();

99+ let next_col = prev_col + grapheme_width(&g);

100+

101+ if next_col > col {

102+ break;

103+ }

104+

105+ prev_col = next_col;

106+ col_char_offset += g.chars().count();

107+ }

108+

109+ line_start + col_char_offset

72110}

73111

74112#[cfg(test)]

@@ -84,13 +122,24 @@ mod test {

84122

85123 #[test]

86124 fn test_coords_at_pos() {

87− // let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ");

88− // let slice = text.slice(..);

89− // assert_eq!(coords_at_pos(slice, 0), (0, 0).into());

90− // assert_eq!(coords_at_pos(slice, 5), (0, 5).into()); // position on \n

91− // assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); // position on w

92− // assert_eq!(coords_at_pos(slice, 7), (1, 1).into()); // position on o

93− // assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d

125+ let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ");

126+ let slice = text.slice(..);

127+ assert_eq!(coords_at_pos(slice, 0), (0, 0).into());

128+ assert_eq!(coords_at_pos(slice, 5), (0, 5).into()); // position on \n

129+ assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); // position on w

130+ assert_eq!(coords_at_pos(slice, 7), (1, 1).into()); // position on o

131+ assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d

132+

133+ // Test with wide characters.

134+ let text = Rope::from("今日はいい\n");

135+ let slice = text.slice(..);

136+ assert_eq!(coords_at_pos(slice, 0), (0, 0).into());

137+ assert_eq!(coords_at_pos(slice, 1), (0, 2).into());

138+ assert_eq!(coords_at_pos(slice, 2), (0, 4).into());

139+ assert_eq!(coords_at_pos(slice, 3), (0, 6).into());

140+ assert_eq!(coords_at_pos(slice, 4), (0, 8).into());

141+ assert_eq!(coords_at_pos(slice, 5), (0, 10).into());

142+ assert_eq!(coords_at_pos(slice, 6), (1, 0).into());

94143

95144 // test with grapheme clusters

96145 let text = Rope::from("a̐éö̲\r\n");

@@ -99,40 +148,64 @@ mod test {

99148 assert_eq!(coords_at_pos(slice, 2), (0, 1).into());

100149 assert_eq!(coords_at_pos(slice, 4), (0, 2).into());

101150 assert_eq!(coords_at_pos(slice, 7), (0, 3).into());

151+ assert_eq!(coords_at_pos(slice, 9), (1, 0).into());

102152

103− let text = Rope::from("किमपि");

153+ let text = Rope::from("किमपि\n");

104154 let slice = text.slice(..);

105155 assert_eq!(coords_at_pos(slice, 0), (0, 0).into());

106− assert_eq!(coords_at_pos(slice, 2), (0, 1).into());

107− assert_eq!(coords_at_pos(slice, 3), (0, 2).into());

108− assert_eq!(coords_at_pos(slice, 5), (0, 3).into());

156+ assert_eq!(coords_at_pos(slice, 2), (0, 2).into());

157+ assert_eq!(coords_at_pos(slice, 3), (0, 3).into());

158+ assert_eq!(coords_at_pos(slice, 5), (0, 5).into());

159+ assert_eq!(coords_at_pos(slice, 6), (1, 0).into());

109160 }

110161

111162 #[test]

112163 fn test_pos_at_coords() {

113164 let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ");

114165 let slice = text.slice(..);

115− assert_eq!(pos_at_coords(slice, (0, 0).into()), 0);

116− assert_eq!(pos_at_coords(slice, (0, 5).into()), 5); // position on \n

117− assert_eq!(pos_at_coords(slice, (1, 0).into()), 6); // position on w

118− assert_eq!(pos_at_coords(slice, (1, 1).into()), 7); // position on o

119− assert_eq!(pos_at_coords(slice, (1, 4).into()), 10); // position on d

166+ assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0);

167+ assert_eq!(pos_at_coords(slice, (0, 5).into(), false), 5); // position on \n

168+ assert_eq!(pos_at_coords(slice, (0, 6).into(), false), 6); // position after \n

169+ assert_eq!(pos_at_coords(slice, (0, 6).into(), true), 5); // position after \n

170+ assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 6); // position on w

171+ assert_eq!(pos_at_coords(slice, (1, 1).into(), false), 7); // position on o

172+ assert_eq!(pos_at_coords(slice, (1, 4).into(), false), 10); // position on d

173+

174+ // Test with wide characters.

175+ let text = Rope::from("今日はいい\n");

176+ let slice = text.slice(..);

177+ assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0);

178+ assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 0);

179+ assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 1);

180+ assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 1);

181+ assert_eq!(pos_at_coords(slice, (0, 4).into(), false), 2);

182+ assert_eq!(pos_at_coords(slice, (0, 6).into(), false), 3);

183+ assert_eq!(pos_at_coords(slice, (0, 8).into(), false), 4);

184+ assert_eq!(pos_at_coords(slice, (0, 10).into(), false), 5);

185+ assert_eq!(pos_at_coords(slice, (0, 11).into(), false), 6);

186+ assert_eq!(pos_at_coords(slice, (0, 11).into(), true), 5);

187+ assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 6);

120188

121189 // test with grapheme clusters

122190 let text = Rope::from("a̐éö̲\r\n");

123191 let slice = text.slice(..);

124− assert_eq!(pos_at_coords(slice, (0, 0).into()), 0);

125− assert_eq!(pos_at_coords(slice, (0, 1).into()), 2);

126− assert_eq!(pos_at_coords(slice, (0, 2).into()), 4);

127− assert_eq!(pos_at_coords(slice, (0, 3).into()), 7); // \r\n is one char here

128− assert_eq!(pos_at_coords(slice, (0, 4).into()), 9);

192+ assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0);

193+ assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 2);

194+ assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 4);

195+ assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 7); // \r\n is one char here

196+ assert_eq!(pos_at_coords(slice, (0, 4).into(), false), 9);

197+ assert_eq!(pos_at_coords(slice, (0, 4).into(), true), 7);

198+ assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 9);

199+

129200 let text = Rope::from("किमपि");

130201 // 2 - 1 - 2 codepoints

131202 // TODO: delete handling as per https://news.ycombinator.com/item?id=20058454

132203 let slice = text.slice(..);

133− assert_eq!(pos_at_coords(slice, (0, 0).into()), 0);

134− assert_eq!(pos_at_coords(slice, (0, 1).into()), 2);

135− assert_eq!(pos_at_coords(slice, (0, 2).into()), 3);

136− assert_eq!(pos_at_coords(slice, (0, 3).into()), 5); // eol

204+ assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0);

205+ assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 0);

206+ assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 2);

207+ assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 3);

208+ assert_eq!(pos_at_coords(slice, (0, 4).into(), false), 3);

209+ assert_eq!(pos_at_coords(slice, (0, 5).into(), false), 5); // eol

137210 }

138211}

helix-core/src/selection.rs+1−1

@@ -286,7 +286,7 @@ impl Selection {

286286

287287 // For 1-width cursor semantics.

288288 if range.anchor < range.head {

289− prev_grapheme_boundary(text, range.head)

289+ prev_grapheme_boundary(text, range.head).max(range.anchor)

290290 } else {

291291 range.head

292292 }

helix-term/src/commands.rs+1−1

@@ -917,7 +917,7 @@ fn scroll(cx: &mut Context, offset: usize, direction: Direction) {

917917 .min(last_line.saturating_sub(scrolloff));

918918

919919 let text = doc.text().slice(..);

920− let pos = pos_at_coords(text, Position::new(line, cursor.col)); // this func will properly truncate to line end

920+ let pos = pos_at_coords(text, Position::new(line, cursor.col), true); // this func will properly truncate to line end

921921

922922 // TODO: only manipulate main selection

923923 doc.set_selection(view.id, Selection::point(pos));

helix-view/src/view.rs+2−2

@@ -97,9 +97,9 @@ impl View {

9797 const OFFSET: usize = 7; // 1 diagnostic + 5 linenr + 1 gutter

9898 let last_col = self.first_col + (self.area.width as usize - OFFSET);

9999

100− if line > last_line.saturating_sub(scrolloff) {

100+ if line > last_line.saturating_sub(scrolloff + 1) {

101101 // scroll down

102− self.first_line += line - (last_line.saturating_sub(scrolloff));

102+ self.first_line += line - (last_line.saturating_sub(scrolloff + 1));

103103 } else if line < self.first_line + scrolloff {

104104 // scroll up

105105 self.first_line = line.saturating_sub(scrolloff);