72b9311 · helix · git.iko.soy

git.iko.soy/helix

fix view anchors not at start of a visual line

The top of a view is marked by a char idx anchor. That char idx is usually the first character of the visual line it's on. We use a char index instead of a line index because the view may start in the middle of a line with soft wrapping. However, it's possible to temporarily endup in a state where this anchor is not the first character of the first visual line. This is pretty rare because edits usually happen inside/after the view. In most cases we handle this case correctly. However, if the cursor is before the anchor (but still in view) there can be crashes or visual artifacts. This is caused by the fact that visual_offset_from_anchor (and the positioning code in view.rs) incorrectly assumed that the (cursor) position is always after the view anchor if the cursor is in view. But if the anchor is not the first character of the first visual line this is not the case anymore. In that case crashes and visual artifacts are possible. This commit fixes that problem by changing `visual_offset_from_anchor` (and callsites) to properly consider that case.

commit72b93116784ec944f49c7f6a335b0aa663f1430e authorPascal Kuthe <pascal.kuthe@semimod.de> dateMon 6 Mar 2023, 16:25 (4 years ago) parent0ab96cc

3 files changed+70−48

compare with master

unifiedsplit

helix-core/src/lib.rs+1−1 helix-core/src/position.rs+32−8 helix-view/src/view.rs+37−39

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

@@ -98,7 +98,7 @@ pub use {regex, tree_sitter};

9898pub use graphemes::RopeGraphemes;

9999pub use position::{

100100 char_idx_at_visual_offset, coords_at_pos, pos_at_coords, visual_offset_from_anchor,

101− visual_offset_from_block, Position,

101+ visual_offset_from_block, Position, VisualOffsetError,

102102};

103103#[allow(deprecated)]

104104pub use position::{pos_at_visual_coords, visual_coords_at_pos};

helix-core/src/position.rs+32−8

@@ -137,6 +137,12 @@ pub fn visual_offset_from_block(

137137 (last_pos, block_start)

138138}

139139

140+#[derive(Debug, PartialEq, Eq, Clone, Copy)]

141+pub enum VisualOffsetError {

142+ PosBeforeAnchorRow,

143+ PosAfterMaxRow,

144+}

145+

140146/// Returns the visual offset from the start of the visual line

141147/// that contains anchor.

