c848ed7 · helix · git.iko.soy

git.iko.soy/helix

on master help sign in

Fixes for misc bugs with view movement.

c848ed7Nathan Vegdahl5 years ago

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

compare with master

unifiedsplit

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

7 coords_at_pos,

8 graphemes::{

9 next_grapheme_boundary, nth_next_grapheme_boundary, nth_prev_grapheme_boundary,

10− prev_grapheme_boundary, RopeGraphemes,

10+ prev_grapheme_boundary,

11 },

12− line_ending::line_without_line_ending,

13 pos_at_coords, Position, Range, RopeSlice,

12 pos_at_coords, Position, Range, RopeSlice,

14};

13};

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

85 } else {

84 } else {

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

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

87 };

86 };

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);

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

90−

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

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

92 };

89 };

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

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

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

365 fn test_vertical_move() {

362 fn test_vertical_move() {

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

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

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

364 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);

369

366

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

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

371 assert_eq!(

368 assert_eq!(

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

381 fn horizontal_moves_through_single_line_text() {

378 fn horizontal_moves_through_single_line_text() {

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

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

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

380 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);

385

382

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

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

387

384

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

404 fn horizontal_moves_through_multiline_text() {

401 fn horizontal_moves_through_multiline_text() {

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

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

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

403 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);

408

405

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

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

410

407

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

431 fn selection_extending_moves_in_single_line_text() {

428 fn selection_extending_moves_in_single_line_text() {

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

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

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

430 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);

435

432

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

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

437 let original_anchor = range.anchor;

434 let original_anchor = range.anchor;

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

452 fn vertical_moves_in_single_column() {

449 fn vertical_moves_in_single_column() {

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

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

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

451 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);

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

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

457 let moves_and_expected_coordinates = IntoIter::new([

454 let moves_and_expected_coordinates = IntoIter::new([

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

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

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

477 fn vertical_moves_jumping_column() {

474 fn vertical_moves_jumping_column() {

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

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

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

476 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);

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

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

482

479

483 enum Axis {

480 enum Axis {

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

510 }

507 }

511

508

512 #[test]

509 #[test]

513− fn multibyte_character_column_jumps() {

510+ fn multibyte_character_wide_column_jumps() {

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

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

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

512 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);

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

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

518

515

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

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

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

524 V,

521 V,

525 }

522 }

526 let moves_and_expected_coordinates = IntoIter::new([

523 let moves_and_expected_coordinates = IntoIter::new([

527− // Places cursor at the fourth kana

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

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

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

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

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

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

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

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)),

531 ]);

534 ]);

532

535

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

536 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;

1use crate::{

3use crate::{

2 chars::char_is_line_ending,

4 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,

4 RopeSlice,

7 RopeSlice,

5};

8};

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

52 }

55 }

53}

56}

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

57/// 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.

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

64pub 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.

58 let line = text.char_to_line(pos);

65 let line = text.char_to_line(pos);

66+

59 let line_start = text.line_to_char(line);

67 let line_start = text.line_to_char(line);

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

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

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+

61 Position::new(line, col)

76 Position::new(line, col)

62}

77}

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

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

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.

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

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

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

67− // visual horizontal position, not graphemes.

68 let Position { row, col } = coords;

87 let Position { row, col } = coords;

69 let line_start = text.line_to_char(row);

88 let line_start = text.line_to_char(row);

70− // line_start + col

89+ let line_end = if is_1_width {

71− nth_next_grapheme_boundary(text, line_start, col)

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

72}

110}

111

74#[cfg(test)]

112#[cfg(test)]

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

122

85 #[test]

123 #[test]

86 fn test_coords_at_pos() {

124 fn test_coords_at_pos() {

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

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

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

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

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

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

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

128+ 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

129+ 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

130+ 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

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());

143

95 // test with grapheme clusters

144 // test with grapheme clusters

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

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

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

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

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

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

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

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

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

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

102

152

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

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

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

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

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

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

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

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

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

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

108− assert_eq!(coords_at_pos(slice, 5), (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());

109 }

160 }

110

161

111 #[test]

162 #[test]

112 fn test_pos_at_coords() {

163 fn test_pos_at_coords() {

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

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

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

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

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

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

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

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

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

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

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

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

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

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);

120

188

121 // test with grapheme clusters

189 // test with grapheme clusters

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

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

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

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

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

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

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

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

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

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

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

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

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

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+

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

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

130 // 2 - 1 - 2 codepoints

201 // 2 - 1 - 2 codepoints

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

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

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

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

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

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

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

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

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

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

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

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

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

137 }

210 }

138}

211}

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

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

286

287 // For 1-width cursor semantics.

288 if range.anchor < range.head {

289− prev_grapheme_boundary(text, range.head)

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

290 } else {

291 range.head

292 }

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

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

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

918

919 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

921

922 // TODO: only manipulate main selection

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

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

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

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

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

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

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

101 // scroll down

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

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

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

104 // scroll up

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