dc53e65 · helix · git.iko.soy

git.iko.soy/helix

Fix surround cursor position calculation (#1183)

Fixes #1077. This was caused by the assumption that a block cursor is represented as zero width internally and simply rendered to be a single width selection, where as in reality a block cursor is an actual single width selection in form and function. Behavioural changes: 1. Surround selection no longer works when cursor is _on_ a surround character that has matching pairs (like `'` or `"`). This was the intended behaviour from the start but worked till now because of the cursor position calculation mismatch.

commitdc53e65b9e9be71c49eaa86e0f4dabb69f586e2e authorGokul Soumya <gokulps15@gmail.com> dateMon 29 Nov 2021, 07:03 (5 years ago) parent1d773bc

3 files changed+92−72

compare with master

unifiedsplit

helix-core/src/selection.rs+3−3 helix-core/src/surround.rs+83−65 helix-core/src/textobject.rs+6−4

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

@@ -308,10 +308,10 @@ impl Range {

308308}

309309

310310impl From<(usize, usize)> for Range {

311− fn from(tuple: (usize, usize)) -> Self {

311+ fn from((anchor, head): (usize, usize)) -> Self {

312312 Self {

313− anchor: tuple.0,

314− head: tuple.1,

313+ anchor,

314+ head,

315315 horiz: None,

316316 }

317317 }

helix-core/src/surround.rs+83−65

@@ -1,4 +1,4 @@

1−use crate::{search, Selection};

1+use crate::{search, Range, Selection};

22use ropey::RopeSlice;

44pub const PAIRS: &[(char, char)] = &[

@@ -35,33 +35,27 @@ pub fn get_pair(ch: char) -> (char, char) {

3535pub fn find_nth_pairs_pos(

3636 text: RopeSlice,

3737 ch: char,

38− pos: usize,

38+ range: Range,

3939 n: usize,

4040) -> Option<(usize, usize)> {

41− let (open, close) = get_pair(ch);

42−

43− if text.len_chars() < 2 || pos >= text.len_chars() {

41+ if text.len_chars() < 2 || range.to() >= text.len_chars() {

4442 return None;

4543 }

4644

45+ let (open, close) = get_pair(ch);

46+ let pos = range.cursor(text);

47+

4748 if open == close {

4849 if Some(open) == text.get_char(pos) {

49− // Special case: cursor is directly on a matching char.

50− match pos {

51− 0 => Some((pos, search::find_nth_next(text, close, pos + 1, n)?)),

52− _ if (pos + 1) == text.len_chars() => {

53− Some((search::find_nth_prev(text, open, pos, n)?, pos))

54− }

55− // We return no match because there's no way to know which

56− // side of the char we should be searching on.

57− _ => None,

58− }

59− } else {

60− Some((

61− search::find_nth_prev(text, open, pos, n)?,

62− search::find_nth_next(text, close, pos, n)?,

63− ))

50+ // Cursor is directly on match char. We return no match

51+ // because there's no way to know which side of the char

52+ // we should be searching on.

53+ return None;

6454 }

55+ Some((

56+ search::find_nth_prev(text, open, pos, n)?,

57+ search::find_nth_next(text, close, pos, n)?,

58+ ))

6559 } else {

6660 Some((

6761 find_nth_open_pair(text, open, close, pos, n)?,

@@ -160,8 +154,8 @@ pub fn get_surround_pos(

160154) -> Option<Vec<usize>> {

161155 let mut change_pos = Vec::new();

162156

163− for range in selection {

164− let (open_pos, close_pos) = find_nth_pairs_pos(text, ch, range.head, skip)?;

157+ for &range in selection {

158+ let (open_pos, close_pos) = find_nth_pairs_pos(text, ch, range, skip)?;

165159 if change_pos.contains(&open_pos) || change_pos.contains(&close_pos) {

166160 return None;

167161 }

@@ -178,67 +172,91 @@ mod test {

178172 use ropey::Rope;

179173 use smallvec::SmallVec;

180174

181− #[test]

182− fn test_find_nth_pairs_pos() {

183− let doc = Rope::from("some (text) here");

175+ fn check_find_nth_pair_pos(

176+ text: &str,

177+ cases: Vec<(usize, char, usize, Option<(usize, usize)>)>,

178+ ) {

179+ let doc = Rope::from(text);

184180 let slice = doc.slice(..);

185181

186− // cursor on [t]ext

187− assert_eq!(find_nth_pairs_pos(slice, '(', 6, 1), Some((5, 10)));

188− assert_eq!(find_nth_pairs_pos(slice, ')', 6, 1), Some((5, 10)));

189− // cursor on so[m]e

190− assert_eq!(find_nth_pairs_pos(slice, '(', 2, 1), None);

191− // cursor on bracket itself

192− assert_eq!(find_nth_pairs_pos(slice, '(', 5, 1), Some((5, 10)));

193− assert_eq!(find_nth_pairs_pos(slice, '(', 10, 1), Some((5, 10)));

182+ for (cursor_pos, ch, n, expected_range) in cases {

183+ let range = find_nth_pairs_pos(slice, ch, (cursor_pos, cursor_pos + 1).into(), n);

184+ assert_eq!(

185+ range, expected_range,

186+ "Expected {:?}, got {:?}",

187+ expected_range, range

188+ );

189+ }

194190 }

195191

196192 #[test]

197− fn test_find_nth_pairs_pos_skip() {

198− let doc = Rope::from("(so (many (good) text) here)");

199− let slice = doc.slice(..);

193+ fn test_find_nth_pairs_pos() {

194+ check_find_nth_pair_pos(

195+ "some (text) here",

196+ vec![

197+ // cursor on [t]ext

198+ (6, '(', 1, Some((5, 10))),

199+ (6, ')', 1, Some((5, 10))),

200+ // cursor on so[m]e

201+ (2, '(', 1, None),

202+ // cursor on bracket itself

203+ (5, '(', 1, Some((5, 10))),

204+ (10, '(', 1, Some((5, 10))),

205+ ],

206+ );

207+ }

200208

201− // cursor on go[o]d

202− assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((10, 15)));

203− assert_eq!(find_nth_pairs_pos(slice, '(', 13, 2), Some((4, 21)));

204− assert_eq!(find_nth_pairs_pos(slice, '(', 13, 3), Some((0, 27)));

209+ #[test]

210+ fn test_find_nth_pairs_pos_skip() {

211+ check_find_nth_pair_pos(

212+ "(so (many (good) text) here)",

213+ vec![

214+ // cursor on go[o]d

215+ (13, '(', 1, Some((10, 15))),

216+ (13, '(', 2, Some((4, 21))),

217+ (13, '(', 3, Some((0, 27))),

218+ ],

219+ );

205220 }

206221

207222 #[test]

208223 fn test_find_nth_pairs_pos_same() {

209− let doc = Rope::from("'so 'many 'good' text' here'");

210− let slice = doc.slice(..);

211−

212− // cursor on go[o]d

213− assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 1), Some((10, 15)));

214− assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 2), Some((4, 21)));

215− assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 3), Some((0, 27)));

216− // cursor on the quotes

217− assert_eq!(find_nth_pairs_pos(slice, '\'', 10, 1), None);

218− // this is the best we can do since opening and closing pairs are same

219− assert_eq!(find_nth_pairs_pos(slice, '\'', 0, 1), Some((0, 4)));

220− assert_eq!(find_nth_pairs_pos(slice, '\'', 27, 1), Some((21, 27)));

224+ check_find_nth_pair_pos(

225+ "'so 'many 'good' text' here'",

226+ vec![

227+ // cursor on go[o]d

228+ (13, '\'', 1, Some((10, 15))),

229+ (13, '\'', 2, Some((4, 21))),

230+ (13, '\'', 3, Some((0, 27))),

231+ // cursor on the quotes

232+ (10, '\'', 1, None),

233+ ],

234+ )

221235 }

222236

223237 #[test]

224238 fn test_find_nth_pairs_pos_step() {

225− let doc = Rope::from("((so)((many) good (text))(here))");

226− let slice = doc.slice(..);

227−

228− // cursor on go[o]d

229− assert_eq!(find_nth_pairs_pos(slice, '(', 15, 1), Some((5, 24)));

230− assert_eq!(find_nth_pairs_pos(slice, '(', 15, 2), Some((0, 31)));

239+ check_find_nth_pair_pos(

240+ "((so)((many) good (text))(here))",

241+ vec![

242+ // cursor on go[o]d

243+ (15, '(', 1, Some((5, 24))),

244+ (15, '(', 2, Some((0, 31))),

245+ ],

246+ )

231247 }

232248

233249 #[test]

234250 fn test_find_nth_pairs_pos_mixed() {

235− let doc = Rope::from("(so [many {good} text] here)");

236− let slice = doc.slice(..);

237−

238− // cursor on go[o]d

239− assert_eq!(find_nth_pairs_pos(slice, '{', 13, 1), Some((10, 15)));

240− assert_eq!(find_nth_pairs_pos(slice, '[', 13, 1), Some((4, 21)));

241− assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((0, 27)));

251+ check_find_nth_pair_pos(

252+ "(so [many {good} text] here)",

253+ vec![

254+ // cursor on go[o]d

255+ (13, '{', 1, Some((10, 15))),

256+ (13, '[', 1, Some((4, 21))),

257+ (13, '(', 1, Some((0, 27))),

258+ ],

259+ )

242260 }

243261

244262 #[test]

helix-core/src/textobject.rs+6−4

@@ -114,7 +114,7 @@ pub fn textobject_surround(

114114 ch: char,

115115 count: usize,

116116) -> Range {

117− surround::find_nth_pairs_pos(slice, ch, range.head, count)

117+ surround::find_nth_pairs_pos(slice, ch, range, count)

118118 .map(|(anchor, head)| match textobject {

119119 TextObject::Inside => Range::new(next_grapheme_boundary(slice, anchor), head),

120120 TextObject::Around => Range::new(anchor, next_grapheme_boundary(slice, head)),

@@ -170,7 +170,7 @@ mod test {

170170

171171 #[test]

172172 fn test_textobject_word() {

173− // (text, [(cursor position, textobject, final range), ...])

173+ // (text, [(char position, textobject, final range), ...])

174174 let tests = &[

175175 (

176176 "cursor at beginning of doc",

@@ -269,7 +269,9 @@ mod test {

269269 let slice = doc.slice(..);

270270 for &case in scenario {

271271 let (pos, objtype, expected_range) = case;

272− let result = textobject_word(slice, Range::point(pos), objtype, 1, false);

272+ // cursor is a single width selection

273+ let range = Range::new(pos, pos + 1);

274+ let result = textobject_word(slice, range, objtype, 1, false);

273275 assert_eq!(

274276 result,

275277 expected_range.into(),

@@ -283,7 +285,7 @@ mod test {

283285

284286 #[test]

285287 fn test_textobject_surround() {

286− // (text, [(cursor position, textobject, final range, count), ...])

288+ // (text, [(cursor position, textobject, final range, surround char, count), ...])

287289 let tests = &[

288290 (

289291 "simple (single) surround pairs",