142148pub fn visual_offset_from_anchor(

@@ -146,28 +152,46 @@ pub fn visual_offset_from_anchor(

146152 text_fmt: &TextFormat,

147153 annotations: &TextAnnotations,

148154 max_rows: usize,

149−) -> Option<(Position, usize)> {

155+) -> Result<(Position, usize), VisualOffsetError> {

150156 let (formatter, block_start) =

151157 DocumentFormatter::new_at_prev_checkpoint(text, text_fmt, annotations, anchor);

152158 let mut char_pos = block_start;

153159 let mut anchor_line = None;

160+ let mut found_pos = None;

154161 let mut last_pos = Position::default();

155162

163+ if pos < block_start {

164+ return Err(VisualOffsetError::PosBeforeAnchorRow);

165+ }

166+

156167 for (grapheme, vpos) in formatter {

157168 last_pos = vpos;

158169 char_pos += grapheme.doc_chars();

159170

160− if char_pos > anchor && anchor_line.is_none() {

161− anchor_line = Some(last_pos.row);

162− }

163171 if char_pos > pos {

164− last_pos.row -= anchor_line.unwrap();

165− return Some((last_pos, block_start));

172+ if let Some(anchor_line) = anchor_line {

173+ last_pos.row -= anchor_line;

174+ return Ok((last_pos, block_start));

175+ } else {

176+ found_pos = Some(last_pos);

177+ }

178+ }

179+ if char_pos > anchor && anchor_line.is_none() {

180+ if let Some(mut found_pos) = found_pos {

181+ return if found_pos.row == last_pos.row {

182+ found_pos.row = 0;

183+ Ok((found_pos, block_start))

184+ } else {

185+ Err(VisualOffsetError::PosBeforeAnchorRow)

186+ };

187+ } else {

188+ anchor_line = Some(last_pos.row);

189+ }

166190 }

167191

168192 if let Some(anchor_line) = anchor_line {

169193 if vpos.row >= anchor_line + max_rows {

170− return None;

194+ return Err(VisualOffsetError::PosAfterMaxRow);

171195 }

172196 }

173197 }

@@ -175,7 +199,7 @@ pub fn visual_offset_from_anchor(

175199 let anchor_line = anchor_line.unwrap_or(last_pos.row);

176200 last_pos.row -= anchor_line;

177201

178− Some((last_pos, block_start))

202+ Ok((last_pos, block_start))

179203}

180204

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

helix-view/src/view.rs+37−39

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

77};

99use helix_core::{

10− char_idx_at_visual_offset, doc_formatter::TextFormat, syntax::Highlight,

11− text_annotations::TextAnnotations, visual_offset_from_anchor, visual_offset_from_block,

12− Position, RopeSlice, Selection, Transaction,

10+ char_idx_at_visual_offset,

11+ doc_formatter::TextFormat,

12+ syntax::Highlight,

13+ text_annotations::TextAnnotations,

14+ visual_offset_from_anchor, visual_offset_from_block, Position, RopeSlice, Selection,

15+ Transaction,

16+ VisualOffsetError::{PosAfterMaxRow, PosBeforeAnchorRow},

1317};

1418

1519use std::{

@@ -213,46 +217,38 @@ impl View {

213217 // - 1 so we have at least one gap in the middle.

214218 // a height of 6 with padding of 3 on each side will keep shifting the view back and forth

215219 // as we type

216− let scrolloff = scrolloff.min(viewport.height.saturating_sub(1) as usize / 2);

220+ let scrolloff = if CENTERING {

221+ 0

222+ } else {

223+ scrolloff.min(viewport.height.saturating_sub(1) as usize / 2)

224+ };

217225

218226 let cursor = doc.selection(self.id).primary().cursor(doc_text);

219227 let mut offset = self.offset;

228+ let off = visual_offset_from_anchor(

229+ doc_text,

230+ offset.anchor,

231+ cursor,

232+ &text_fmt,

233+ &annotations,

234+ vertical_viewport_end,

235+ );

220236

221− let (visual_off, mut at_top) = if cursor >= offset.anchor {

222− let off = visual_offset_from_anchor(

223− doc_text,

224− offset.anchor,

225− cursor,

226− &text_fmt,

227− &annotations,

228− vertical_viewport_end,

229− );

230− (off, false)

231− } else if CENTERING {

232− // cursor out of view

233− return None;

234− } else {

235− (None, true)

236− };

237−

238− let new_anchor = match visual_off {

239− Some((visual_pos, _)) if visual_pos.row < scrolloff + offset.vertical_offset => {

240− if CENTERING && visual_pos.row < offset.vertical_offset {

237+ let (new_anchor, at_top) = match off {

238+ Ok((visual_pos, _)) if visual_pos.row < scrolloff + offset.vertical_offset => {

239+ if CENTERING {

241240 // cursor out of view

242241 return None;

243242 }

244− at_top = true;

245− true

243+ (true, true)

246244 }

247− Some((visual_pos, _)) if visual_pos.row + scrolloff + 1 >= vertical_viewport_end => {

248− if CENTERING && visual_pos.row >= vertical_viewport_end {

249− // cursor out of view

250− return None;

251− }

252− true

245+ Ok((visual_pos, _)) if visual_pos.row + scrolloff >= vertical_viewport_end => {

246+ (true, false)

253247 }

254− Some(_) => false,

255− None => true,

248+ Ok((_, _)) => (false, false),

249+ Err(_) if CENTERING => return None,

250+ Err(PosBeforeAnchorRow) => (true, true),

251+ Err(PosAfterMaxRow) => (true, false),

256252 };

257253

258254 if new_anchor {

@@ -269,8 +265,8 @@ impl View {

269265 offset.horizontal_offset = 0;

270266 } else {

271267 // determine the current visual column of the text

272− let col = visual_off

273− .unwrap_or_else(|| {

268+ let col = off

269+ .unwrap_or_else(|_| {

274270 visual_offset_from_block(

275271 doc_text,

276272 offset.anchor,

@@ -360,8 +356,9 @@ impl View {

360356 );

361357

362358 match pos {

363− Some((Position { row, .. }, _)) => row.saturating_sub(self.offset.vertical_offset),

364− None => visual_height.saturating_sub(1),

359+ Ok((Position { row, .. }, _)) => row.saturating_sub(self.offset.vertical_offset),

360+ Err(PosAfterMaxRow) => visual_height.saturating_sub(1),

361+ Err(PosBeforeAnchorRow) => 0,

365362 }

366363 }

367364

@@ -390,7 +387,8 @@ impl View {

390387 &text_fmt,

391388 &annotations,

392389 viewport.height as usize,

393− )?

390+ )

391+ .ok()?

394392 .0;

395393 if pos.row < self.offset.vertical_offset {

396394 return None;