From 2389563f1a99b3753137f75b7504e9f81c076743 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:35:40 +0100 Subject: [PATCH 01/13] implement Add/Sub for position being able to add/subtract positions is very handy when writing rendering code --- helix-core/src/position.rs | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs index ee764bc64aa7..dba11212181d 100644 --- a/helix-core/src/position.rs +++ b/helix-core/src/position.rs @@ -1,4 +1,8 @@ -use std::{borrow::Cow, cmp::Ordering}; +use std::{ + borrow::Cow, + cmp::Ordering, + ops::{Add, AddAssign, Sub, SubAssign}, +}; use crate::{ chars::char_is_line_ending, @@ -16,6 +20,38 @@ pub struct Position { pub col: usize, } +impl AddAssign for Position { + fn add_assign(&mut self, rhs: Self) { + self.row += rhs.row; + self.col += rhs.col; + } +} + +impl SubAssign for Position { + fn sub_assign(&mut self, rhs: Self) { + self.row -= rhs.row; + self.col -= rhs.col; + } +} + +impl Sub for Position { + type Output = Position; + + fn sub(mut self, rhs: Self) -> Self::Output { + self -= rhs; + self + } +} + +impl Add for Position { + type Output = Position; + + fn add(mut self, rhs: Self) -> Self::Output { + self += rhs; + self + } +} + impl Position { pub const fn new(row: usize, col: usize) -> Self { Self { row, col } From 7b1e38aa1bd8679b9dc439f02a6f275d88137c22 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:05 +0100 Subject: [PATCH 02/13] ensure highlight scopes are skipped properly --- helix-term/src/ui/document.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/helix-term/src/ui/document.rs b/helix-term/src/ui/document.rs index b571b83c2ddd..3ed566f2f06f 100644 --- a/helix-term/src/ui/document.rs +++ b/helix-term/src/ui/document.rs @@ -54,10 +54,7 @@ impl> Iterator for StyleIter<'_, H> { HighlightEvent::HighlightEnd => { self.active_highlights.pop(); } - HighlightEvent::Source { start, end } => { - if start == end { - continue; - } + HighlightEvent::Source { end, .. } => { let style = self .active_highlights .iter() @@ -279,12 +276,12 @@ pub fn render_text<'t>( } // acquire the correct grapheme style - if char_pos >= syntax_style_span.1 { + while char_pos >= syntax_style_span.1 { syntax_style_span = syntax_styles .next() .unwrap_or((Style::default(), usize::MAX)); } - if char_pos >= overlay_style_span.1 { + while char_pos >= overlay_style_span.1 { overlay_style_span = overlay_styles .next() .unwrap_or((Style::default(), usize::MAX)); From 0a5b8971f5d6f82d19645588a62f76ee48f183d8 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:06 +0100 Subject: [PATCH 03/13] track char_idx in DocFormatter --- helix-core/src/doc_formatter.rs | 193 ++++++++++++++++----------- helix-core/src/doc_formatter/test.rs | 18 +-- helix-core/src/position.rs | 50 ++++--- helix-term/src/ui/document.rs | 100 ++++++++------ 4 files changed, 206 insertions(+), 155 deletions(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index cbe2da3b697a..f934b38a1095 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -37,52 +37,91 @@ pub enum GraphemeSource { }, } +impl GraphemeSource { + /// Returns whether this grapheme is virtual inline text + pub fn is_virtual(self) -> bool { + matches!(self, GraphemeSource::VirtualText { .. }) + } + + pub fn doc_chars(self) -> usize { + match self { + GraphemeSource::Document { codepoints } => codepoints as usize, + GraphemeSource::VirtualText { .. } => 0, + } + } +} + #[derive(Debug, Clone)] pub struct FormattedGrapheme<'a> { - pub grapheme: Grapheme<'a>, + pub raw: Grapheme<'a>, pub source: GraphemeSource, + pub visual_pos: Position, + /// Document line at the start of the grapheme + pub line_idx: usize, + /// Document char position at the start of the grapheme + pub char_idx: usize, } -impl<'a> FormattedGrapheme<'a> { - pub fn new( +impl FormattedGrapheme<'_> { + pub fn is_virtual(&self) -> bool { + self.source.is_virtual() + } + + pub fn doc_chars(&self) -> usize { + self.source.doc_chars() + } + + pub fn is_whitespace(&self) -> bool { + self.raw.is_whitespace() + } + + pub fn width(&self) -> usize { + self.raw.width() + } + + pub fn is_word_boundary(&self) -> bool { + self.raw.is_word_boundary() + } +} + +#[derive(Debug, Clone)] +struct GraphemeWithSource<'a> { + grapheme: Grapheme<'a>, + source: GraphemeSource, +} + +impl<'a> GraphemeWithSource<'a> { + fn new( g: GraphemeStr<'a>, visual_x: usize, tab_width: u16, source: GraphemeSource, - ) -> FormattedGrapheme<'a> { - FormattedGrapheme { + ) -> GraphemeWithSource<'a> { + GraphemeWithSource { grapheme: Grapheme::new(g, visual_x, tab_width), source, } } - /// Returns whether this grapheme is virtual inline text - pub fn is_virtual(&self) -> bool { - matches!(self.source, GraphemeSource::VirtualText { .. }) - } - - pub fn placeholder() -> Self { - FormattedGrapheme { + fn placeholder() -> Self { + GraphemeWithSource { grapheme: Grapheme::Other { g: " ".into() }, source: GraphemeSource::Document { codepoints: 0 }, } } - pub fn doc_chars(&self) -> usize { - match self.source { - GraphemeSource::Document { codepoints } => codepoints as usize, - GraphemeSource::VirtualText { .. } => 0, - } + fn doc_chars(&self) -> usize { + self.source.doc_chars() } - pub fn is_whitespace(&self) -> bool { + fn is_whitespace(&self) -> bool { self.grapheme.is_whitespace() } - pub fn width(&self) -> usize { + fn width(&self) -> usize { self.grapheme.width() } - pub fn is_word_boundary(&self) -> bool { + fn is_word_boundary(&self) -> bool { self.grapheme.is_word_boundary() } } @@ -139,9 +178,9 @@ pub struct DocumentFormatter<'t> { indent_level: Option, /// In case a long word needs to be split a single grapheme might need to be wrapped /// while the rest of the word stays on the same line - peeked_grapheme: Option<(FormattedGrapheme<'t>, usize)>, + peeked_grapheme: Option>, /// A first-in first-out (fifo) buffer for the Graphemes of any given word - word_buf: Vec>, + word_buf: Vec>, /// The index of the next grapheme that will be yielded from the `word_buf` word_i: usize, } @@ -157,32 +196,33 @@ impl<'t> DocumentFormatter<'t> { text_fmt: &'t TextFormat, annotations: &'t TextAnnotations, char_idx: usize, - ) -> (Self, usize) { + ) -> Self { // TODO divide long lines into blocks to avoid bad performance for long lines let block_line_idx = text.char_to_line(char_idx.min(text.len_chars())); let block_char_idx = text.line_to_char(block_line_idx); annotations.reset_pos(block_char_idx); - ( - DocumentFormatter { - text_fmt, - annotations, - visual_pos: Position { row: 0, col: 0 }, - graphemes: RopeGraphemes::new(text.slice(block_char_idx..)), - char_pos: block_char_idx, - exhausted: false, - virtual_lines: 0, - indent_level: None, - peeked_grapheme: None, - word_buf: Vec::with_capacity(64), - word_i: 0, - line_pos: block_line_idx, - inline_anntoation_graphemes: None, - }, - block_char_idx, - ) + + DocumentFormatter { + text_fmt, + annotations, + visual_pos: Position { row: 0, col: 0 }, + graphemes: RopeGraphemes::new(text.slice(block_char_idx..)), + char_pos: block_char_idx, + exhausted: false, + virtual_lines: 0, + indent_level: None, + peeked_grapheme: None, + word_buf: Vec::with_capacity(64), + word_i: 0, + line_pos: block_line_idx, + inline_anntoation_graphemes: None, + } } - fn next_inline_annotation_grapheme(&mut self) -> Option<(&'t str, Option)> { + fn next_inline_annotation_grapheme( + &mut self, + char_pos: usize, + ) -> Option<(&'t str, Option)> { loop { if let Some(&mut (ref mut annotation, highlight)) = self.inline_anntoation_graphemes.as_mut() @@ -193,7 +233,7 @@ impl<'t> DocumentFormatter<'t> { } if let Some((annotation, highlight)) = - self.annotations.next_inline_annotation_at(self.char_pos) + self.annotations.next_inline_annotation_at(char_pos) { self.inline_anntoation_graphemes = Some(( UnicodeSegmentation::graphemes(&*annotation.text, true), @@ -205,21 +245,20 @@ impl<'t> DocumentFormatter<'t> { } } - fn advance_grapheme(&mut self, col: usize) -> Option> { + fn advance_grapheme(&mut self, col: usize, char_pos: usize) -> Option> { let (grapheme, source) = - if let Some((grapheme, highlight)) = self.next_inline_annotation_grapheme() { + if let Some((grapheme, highlight)) = self.next_inline_annotation_grapheme(char_pos) { (grapheme.into(), GraphemeSource::VirtualText { highlight }) } else if let Some(grapheme) = self.graphemes.next() { self.virtual_lines += self.annotations.annotation_lines_at(self.char_pos); let codepoints = grapheme.len_chars() as u32; - let overlay = self.annotations.overlay_at(self.char_pos); + let overlay = self.annotations.overlay_at(char_pos); let grapheme = match overlay { Some((overlay, _)) => overlay.grapheme.as_str().into(), None => Cow::from(grapheme).into(), }; - self.char_pos += codepoints as usize; (grapheme, GraphemeSource::Document { codepoints }) } else { if self.exhausted { @@ -228,19 +267,19 @@ impl<'t> DocumentFormatter<'t> { self.exhausted = true; // EOF grapheme is required for rendering // and correct position computations - return Some(FormattedGrapheme { + return Some(GraphemeWithSource { grapheme: Grapheme::Other { g: " ".into() }, source: GraphemeSource::Document { codepoints: 0 }, }); }; - let grapheme = FormattedGrapheme::new(grapheme, col, self.text_fmt.tab_width, source); + let grapheme = GraphemeWithSource::new(grapheme, col, self.text_fmt.tab_width, source); Some(grapheme) } /// Move a word to the next visual line - fn wrap_word(&mut self, virtual_lines_before_word: usize) -> usize { + fn wrap_word(&mut self) -> usize { // softwrap this word to the next line let indent_carry_over = if let Some(indent) = self.indent_level { if indent as u16 <= self.text_fmt.max_indent_retain { @@ -255,14 +294,13 @@ impl<'t> DocumentFormatter<'t> { }; self.visual_pos.col = indent_carry_over as usize; - self.virtual_lines -= virtual_lines_before_word; - self.visual_pos.row += 1 + virtual_lines_before_word; + self.visual_pos.row += 1 + take(&mut self.virtual_lines); let mut i = 0; let mut word_width = 0; let wrap_indicator = UnicodeSegmentation::graphemes(&*self.text_fmt.wrap_indicator, true) .map(|g| { i += 1; - let grapheme = FormattedGrapheme::new( + let grapheme = GraphemeWithSource::new( g.into(), self.visual_pos.col + word_width, self.text_fmt.tab_width, @@ -288,8 +326,7 @@ impl<'t> DocumentFormatter<'t> { fn advance_to_next_word(&mut self) { self.word_buf.clear(); let mut word_width = 0; - let virtual_lines_before_word = self.virtual_lines; - let mut virtual_lines_before_grapheme = self.virtual_lines; + let mut word_chars = 0; loop { // softwrap word if necessary @@ -301,27 +338,24 @@ impl<'t> DocumentFormatter<'t> { // However if the last grapheme is multiple columns wide it might extend beyond the EOL. // The condition below ensures that this grapheme is not cutoff and instead wrapped to the next line if word_width + self.visual_pos.col > self.text_fmt.viewport_width as usize { - self.peeked_grapheme = self.word_buf.pop().map(|grapheme| { - (grapheme, self.virtual_lines - virtual_lines_before_grapheme) - }); - self.virtual_lines = virtual_lines_before_grapheme; + self.peeked_grapheme = self.word_buf.pop(); } return; } - word_width = self.wrap_word(virtual_lines_before_word); + word_width = self.wrap_word(); } - virtual_lines_before_grapheme = self.virtual_lines; - - let grapheme = if let Some((grapheme, virtual_lines)) = self.peeked_grapheme.take() { - self.virtual_lines += virtual_lines; + let grapheme = if let Some(grapheme) = self.peeked_grapheme.take() { grapheme - } else if let Some(grapheme) = self.advance_grapheme(self.visual_pos.col + word_width) { + } else if let Some(grapheme) = + self.advance_grapheme(self.visual_pos.col + word_width, self.char_pos + word_chars) + { grapheme } else { return; }; + word_chars += grapheme.doc_chars(); // Track indentation if !grapheme.is_whitespace() && self.indent_level.is_none() { @@ -340,19 +374,19 @@ impl<'t> DocumentFormatter<'t> { } } - /// returns the document line pos of the **next** grapheme that will be yielded - pub fn line_pos(&self) -> usize { - self.line_pos + /// returns the char index at the end of the last yielded grapheme + pub fn next_char_pos(&self) -> usize { + self.char_pos } - /// returns the visual pos of the **next** grapheme that will be yielded - pub fn visual_pos(&self) -> Position { + /// returns the visual position at the end of the last yielded grapheme + pub fn next_visual_pos(&self) -> Position { self.visual_pos } } impl<'t> Iterator for DocumentFormatter<'t> { - type Item = (FormattedGrapheme<'t>, Position); + type Item = FormattedGrapheme<'t>; fn next(&mut self) -> Option { let grapheme = if self.text_fmt.soft_wrap { @@ -362,15 +396,18 @@ impl<'t> Iterator for DocumentFormatter<'t> { } let grapheme = replace( self.word_buf.get_mut(self.word_i)?, - FormattedGrapheme::placeholder(), + GraphemeWithSource::placeholder(), ); self.word_i += 1; grapheme } else { - self.advance_grapheme(self.visual_pos.col)? + self.advance_grapheme(self.visual_pos.col, self.char_pos)? }; - let pos = self.visual_pos; + let visual_pos = self.visual_pos; + let char_pos = self.char_pos; + self.char_pos += grapheme.doc_chars(); + let line_idx = self.line_pos; if grapheme.grapheme == Grapheme::Newline { self.visual_pos.row += 1; self.visual_pos.row += take(&mut self.virtual_lines); @@ -379,6 +416,12 @@ impl<'t> Iterator for DocumentFormatter<'t> { } else { self.visual_pos.col += grapheme.width(); } - Some((grapheme, pos)) + Some(FormattedGrapheme { + raw: grapheme.grapheme, + source: grapheme.source, + visual_pos, + line_idx, + char_idx: char_pos, + }) } } diff --git a/helix-core/src/doc_formatter/test.rs b/helix-core/src/doc_formatter/test.rs index d2b6ddc74a91..b214ab944027 100644 --- a/helix-core/src/doc_formatter/test.rs +++ b/helix-core/src/doc_formatter/test.rs @@ -23,18 +23,18 @@ impl<'t> DocumentFormatter<'t> { let viewport_width = self.text_fmt.viewport_width; let mut line = 0; - for (grapheme, pos) in self { - if pos.row != line { + for grapheme in self { + if grapheme.visual_pos.row != line { line += 1; - assert_eq!(pos.row, line); - write!(res, "\n{}", ".".repeat(pos.col)).unwrap(); + assert_eq!(grapheme.visual_pos.row, line); + write!(res, "\n{}", ".".repeat(grapheme.visual_pos.col)).unwrap(); assert!( - pos.col <= viewport_width as usize, + grapheme.visual_pos.col <= viewport_width as usize, "softwrapped failed {}<={viewport_width}", - pos.col + grapheme.visual_pos.col ); } - write!(res, "{}", grapheme.grapheme).unwrap(); + write!(res, "{}", grapheme.raw).unwrap(); } res @@ -48,7 +48,6 @@ fn softwrap_text(text: &str) -> String { &TextAnnotations::default(), 0, ) - .0 .collect_to_str() } @@ -106,7 +105,6 @@ fn overlay_text(text: &str, char_pos: usize, softwrap: bool, overlays: &[Overlay TextAnnotations::default().add_overlay(overlays, None), char_pos, ) - .0 .collect_to_str() } @@ -143,7 +141,6 @@ fn annotate_text(text: &str, softwrap: bool, annotations: &[InlineAnnotation]) - TextAnnotations::default().add_inline_annotations(annotations, None), 0, ) - .0 .collect_to_str() } @@ -182,7 +179,6 @@ fn annotation_and_overlay() { .add_overlay(overlay.as_slice(), None), 0, ) - .0 .collect_to_str(), "fooo bar " ); diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs index dba11212181d..0860da12f1df 100644 --- a/helix-core/src/position.rs +++ b/helix-core/src/position.rs @@ -157,16 +157,14 @@ pub fn visual_offset_from_block( annotations: &TextAnnotations, ) -> (Position, usize) { let mut last_pos = Position::default(); - let (formatter, block_start) = + let mut formatter = DocumentFormatter::new_at_prev_checkpoint(text, text_fmt, annotations, anchor); - let mut char_pos = block_start; + let block_start = formatter.next_char_pos(); - for (grapheme, vpos) in formatter { - last_pos = vpos; - char_pos += grapheme.doc_chars(); - - if char_pos > pos { - return (last_pos, block_start); + while let Some(grapheme) = formatter.next() { + last_pos = grapheme.visual_pos; + if formatter.next_char_pos() > pos { + return (grapheme.visual_pos, block_start); } } @@ -189,22 +187,21 @@ pub fn visual_offset_from_anchor( annotations: &TextAnnotations, max_rows: usize, ) -> Result<(Position, usize), VisualOffsetError> { - let (formatter, block_start) = + let mut formatter = DocumentFormatter::new_at_prev_checkpoint(text, text_fmt, annotations, anchor); - let mut char_pos = block_start; let mut anchor_line = None; let mut found_pos = None; let mut last_pos = Position::default(); + let block_start = formatter.next_char_pos(); if pos < block_start { return Err(VisualOffsetError::PosBeforeAnchorRow); } - for (grapheme, vpos) in formatter { - last_pos = vpos; - char_pos += grapheme.doc_chars(); + while let Some(grapheme) = formatter.next() { + last_pos = grapheme.visual_pos; - if char_pos > pos { + if formatter.next_char_pos() > pos { if let Some(anchor_line) = anchor_line { last_pos.row -= anchor_line; return Ok((last_pos, block_start)); @@ -212,7 +209,7 @@ pub fn visual_offset_from_anchor( found_pos = Some(last_pos); } } - if char_pos > anchor && anchor_line.is_none() { + if formatter.next_char_pos() > anchor && anchor_line.is_none() { if let Some(mut found_pos) = found_pos { return if found_pos.row == last_pos.row { found_pos.row = 0; @@ -226,7 +223,7 @@ pub fn visual_offset_from_anchor( } if let Some(anchor_line) = anchor_line { - if vpos.row >= anchor_line + max_rows { + if grapheme.visual_pos.row >= anchor_line + max_rows { return Err(VisualOffsetError::PosAfterMaxRow); } } @@ -404,34 +401,33 @@ pub fn char_idx_at_visual_block_offset( text_fmt: &TextFormat, annotations: &TextAnnotations, ) -> (usize, usize) { - let (formatter, mut char_idx) = + let mut formatter = DocumentFormatter::new_at_prev_checkpoint(text, text_fmt, annotations, anchor); - let mut last_char_idx = char_idx; + let mut last_char_idx = formatter.next_char_pos(); let mut last_char_idx_on_line = None; let mut last_row = 0; - for (grapheme, grapheme_pos) in formatter { - match grapheme_pos.row.cmp(&row) { + for grapheme in &mut formatter { + match grapheme.visual_pos.row.cmp(&row) { Ordering::Equal => { - if grapheme_pos.col + grapheme.width() > column { + if grapheme.visual_pos.col + grapheme.width() > column { if !grapheme.is_virtual() { - return (char_idx, 0); + return (grapheme.char_idx, 0); } else if let Some(char_idx) = last_char_idx_on_line { return (char_idx, 0); } } else if !grapheme.is_virtual() { - last_char_idx_on_line = Some(char_idx) + last_char_idx_on_line = Some(grapheme.char_idx) } } Ordering::Greater => return (last_char_idx, row - last_row), _ => (), } - last_char_idx = char_idx; - last_row = grapheme_pos.row; - char_idx += grapheme.doc_chars(); + last_char_idx = grapheme.char_idx; + last_row = grapheme.visual_pos.row; } - (char_idx, 0) + (formatter.next_char_pos(), 0) } #[cfg(test)] diff --git a/helix-term/src/ui/document.rs b/helix-term/src/ui/document.rs index 3ed566f2f06f..fee08ed3317f 100644 --- a/helix-term/src/ui/document.rs +++ b/helix-term/src/ui/document.rs @@ -162,21 +162,18 @@ pub fn render_text<'t>( line_decorations: &mut [Box], translated_positions: &mut [TranslatedPosition], ) { - let ( - Position { - row: mut row_off, .. - }, - mut char_pos, - ) = visual_offset_from_block( + let mut row_off = visual_offset_from_block( text, offset.anchor, offset.anchor, text_fmt, text_annotations, - ); + ) + .0 + .row; row_off += offset.vertical_offset; - let (mut formatter, mut first_visible_char_idx) = + let mut formatter = DocumentFormatter::new_at_prev_checkpoint(text, text_fmt, text_annotations, offset.anchor); let mut syntax_styles = StyleIter { text_style: renderer.text_style, @@ -205,19 +202,19 @@ pub fn render_text<'t>( let mut overlay_style_span = overlay_styles .next() .unwrap_or_else(|| (Style::default(), usize::MAX)); + let mut first_visible_char_idx = formatter.next_char_pos(); loop { // formattter.line_pos returns to line index of the next grapheme // so it must be called before formatter.next - let doc_line = formatter.line_pos(); - let Some((grapheme, mut pos)) = formatter.next() else { - let mut last_pos = formatter.visual_pos(); + let Some(mut grapheme) = formatter.next() else { + let mut last_pos = formatter.next_visual_pos(); if last_pos.row >= row_off { last_pos.col -= 1; last_pos.row -= row_off; // check if any positions translated on the fly (like cursor) are at the EOF translate_positions( - char_pos + 1, + text.len_chars() + 1, first_visible_char_idx, translated_positions, text_fmt, @@ -229,46 +226,56 @@ pub fn render_text<'t>( }; // skip any graphemes on visual lines before the block start - if pos.row < row_off { - if char_pos >= syntax_style_span.1 { - syntax_style_span = if let Some(syntax_style_span) = syntax_styles.next() { - syntax_style_span + // if pos.row < row_off { + // if char_pos >= syntax_style_span.1 { + // syntax_style_span = if let Some(syntax_style_span) = syntax_styles.next() { + // syntax_style_span + // } else { + // break; + // } + // } + // if char_pos >= overlay_style_span.1 { + // overlay_style_span = if let Some(overlay_style_span) = overlay_styles.next() { + // overlay_style_span + if grapheme.visual_pos.row < row_off { + if grapheme.char_idx >= style_span.1 { + style_span = if let Some(style_span) = styles.next() { + style_span } else { break; - } - } - if char_pos >= overlay_style_span.1 { - overlay_style_span = if let Some(overlay_style_span) = overlay_styles.next() { - overlay_style_span + }; + overlay_span = if let Some(overlay_span) = overlays.next() { + overlay_span } else { break; - } + }; } - char_pos += grapheme.doc_chars(); - first_visible_char_idx = char_pos + 1; + first_visible_char_idx = formatter.next_char_pos(); continue; } - pos.row -= row_off; + grapheme.visual_pos.row -= row_off; // if the end of the viewport is reached stop rendering - if pos.row as u16 >= renderer.viewport.height { + if grapheme.visual_pos.row as u16 >= renderer.viewport.height + renderer.offset.row as u16 { break; } // apply decorations before rendering a new line - if pos.row as u16 != last_line_pos.visual_line { - if pos.row > 0 { - renderer.draw_indent_guides(last_line_indent_level, last_line_pos.visual_line); + if grapheme.visual_pos.row as u16 != last_line_pos.visual_line { + if grapheme.visual_pos.row > 0 { + // draw indent guides for the last line + renderer + .draw_indent_guides(last_line_indent_level, last_line_pos.visual_line as u16); is_in_indent_area = true; for line_decoration in &mut *line_decorations { - line_decoration.render_foreground(renderer, last_line_pos, char_pos); + line_decoration.render_foreground(renderer, last_line_pos, grapheme.char_idx); } } last_line_pos = LinePos { - first_visual_line: doc_line != last_line_pos.doc_line, - doc_line, - visual_line: pos.row as u16, - start_char_idx: char_pos, + first_visual_line: grapheme.line_idx != last_line_pos.doc_line, + doc_line: grapheme.line_idx, + visual_line: grapheme.visual_pos.row as u16, + start_char_idx: grapheme.char_idx, }; for line_decoration in &mut *line_decorations { line_decoration.render_background(renderer, last_line_pos); @@ -276,26 +283,25 @@ pub fn render_text<'t>( } // acquire the correct grapheme style - while char_pos >= syntax_style_span.1 { + while grapheme.char_idx >= syntax_style_span.1 { syntax_style_span = syntax_styles .next() .unwrap_or((Style::default(), usize::MAX)); } - while char_pos >= overlay_style_span.1 { + while grapheme.char_idx >= overlay_style_span.1 { overlay_style_span = overlay_styles .next() .unwrap_or((Style::default(), usize::MAX)); } - char_pos += grapheme.doc_chars(); // check if any positions translated on the fly (like cursor) has been reached translate_positions( - char_pos, + formatter.next_char_pos(), first_visible_char_idx, translated_positions, text_fmt, renderer, - pos, + grapheme.visual_pos, ); let (syntax_style, overlay_style) = @@ -311,27 +317,37 @@ pub fn render_text<'t>( let is_virtual = grapheme.is_virtual(); renderer.draw_grapheme( +<<<<<<< HEAD grapheme.grapheme, GraphemeStyle { syntax_style, overlay_style, }, is_virtual, +||||||| parent of 5e32edd8 (track char_idx in DocFormatter) + grapheme.grapheme, + grapheme_style, + virt, +======= + grapheme.raw, + grapheme_style, + virt, +>>>>>>> 5e32edd8 (track char_idx in DocFormatter) &mut last_line_indent_level, &mut is_in_indent_area, - pos, + grapheme.visual_pos, ); } renderer.draw_indent_guides(last_line_indent_level, last_line_pos.visual_line); for line_decoration in &mut *line_decorations { - line_decoration.render_foreground(renderer, last_line_pos, char_pos); + line_decoration.render_foreground(renderer, last_line_pos, formatter.next_char_pos()); } } #[derive(Debug)] pub struct TextRenderer<'a> { - pub surface: &'a mut Surface, + surface: &'a mut Surface, pub text_style: Style, pub whitespace_style: Style, pub indent_guide_char: String, From b371a8820abd82d084cfbe9241a1981703ab1cdf Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:06 +0100 Subject: [PATCH 04/13] Improve line annotation API The line annotation as implemented in #5420 had two shortcomings: * It required the height of virtual text lines to be known ahead time * It checked for line anchors at every grapheme The first problem made the API impractical to use in practice because almost all virtual text needs to be softwrapped. For example inline diagnostics should be softwrapped to avoid cutting off the diagnostic message (as no scrolling is possible). While more complex virtual text like side by side diffs must dynamically calculate the number of empty lines two align two documents (which requires taking account both softwrap and virtual text). To address this, the API has been refactored to use a trait. The second issue caused some performance overhead and unnecessarily complicated the `DocumentFormatter`. It was addressed by only calling the trait mentioned above at line breaks (instead of always). This allows offers additional flexibility to annotations as it offers the flexibility to align lines (needed for side by side diffs). --- helix-core/src/doc_formatter.rs | 48 ++++--- helix-core/src/text_annotations.rs | 220 ++++++++++++++++++++++++----- 2 files changed, 213 insertions(+), 55 deletions(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index f934b38a1095..0f9a52b5d3e5 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -11,7 +11,7 @@ use std::borrow::Cow; use std::fmt::Debug; -use std::mem::{replace, take}; +use std::mem::replace; #[cfg(test)] mod test; @@ -166,9 +166,6 @@ pub struct DocumentFormatter<'t> { line_pos: usize, exhausted: bool, - /// Line breaks to be reserved for virtual text - /// at the next line break - virtual_lines: usize, inline_anntoation_graphemes: Option<(Graphemes<'t>, Option)>, // softwrap specific @@ -209,7 +206,6 @@ impl<'t> DocumentFormatter<'t> { graphemes: RopeGraphemes::new(text.slice(block_char_idx..)), char_pos: block_char_idx, exhausted: false, - virtual_lines: 0, indent_level: None, peeked_grapheme: None, word_buf: Vec::with_capacity(64), @@ -250,7 +246,6 @@ impl<'t> DocumentFormatter<'t> { if let Some((grapheme, highlight)) = self.next_inline_annotation_grapheme(char_pos) { (grapheme.into(), GraphemeSource::VirtualText { highlight }) } else if let Some(grapheme) = self.graphemes.next() { - self.virtual_lines += self.annotations.annotation_lines_at(self.char_pos); let codepoints = grapheme.len_chars() as u32; let overlay = self.annotations.overlay_at(char_pos); @@ -293,8 +288,11 @@ impl<'t> DocumentFormatter<'t> { 0 }; + let virtual_lines = + self.annotations + .virtual_lines_at(self.char_pos, self.visual_pos, self.line_pos); self.visual_pos.col = indent_carry_over as usize; - self.visual_pos.row += 1 + take(&mut self.virtual_lines); + self.visual_pos.row += 1 + virtual_lines; let mut i = 0; let mut word_width = 0; let wrap_indicator = UnicodeSegmentation::graphemes(&*self.text_fmt.wrap_indicator, true) @@ -404,24 +402,32 @@ impl<'t> Iterator for DocumentFormatter<'t> { self.advance_grapheme(self.visual_pos.col, self.char_pos)? }; - let visual_pos = self.visual_pos; - let char_pos = self.char_pos; + let grapheme = FormattedGrapheme { + raw: grapheme.grapheme, + source: grapheme.source, + visual_pos: self.visual_pos, + line_idx: self.line_pos, + char_idx: self.char_pos, + }; + self.char_pos += grapheme.doc_chars(); - let line_idx = self.line_pos; - if grapheme.grapheme == Grapheme::Newline { - self.visual_pos.row += 1; - self.visual_pos.row += take(&mut self.virtual_lines); + if !grapheme.is_virtual() { + self.annotations.process_virtual_text_anchors(&grapheme); + } + if grapheme.raw == Grapheme::Newline { + // move to end of newline char + self.visual_pos.col += 1; + let virtual_lines = + self.annotations + .virtual_lines_at(self.char_pos, self.visual_pos, self.line_pos); + self.visual_pos.row += 1 + virtual_lines; self.visual_pos.col = 0; - self.line_pos += 1; + if !grapheme.is_virtual() { + self.line_pos += 1; + } } else { self.visual_pos.col += grapheme.width(); } - Some(FormattedGrapheme { - raw: grapheme.grapheme, - source: grapheme.source, - visual_pos, - line_idx, - char_idx: char_pos, - }) + Some(grapheme) } } diff --git a/helix-core/src/text_annotations.rs b/helix-core/src/text_annotations.rs index 1576914e3653..1efd2d679cbc 100644 --- a/helix-core/src/text_annotations.rs +++ b/helix-core/src/text_annotations.rs @@ -1,8 +1,12 @@ use std::cell::Cell; +use std::cmp::Ordering; +use std::fmt::Debug; use std::ops::Range; +use std::ptr::NonNull; +use crate::doc_formatter::FormattedGrapheme; use crate::syntax::Highlight; -use crate::Tendril; +use crate::{Position, Tendril}; /// An inline annotation is continuous text shown /// on the screen before the grapheme that starts at @@ -75,19 +79,98 @@ impl Overlay { } } -/// Line annotations allow for virtual text between normal -/// text lines. They cause `height` empty lines to be inserted -/// below the document line that contains `anchor_char_idx`. +/// Line annotations allow inserting virtual text lines between normal text +/// lines. These lines can be filled with text in the rendering code as their +/// contents have no effect beyond visual appearance. /// -/// These lines can be filled with text in the rendering code -/// as their contents have no effect beyond visual appearance. +/// The height of virtual text is usually not known ahead of time as virtual +/// text often requires softwrapping. Furthermore the height of some virtual +/// text like side-by-side diffs depends on the height of the text (again +/// influenced by softwrap) and other virtual text. Therefore line annotations +/// are computed on the fly instead of ahead of time like other annotations. /// -/// To insert a line after a document line simply set -/// `anchor_char_idx` to `doc.line_to_char(line_idx)` -#[derive(Debug, Clone)] -pub struct LineAnnotation { - pub anchor_char_idx: usize, - pub height: usize, +/// The core of this trait `insert_virtual_lines` function. It is called at the +/// end of every visual line and allows the `LineAnnotation` to insert empty +/// virtual lines. Apart from that the `LineAnnotation` trait has multiple +/// methods that allow it to track anchors in the document. +/// +/// When a new traversal of a document starts `reset_pos` is called. Afterwards +/// the other functions are called with indices that are larger then the +/// one passed to `reset_pos`. This allows performing a binary search (use +/// `partition_point`) in `reset_pos` once and then to only look at the next +/// anchor during each method call. +/// +/// The `reset_pos`, `skip_conceal` and `process_anchor` functions all return a +/// `char_idx` anchor. This anchor is stored when transversing the document and +/// when the grapheme at the anchor is traversed the `process_anchor` function +/// is called. +/// +/// # Note +/// +/// All functions only receive immutable references to `self`. +/// `LineAnnotation`s that want to store an internal position or +/// state of some kind should use `Cell`. Using interior mutability for +/// caches is preferable as otherwise a lot of lifetimes become invariant +/// which complicates APIs a lot. +pub trait LineAnnotation { + /// Resets the internal position to `char_idx`. This function is called + //// when a new transveral of a document starts. + /// + /// All `char_idx` passed to `insert_virtual_lines` are strictly monotonically increasing + /// with the first `char_idx` greater or equal to the `char_idx` + /// passed to this function. + /// + /// # Returns + /// + /// The `char_idx` of the next anchor this `LineAnnotation` is interested in, + /// replaces the currently registered anchor. Return `usize::MAX` to ignore + fn reset_pos(&mut self, _char_idx: usize) -> usize { + usize::MAX + } + + /// Called when a text is concealed that contains an anchor registered by this `LineAnnotation` + ///. In this case the line decorations **must** ensure that virtual text anchored within that + /// char range is skipped. + /// + /// # Returns + /// + /// The `char_idx` of the next anchor this `LineAnnotation` is interested in, + /// **after the end of conceal_end_char_idx** + /// replaces the currently registered anchor. Return `usize::MAX` to ignore + fn skip_concealed_anchors(&mut self, conceal_end_char_idx: usize) -> usize { + self.reset_pos(conceal_end_char_idx) + } + + /// Process an anchor (horizontal position is provided) and returns the next anchor. + /// + /// # Returns + /// + /// The `char_idx` of the next anchor this `LineAnnotation` is interested in, + /// replaces the currently registered anchor. Return `usize::MAX` to ignore + fn process_anchor(&mut self, _grapheme: &FormattedGrapheme) -> usize { + usize::MAX + } + + /// This function is called at the end of a visual line to insert virtual text + /// + /// # Returns + /// + /// The number of additional virtual lines to reserve + /// + /// # Note + /// + /// The `line_end_visual_pos` paramater indiciates the visual vertical distance + /// from the start of block where the transversal start. This includes the offset + /// from other `LineAnnotations`. This allows inline annotations to consider + /// the height of the text and "align" two different documents (like for side + /// by side diffs). These annotations that want to "align" two documents should + /// therefore be added last so that other virtual text is also considered while aligning + fn insert_virtual_lines( + &mut self, + line_end_char_idx: usize, + line_end_visual_pos: Position, + doc_line: usize, + ) -> Position; } #[derive(Debug)] @@ -143,13 +226,67 @@ fn reset_pos(layers: &[Layer], pos: usize, get_pos: impl Fn(&A) -> u } } +/// Safety: We store LineAnnotation in a NonNull pointer. This is necessary to work +/// around an unfortunate inconsistency in rusts variance system that unnnecesarily +/// makes the lifetime invariant if implemented with safe code. This makes the +/// DocFormatter API very cumbersome/basically impossible to work with. +/// +/// Normally object types dyn Foo + 'a so if we used Box below +/// everything would be alright. However we want to use `Cell>` +/// to be able to call the mutable function on `LineAnnotation`. The problem is that +/// some types like `Cell` make all their arguments invariant. This is important for soundness +/// normally for the same reasons that &'a mut T is invariant over T +/// (see ). However for &'a mut (dyn Foo + 'b) +/// there is a specical rule in the language to make 'b covariant (otherwise trait objects would be +/// super annoying to use). See for +/// why this is sound. Sadly that rule doesn't apply to Cell +/// (or other invariant types like `UnsafeCell` or `*mut (dyn Foo + 'a)`). +/// +/// We sidestep the problem by using `NonNull` which is covariant. In the specical case +/// of trait objects this is sound (easily checked by adding a `PhantomData<&'a mut Foo + 'a)>` field). +/// We don't need an explicit Cell type here because we never hand out any refereces +/// to the trait objects so even without guarding mutable access to the pointer data behind a `` +/// are covariant over 'a (or in other words it's a raw pointer, as long as we don't hand out +/// references we are free to do whatever we want). +struct RawBox(NonNull); + +impl RawBox { + /// Safety: Only a single mutable reference + /// created by this function may exist at a given time. + #[allow(clippy::mut_from_ref)] + unsafe fn get(&self) -> &mut T { + &mut *self.0.as_ptr() + } +} +impl From> for RawBox { + fn from(box_: Box) -> Self { + // obviously safe because Box::into_raw never returns null + unsafe { Self(NonNull::new_unchecked(Box::into_raw(box_))) } + } +} + +impl Drop for RawBox { + fn drop(&mut self) { + unsafe { drop(Box::from_raw(self.0.as_ptr())) } + } +} + /// Annotations that change that is displayed when the document is render. /// Also commonly called virtual text. -#[derive(Default, Debug, Clone)] +#[derive(Default)] pub struct TextAnnotations<'a> { inline_annotations: Vec>>, overlays: Vec>>, - line_annotations: Vec>, + line_annotations: Vec<(Cell, RawBox)>, +} + +impl Debug for TextAnnotations<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TextAnnotations") + .field("inline_annotations", &self.inline_annotations) + .field("overlays", &self.overlays) + .finish_non_exhaustive() + } } impl<'a> TextAnnotations<'a> { @@ -157,9 +294,9 @@ impl<'a> TextAnnotations<'a> { pub fn reset_pos(&self, char_idx: usize) { reset_pos(&self.inline_annotations, char_idx, |annot| annot.char_idx); reset_pos(&self.overlays, char_idx, |annot| annot.char_idx); - reset_pos(&self.line_annotations, char_idx, |annot| { - annot.anchor_char_idx - }); + for (next_anchor, layer) in &self.line_annotations { + next_anchor.set(unsafe { layer.get().reset_pos(char_idx) }); + } } pub fn collect_overlay_highlights( @@ -219,8 +356,9 @@ impl<'a> TextAnnotations<'a> { /// /// The line annotations **must be sorted** by their `char_idx`. /// Multiple line annotations with the same `char_idx` **are not allowed**. - pub fn add_line_annotation(&mut self, layer: &'a [LineAnnotation]) -> &mut Self { - self.line_annotations.push((layer, ()).into()); + pub fn add_line_annotation(&mut self, layer: Box) -> &mut Self { + self.line_annotations + .push((Cell::new(usize::MAX), layer.into())); self } @@ -250,21 +388,35 @@ impl<'a> TextAnnotations<'a> { overlay } - pub(crate) fn annotation_lines_at(&self, char_idx: usize) -> usize { - self.line_annotations - .iter() - .map(|layer| { - let mut lines = 0; - while let Some(annot) = layer.annotations.get(layer.current_index.get()) { - if annot.anchor_char_idx == char_idx { - layer.current_index.set(layer.current_index.get() + 1); - lines += annot.height - } else { - break; + pub(crate) fn process_virtual_text_anchors(&self, grapheme: &FormattedGrapheme) { + for (next_anchor, layer) in &self.line_annotations { + loop { + match next_anchor.get().cmp(&grapheme.char_idx) { + Ordering::Less => next_anchor + .set(unsafe { layer.get().skip_concealed_anchors(grapheme.char_idx) }), + Ordering::Equal => { + next_anchor.set(unsafe { layer.get().process_anchor(grapheme) }) } - } - lines - }) - .sum() + Ordering::Greater => break, + }; + } + } + } + + pub(crate) fn virtual_lines_at( + &self, + char_idx: usize, + line_end_visual_pos: Position, + doc_line: usize, + ) -> usize { + let mut virt_off = Position::new(0, 0); + for (_, layer) in &self.line_annotations { + virt_off += unsafe { + layer + .get() + .insert_virtual_lines(char_idx, line_end_visual_pos + virt_off, doc_line) + }; + } + virt_off.row } } From 57ec259d1ed522725fb1ccdd07dc0c2a10a3e8eb Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:06 +0100 Subject: [PATCH 05/13] correctly wrap at text-width --- helix-core/src/doc_formatter.rs | 86 +++++++++++++++++++++------- helix-core/src/doc_formatter/test.rs | 20 +++++++ helix-view/src/document.rs | 16 +++--- 3 files changed, 94 insertions(+), 28 deletions(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index 0f9a52b5d3e5..030bfe1f60a1 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -10,6 +10,7 @@ //! called a "block" and the caller must advance it as needed. use std::borrow::Cow; +use std::cmp::Ordering; use std::fmt::Debug; use std::mem::replace; @@ -43,6 +44,11 @@ impl GraphemeSource { matches!(self, GraphemeSource::VirtualText { .. }) } + pub fn is_eof(self) -> bool { + // all doc chars except the EOF char have non-zero codepoints + matches!(self, GraphemeSource::Document { codepoints: 0 }) + } + pub fn doc_chars(self) -> usize { match self { GraphemeSource::Document { codepoints } => codepoints as usize, @@ -117,6 +123,14 @@ impl<'a> GraphemeWithSource<'a> { self.grapheme.is_whitespace() } + fn is_newline(&self) -> bool { + matches!(self.grapheme, Grapheme::Newline) + } + + fn is_eof(&self) -> bool { + self.source.is_eof() + } + fn width(&self) -> usize { self.grapheme.width() } @@ -135,6 +149,7 @@ pub struct TextFormat { pub wrap_indicator: Box, pub wrap_indicator_highlight: Option, pub viewport_width: u16, + pub soft_wrap_at_text_width: bool, } // test implementation is basically only used for testing or when softwrap is always disabled @@ -148,6 +163,7 @@ impl Default for TextFormat { wrap_indicator: Box::from(" "), viewport_width: 17, wrap_indicator_highlight: None, + soft_wrap_at_text_width: false, } } } @@ -318,39 +334,68 @@ impl<'t> DocumentFormatter<'t> { .change_position(visual_x, self.text_fmt.tab_width); word_width += grapheme.width(); } + if let Some(grapheme) = &mut self.peeked_grapheme { + let visual_x = self.visual_pos.col + word_width; + grapheme + .grapheme + .change_position(visual_x, self.text_fmt.tab_width); + } word_width } + fn peek_grapheme(&mut self, col: usize, char_pos: usize) -> Option<&GraphemeWithSource<'t>> { + if self.peeked_grapheme.is_none() { + self.peeked_grapheme = self.advance_grapheme(col, char_pos); + } + self.peeked_grapheme.as_ref() + } + + fn next_grapheme(&mut self, col: usize, char_pos: usize) -> Option> { + self.peek_grapheme(col, char_pos); + self.peeked_grapheme.take() + } + fn advance_to_next_word(&mut self) { self.word_buf.clear(); let mut word_width = 0; let mut word_chars = 0; + if self.exhausted { + return; + } + loop { - // softwrap word if necessary - if word_width + self.visual_pos.col >= self.text_fmt.viewport_width as usize { - // wrapping this word would move too much text to the next line - // split the word at the line end instead - if word_width > self.text_fmt.max_wrap as usize { - // Usually we stop accomulating graphemes as soon as softwrapping becomes necessary. - // However if the last grapheme is multiple columns wide it might extend beyond the EOL. - // The condition below ensures that this grapheme is not cutoff and instead wrapped to the next line - if word_width + self.visual_pos.col > self.text_fmt.viewport_width as usize { - self.peeked_grapheme = self.word_buf.pop(); - } + let mut col = self.visual_pos.col + word_width; + let char_pos = self.char_pos + word_chars; + match col.cmp(&(self.text_fmt.viewport_width as usize)) { + // The EOF char and newline chars are always selectable in helix. That means + // that wrapping happens "too-early" if a word fits a line perfectly. This + // is intentional so that all selectable graphemes are always visisble (and + // therefore the cursor never dissapears). However if the user manually set a + // lower softwrap width then this is underisable. Just increasing the viewport- + // width by one doesn't work because if a line is wrapped multiple times then + // some words may extend past the specified width. + // + // So we specialcase a word that ends exactly at linebounds and is followed + // by a newline/eof character here. + Ordering::Equal + if self.text_fmt.soft_wrap_at_text_width + && self.peek_grapheme(col, char_pos).map_or(false, |grapheme| { + grapheme.is_newline() || grapheme.is_eof() + }) => {} + Ordering::Equal if word_width > self.text_fmt.max_wrap as usize => return, + Ordering::Greater if word_width > self.text_fmt.max_wrap as usize => { + self.peeked_grapheme = self.word_buf.pop(); return; } - - word_width = self.wrap_word(); + Ordering::Equal | Ordering::Greater => { + word_width = self.wrap_word(); + col = self.visual_pos.col + word_width; + } + Ordering::Less => (), } - let grapheme = if let Some(grapheme) = self.peeked_grapheme.take() { - grapheme - } else if let Some(grapheme) = - self.advance_grapheme(self.visual_pos.col + word_width, self.char_pos + word_chars) - { - grapheme - } else { + let Some(grapheme) = self.next_grapheme(col, char_pos) else { return; }; word_chars += grapheme.doc_chars(); @@ -376,7 +421,6 @@ impl<'t> DocumentFormatter<'t> { pub fn next_char_pos(&self) -> usize { self.char_pos } - /// returns the visual position at the end of the last yielded grapheme pub fn next_visual_pos(&self) -> Position { self.visual_pos diff --git a/helix-core/src/doc_formatter/test.rs b/helix-core/src/doc_formatter/test.rs index b214ab944027..415ce8f6a131 100644 --- a/helix-core/src/doc_formatter/test.rs +++ b/helix-core/src/doc_formatter/test.rs @@ -12,6 +12,7 @@ impl TextFormat { wrap_indicator_highlight: None, // use a prime number to allow lining up too often with repeat viewport_width: 17, + soft_wrap_at_text_width: false, } } } @@ -21,6 +22,7 @@ impl<'t> DocumentFormatter<'t> { use std::fmt::Write; let mut res = String::new(); let viewport_width = self.text_fmt.viewport_width; + let soft_wrap_at_text_width = self.text_fmt.soft_wrap_at_text_width; let mut line = 0; for grapheme in self { @@ -28,6 +30,8 @@ impl<'t> DocumentFormatter<'t> { line += 1; assert_eq!(grapheme.visual_pos.row, line); write!(res, "\n{}", ".".repeat(grapheme.visual_pos.col)).unwrap(); + } + if !soft_wrap_at_text_width { assert!( grapheme.visual_pos.col <= viewport_width as usize, "softwrapped failed {}<={viewport_width}", @@ -98,6 +102,22 @@ fn long_word_softwrap() { ); } +fn softwrap_text_at_text_width(text: &str) -> String { + let mut text_fmt = TextFormat::new_test(true); + text_fmt.soft_wrap_at_text_width = true; + let annotations = TextAnnotations::default(); + let mut formatter = + DocumentFormatter::new_at_prev_checkpoint(text.into(), &text_fmt, &annotations, 0); + formatter.collect_to_str() +} +#[test] +fn long_word_softwrap_text_width() { + assert_eq!( + softwrap_text_at_text_width("xxxxxxxx1xxxx2xxx\nxxxxxxxx1xxxx2xxx"), + "xxxxxxxx1xxxx2xxx \nxxxxxxxx1xxxx2xxx " + ); +} + fn overlay_text(text: &str, char_pos: usize, softwrap: bool, overlays: &[Overlay]) -> String { DocumentFormatter::new_at_prev_checkpoint( text.into(), diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 0c3a75f1bb0e..7090963ef3b8 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1918,7 +1918,7 @@ impl Document { .language_config() .and_then(|config| config.text_width) .unwrap_or(config.text_width); - let soft_wrap_at_text_width = self + let mut soft_wrap_at_text_width = self .language_config() .and_then(|config| { config @@ -1929,12 +1929,13 @@ impl Document { .or(config.soft_wrap.wrap_at_text_width) .unwrap_or(false); if soft_wrap_at_text_width { - // We increase max_line_len by 1 because softwrap considers the newline character - // as part of the line length while the "typical" expectation is that this is not the case. - // In particular other commands like :reflow do not count the line terminator. - // This is technically inconsistent for the last line as that line never has a line terminator - // but having the last visual line exceed the width by 1 seems like a rare edge case. - viewport_width = viewport_width.min(text_width as u16 + 1) + // if the viewport is smaller than the specified + // width then this setting has no effcet + if text_width >= viewport_width as usize { + soft_wrap_at_text_width = false; + } else { + viewport_width = text_width as u16; + } } let config = self.config.load(); let editor_soft_wrap = &config.soft_wrap; @@ -1971,6 +1972,7 @@ impl Document { wrap_indicator_highlight: theme .and_then(|theme| theme.find_scope_index("ui.virtual.wrap")) .map(Highlight), + soft_wrap_at_text_width, } } From 9cf1c86d4a5b627b23cb3b5b42f04066ff970abc Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:07 +0100 Subject: [PATCH 06/13] streamline text decoration API This commit brings the text decoration API inline with the LineAnnotation API (so they are consistent) resulting in a single streamlined API instead of multiple ADHOK callbacks. --- helix-term/src/application.rs | 2 +- helix-term/src/ui/document.rs | 325 +++++++++++--------------- helix-term/src/ui/editor.rs | 74 +++--- helix-term/src/ui/mod.rs | 1 + helix-term/src/ui/picker.rs | 14 +- helix-term/src/ui/text_decorations.rs | 173 ++++++++++++++ helix-view/src/editor.rs | 39 +++- 7 files changed, 377 insertions(+), 251 deletions(-) create mode 100644 helix-term/src/ui/text_decorations.rs diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 809393c7fd7b..2697948dc2b8 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -278,7 +278,7 @@ impl Application { self.compositor.render(area, surface, &mut cx); let (pos, kind) = self.compositor.cursor(area, &self.editor); // reset cursor cache - self.editor.cursor_cache.set(None); + self.editor.cursor_cache.reset(); let pos = pos.map(|pos| (pos.col as u16, pos.row as u16)); self.terminal.draw(pos, kind).unwrap(); diff --git a/helix-term/src/ui/document.rs b/helix-term/src/ui/document.rs index fee08ed3317f..f7f20a2d9a82 100644 --- a/helix-term/src/ui/document.rs +++ b/helix-term/src/ui/document.rs @@ -11,26 +11,10 @@ use helix_view::editor::{WhitespaceConfig, WhitespaceRenderValue}; use helix_view::graphics::Rect; use helix_view::theme::Style; use helix_view::view::ViewPosition; -use helix_view::Document; -use helix_view::Theme; +use helix_view::{Document, Theme}; use tui::buffer::Buffer as Surface; -pub trait LineDecoration { - fn render_background(&mut self, _renderer: &mut TextRenderer, _pos: LinePos) {} - fn render_foreground( - &mut self, - _renderer: &mut TextRenderer, - _pos: LinePos, - _end_char_idx: usize, - ) { - } -} - -impl LineDecoration for F { - fn render_background(&mut self, renderer: &mut TextRenderer, pos: LinePos) { - self(renderer, pos) - } -} +use crate::ui::text_decorations::DecorationManager; /// A wrapper around a HighlightIterator /// that merges the layered highlights to create the final text style @@ -78,15 +62,8 @@ pub struct LinePos { pub doc_line: usize, /// Vertical offset from the top of the inner view area pub visual_line: u16, - /// The first char index of this visual line. - /// Note that if the visual line is entirely filled by - /// a very long inline virtual text then this index will point - /// at the next (non-virtual) char after this visual line - pub start_char_idx: usize, } -pub type TranslatedPosition<'a> = (usize, Box); - #[allow(clippy::too_many_arguments)] pub fn render_document( surface: &mut Surface, @@ -97,84 +74,46 @@ pub fn render_document( syntax_highlight_iter: impl Iterator, overlay_highlight_iter: impl Iterator, theme: &Theme, - line_decoration: &mut [Box], - translated_positions: &mut [TranslatedPosition], + decorations: DecorationManager, ) { - let mut renderer = TextRenderer::new(surface, doc, theme, offset.horizontal_offset, viewport); + let mut renderer = TextRenderer::new( + surface, + doc, + theme, + Position::new(offset.vertical_offset, offset.horizontal_offset), + viewport, + ); render_text( &mut renderer, doc.text().slice(..), - offset, + offset.anchor, &doc.text_format(viewport.width, Some(theme)), doc_annotations, syntax_highlight_iter, overlay_highlight_iter, theme, - line_decoration, - translated_positions, + decorations, ) } -fn translate_positions( - char_pos: usize, - first_visible_char_idx: usize, - translated_positions: &mut [TranslatedPosition], - text_fmt: &TextFormat, - renderer: &mut TextRenderer, - pos: Position, -) { - // check if any positions translated on the fly (like cursor) has been reached - for (char_idx, callback) in &mut *translated_positions { - if *char_idx < char_pos && *char_idx >= first_visible_char_idx { - // by replacing the char_index with usize::MAX large number we ensure - // that the same position is only translated once - // text will never reach usize::MAX as rust memory allocations are limited - // to isize::MAX - *char_idx = usize::MAX; - - if text_fmt.soft_wrap { - callback(renderer, pos) - } else if pos.col >= renderer.col_offset - && pos.col - renderer.col_offset < renderer.viewport.width as usize - { - callback( - renderer, - Position { - row: pos.row, - col: pos.col - renderer.col_offset, - }, - ) - } - } - } -} - #[allow(clippy::too_many_arguments)] pub fn render_text<'t>( renderer: &mut TextRenderer, - text: RopeSlice<'t>, - offset: ViewPosition, + text: RopeSlice<'_>, + anchor: usize, text_fmt: &TextFormat, text_annotations: &TextAnnotations, syntax_highlight_iter: impl Iterator, overlay_highlight_iter: impl Iterator, theme: &Theme, - line_decorations: &mut [Box], - translated_positions: &mut [TranslatedPosition], + mut decorations: DecorationManager, ) { - let mut row_off = visual_offset_from_block( - text, - offset.anchor, - offset.anchor, - text_fmt, - text_annotations, - ) - .0 - .row; - row_off += offset.vertical_offset; + let row_off = visual_offset_from_block(text, anchor, anchor, text_fmt, text_annotations) + .0 + .row; let mut formatter = - DocumentFormatter::new_at_prev_checkpoint(text, text_fmt, text_annotations, offset.anchor); + DocumentFormatter::new_at_prev_checkpoint(text, text_fmt, text_annotations, anchor); let mut syntax_styles = StyleIter { text_style: renderer.text_style, active_highlights: Vec::with_capacity(64), @@ -192,8 +131,8 @@ pub fn render_text<'t>( first_visual_line: false, doc_line: usize::MAX, visual_line: u16::MAX, - start_char_idx: usize::MAX, }; + let mut last_line_end = 0; let mut is_in_indent_area = true; let mut last_line_indent_level = 0; let mut syntax_style_span = syntax_styles @@ -202,58 +141,22 @@ pub fn render_text<'t>( let mut overlay_style_span = overlay_styles .next() .unwrap_or_else(|| (Style::default(), usize::MAX)); - let mut first_visible_char_idx = formatter.next_char_pos(); + let mut reached_view_top = false; loop { - // formattter.line_pos returns to line index of the next grapheme - // so it must be called before formatter.next let Some(mut grapheme) = formatter.next() else { - let mut last_pos = formatter.next_visual_pos(); - if last_pos.row >= row_off { - last_pos.col -= 1; - last_pos.row -= row_off; - // check if any positions translated on the fly (like cursor) are at the EOF - translate_positions( - text.len_chars() + 1, - first_visible_char_idx, - translated_positions, - text_fmt, - renderer, - last_pos, - ); - } break; }; // skip any graphemes on visual lines before the block start - // if pos.row < row_off { - // if char_pos >= syntax_style_span.1 { - // syntax_style_span = if let Some(syntax_style_span) = syntax_styles.next() { - // syntax_style_span - // } else { - // break; - // } - // } - // if char_pos >= overlay_style_span.1 { - // overlay_style_span = if let Some(overlay_style_span) = overlay_styles.next() { - // overlay_style_span if grapheme.visual_pos.row < row_off { - if grapheme.char_idx >= style_span.1 { - style_span = if let Some(style_span) = styles.next() { - style_span - } else { - break; - }; - overlay_span = if let Some(overlay_span) = overlays.next() { - overlay_span - } else { - break; - }; - } - first_visible_char_idx = formatter.next_char_pos(); continue; } grapheme.visual_pos.row -= row_off; + if !reached_view_top { + decorations.prepare_for_rendering(grapheme.char_idx); + reached_view_top = true; + } // if the end of the viewport is reached stop rendering if grapheme.visual_pos.row as u16 >= renderer.viewport.height + renderer.offset.row as u16 { @@ -262,87 +165,67 @@ pub fn render_text<'t>( // apply decorations before rendering a new line if grapheme.visual_pos.row as u16 != last_line_pos.visual_line { - if grapheme.visual_pos.row > 0 { + // we initiate doc_line with usize::MAX because no file + // can reach that size (memory allocations are limited to isize::MAX) + // initially there is no "previous" line (so doc_line is set to usize::MAX) + // in that case we don't need to draw indent guides/virtual text + if last_line_pos.doc_line != usize::MAX { // draw indent guides for the last line - renderer - .draw_indent_guides(last_line_indent_level, last_line_pos.visual_line as u16); + renderer.draw_indent_guides(last_line_indent_level, last_line_pos.visual_line); is_in_indent_area = true; - for line_decoration in &mut *line_decorations { - line_decoration.render_foreground(renderer, last_line_pos, grapheme.char_idx); - } + decorations.render_virtual_lines(renderer, last_line_pos, last_line_end) } last_line_pos = LinePos { first_visual_line: grapheme.line_idx != last_line_pos.doc_line, doc_line: grapheme.line_idx, visual_line: grapheme.visual_pos.row as u16, - start_char_idx: grapheme.char_idx, }; - for line_decoration in &mut *line_decorations { - line_decoration.render_background(renderer, last_line_pos); - } + decorations.decorate_line(renderer, last_line_pos); } // acquire the correct grapheme style - while grapheme.char_idx >= syntax_style_span.1 { + while grapheme.char_idx >= syntax_style_span.1 { syntax_style_span = syntax_styles .next() .unwrap_or((Style::default(), usize::MAX)); } - while grapheme.char_idx >= overlay_style_span.1 { + while grapheme.char_idx >= overlay_style_span.1 { overlay_style_span = overlay_styles .next() .unwrap_or((Style::default(), usize::MAX)); } - // check if any positions translated on the fly (like cursor) has been reached - translate_positions( - formatter.next_char_pos(), - first_visible_char_idx, - translated_positions, - text_fmt, - renderer, - grapheme.visual_pos, - ); - - let (syntax_style, overlay_style) = - if let GraphemeSource::VirtualText { highlight } = grapheme.source { - let mut style = renderer.text_style; - if let Some(highlight) = highlight { - style = style.patch(theme.highlight(highlight.0)) - } - (style, Style::default()) - } else { - (syntax_style_span.0, overlay_style_span.0) - }; - - let is_virtual = grapheme.is_virtual(); - renderer.draw_grapheme( -<<<<<<< HEAD - grapheme.grapheme, + let grapheme_style = if let GraphemeSource::VirtualText { highlight } = grapheme.source { + let mut style = renderer.text_style; + if let Some(highlight) = highlight { + style = style.patch(theme.highlight(highlight.0)); + } GraphemeStyle { - syntax_style, - overlay_style, - }, - is_virtual, -||||||| parent of 5e32edd8 (track char_idx in DocFormatter) - grapheme.grapheme, - grapheme_style, - virt, -======= + syntax_style: style, + overlay_style: Style::default(), + } + } else { + GraphemeStyle { + syntax_style: syntax_style_span.0, + overlay_style: overlay_style_span.0, + } + }; + decorations.decorate_grapheme(renderer, &grapheme); + + let virt = grapheme.is_virtual(); + let grapheme_width = renderer.draw_grapheme( grapheme.raw, grapheme_style, virt, ->>>>>>> 5e32edd8 (track char_idx in DocFormatter) &mut last_line_indent_level, &mut is_in_indent_area, grapheme.visual_pos, ); + last_line_end = grapheme.visual_pos.col + grapheme_width; } renderer.draw_indent_guides(last_line_indent_level, last_line_pos.visual_line); - for line_decoration in &mut *line_decorations { - line_decoration.render_foreground(renderer, last_line_pos, formatter.next_char_pos()); - } + decorations.render_virtual_lines(renderer, last_line_pos, last_line_end) } #[derive(Debug)] @@ -361,8 +244,8 @@ pub struct TextRenderer<'a> { pub indent_width: u16, pub starting_indent: usize, pub draw_indent_guides: bool, - pub col_offset: usize, pub viewport: Rect, + pub offset: Position, } pub struct GraphemeStyle { @@ -375,7 +258,7 @@ impl<'a> TextRenderer<'a> { surface: &'a mut Surface, doc: &Document, theme: &Theme, - col_offset: usize, + offset: Position, viewport: Rect, ) -> TextRenderer<'a> { let editor_config = doc.config.load(); @@ -430,8 +313,8 @@ impl<'a> TextRenderer<'a> { virtual_tab, whitespace_style: theme.get("ui.virtual.whitespace"), indent_width, - starting_indent: col_offset / indent_width as usize - + (col_offset % indent_width as usize != 0) as usize + starting_indent: offset.col / indent_width as usize + + (offset.col % indent_width as usize != 0) as usize + editor_config.indent_guides.skip_levels as usize, indent_guide_style: text_style.patch( theme @@ -441,7 +324,7 @@ impl<'a> TextRenderer<'a> { text_style, draw_indent_guides: editor_config.indent_guides.render, viewport, - col_offset, + offset, } } @@ -453,9 +336,13 @@ impl<'a> TextRenderer<'a> { is_virtual: bool, last_indent_level: &mut usize, is_in_indent_area: &mut bool, - position: Position, - ) { - let cut_off_start = self.col_offset.saturating_sub(position.col); + mut position: Position, + ) -> usize { + if position.row < self.offset.row { + return 0; + } + position.row -= self.offset.row; + let cut_off_start = self.offset.col.saturating_sub(position.col); let is_whitespace = grapheme.is_whitespace(); // TODO is it correct to apply the whitespace style to all unicode white spaces? @@ -487,12 +374,11 @@ impl<'a> TextRenderer<'a> { Grapheme::Newline => &self.newline, }; - let in_bounds = self.col_offset <= position.col - && position.col < self.viewport.width as usize + self.col_offset; + let in_bounds = self.column_in_bounds(position.col + width - 1); if in_bounds { self.surface.set_string( - self.viewport.x + (position.col - self.col_offset) as u16, + self.viewport.x + (position.col - self.offset.col) as u16, self.viewport.y + position.row as u16, grapheme, style, @@ -512,26 +398,33 @@ impl<'a> TextRenderer<'a> { *last_indent_level = position.col; *is_in_indent_area = false; } + + width + } + + pub fn column_in_bounds(&self, colum: usize) -> bool { + self.offset.col <= colum && colum < self.viewport.width as usize + self.offset.col } /// Overlay indentation guides ontop of a rendered line /// The indentation level is computed in `draw_lines`. /// Therefore this function must always be called afterwards. - pub fn draw_indent_guides(&mut self, indent_level: usize, row: u16) { - if !self.draw_indent_guides { + pub fn draw_indent_guides(&mut self, indent_level: usize, mut row: u16) { + if !self.draw_indent_guides || self.offset.row > row as usize { return; } + row -= self.offset.row as u16; // Don't draw indent guides outside of view let end_indent = min( indent_level, // Add indent_width - 1 to round up, since the first visible // indent might be a bit after offset.col - self.col_offset + self.viewport.width as usize + (self.indent_width as usize - 1), + self.offset.col + self.viewport.width as usize + (self.indent_width as usize - 1), ) / self.indent_width as usize; for i in self.starting_indent..end_indent { - let x = (self.viewport.x as usize + (i * self.indent_width as usize) - self.col_offset) + let x = (self.viewport.x as usize + (i * self.indent_width as usize) - self.offset.col) as u16; let y = self.viewport.y + row; debug_assert!(self.surface.in_bounds(x, y)); @@ -539,4 +432,62 @@ impl<'a> TextRenderer<'a> { .set_string(x, y, &self.indent_guide_char, self.indent_guide_style); } } + + pub fn set_string(&mut self, x: u16, y: u16, string: impl AsRef, style: Style) { + if (y as usize) < self.offset.row { + return; + } + self.surface + .set_string(x, y + self.viewport.y, string, style) + } + + pub fn set_stringn( + &mut self, + x: u16, + y: u16, + string: impl AsRef, + width: usize, + style: Style, + ) { + if (y as usize) < self.offset.row { + return; + } + self.surface + .set_stringn(x, y + self.viewport.y, string, width, style); + } + + /// Sets the style of an area **within the text viewport* this accounts + /// both for the renderers vertical offset and its viewport + pub fn set_style(&mut self, mut area: Rect, style: Style) { + area = area.clip_top(self.offset.row as u16); + area.y += self.viewport.y; + self.surface.set_style(area, style); + } + + /// Sets the style of an area **within the text viewport* this accounts + /// both for the renderers vertical offset and its viewport + #[allow(clippy::too_many_arguments)] + pub fn set_string_truncated( + &mut self, + x: u16, + y: u16, + string: &str, + width: usize, + style: impl Fn(usize) -> Style, // Map a grapheme's string offset to a style + ellipsis: bool, + truncate_start: bool, + ) -> (u16, u16) { + if (y as usize) < self.offset.row { + return (x, y); + } + self.surface.set_string_truncated( + x, + y + self.viewport.y, + string, + width, + style, + ellipsis, + truncate_start, + ) + } } diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index ad7aa5c5a89a..e77a1628fab7 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -5,8 +5,10 @@ use crate::{ key, keymap::{KeymapResult, Keymaps}, ui::{ - document::{render_document, LinePos, TextRenderer, TranslatedPosition}, - Completion, ProgressSpinners, + document::{render_document, LinePos, TextRenderer}, + statusline, + text_decorations::{self, Decoration, DecorationManager, InlineDiagnostics}, + Completion, CompletionItem, ProgressSpinners, }, }; @@ -33,9 +35,6 @@ use std::{mem::take, num::NonZeroUsize, path::PathBuf, rc::Rc, sync::Arc}; use tui::{buffer::Buffer as Surface, text::Span}; -use super::document::LineDecoration; -use super::{completion::CompletionItem, statusline}; - pub struct EditorView { pub keymaps: Keymaps, on_next_key: Option, @@ -96,11 +95,10 @@ impl EditorView { let config = editor.config(); let text_annotations = view.text_annotations(doc, Some(theme)); - let mut line_decorations: Vec> = Vec::new(); - let mut translated_positions: Vec = Vec::new(); + let mut decorations = DecorationManager::default(); if is_focused && config.cursorline { - line_decorations.push(Self::cursorline_decorator(doc, view, theme)) + decorations.add_decoration(Self::cursorline(doc, view, theme)); } if is_focused && config.cursorcolumn { @@ -115,13 +113,10 @@ impl EditorView { if pos.doc_line != dap_line { return; } - renderer.surface.set_style( - Rect::new(inner.x, inner.y + pos.visual_line, inner.width, 1), - style, - ); + renderer.set_style(Rect::new(inner.x, pos.visual_line, inner.width, 1), style); }; - line_decorations.push(Box::new(line_decoration)); + decorations.add_decoration(line_decoration); } let syntax_highlights = @@ -178,22 +173,20 @@ impl EditorView { view.area, theme, is_focused & self.terminal_focused, - &mut line_decorations, + &mut decorations, ); } + let primary_cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); if is_focused { - let cursor = doc - .selection(view.id) - .primary() - .cursor(doc.text().slice(..)); - // set the cursor_cache to out of view in case the position is not found - editor.cursor_cache.set(Some(None)); - let update_cursor_cache = - |_: &mut TextRenderer, pos| editor.cursor_cache.set(Some(Some(pos))); - translated_positions.push((cursor, Box::new(update_cursor_cache))); + decorations.add_decoration(text_decorations::Cursor { + cache: &editor.cursor_cache, + primary_cursor, + }); } - render_document( surface, inner, @@ -203,8 +196,7 @@ impl EditorView { syntax_highlights, overlay_highlights, theme, - &mut line_decorations, - &mut translated_positions, + decorations, ); Self::render_rulers(editor, doc, view, inner, surface, theme); @@ -618,7 +610,7 @@ impl EditorView { viewport: Rect, theme: &Theme, is_focused: bool, - line_decorations: &mut Vec>, + decoration_manager: &mut DecorationManager<'d>, ) { let text = doc.text().slice(..); let cursors: Rc<[_]> = doc @@ -644,7 +636,7 @@ impl EditorView { // TODO handle softwrap in gutters let selected = cursors.contains(&pos.doc_line); let x = viewport.x + offset; - let y = viewport.y + pos.visual_line; + let y = pos.visual_line; let gutter_style = match (selected, pos.first_visual_line) { (false, true) => gutter_style, @@ -656,11 +648,9 @@ impl EditorView { if let Some(style) = gutter(pos.doc_line, selected, pos.first_visual_line, &mut text) { - renderer - .surface - .set_stringn(x, y, &text, width, gutter_style.patch(style)); + renderer.set_stringn(x, y, &text, width, gutter_style.patch(style)); } else { - renderer.surface.set_style( + renderer.set_style( Rect { x, y, @@ -672,7 +662,7 @@ impl EditorView { } text.clear(); }; - line_decorations.push(Box::new(gutter_decoration)); + decoration_manager.add_decoration(gutter_decoration); offset += width as u16; } @@ -742,11 +732,7 @@ impl EditorView { } /// Apply the highlighting on the lines where a cursor is active - pub fn cursorline_decorator( - doc: &Document, - view: &View, - theme: &Theme, - ) -> Box { + pub fn cursorline(doc: &Document, view: &View, theme: &Theme) -> impl Decoration { let text = doc.text().slice(..); // TODO only highlight the visual line that contains the cursor instead of the full visual line let primary_line = doc.selection(view.id).primary().cursor_line(text); @@ -767,16 +753,14 @@ impl EditorView { let secondary_style = theme.get("ui.cursorline.secondary"); let viewport = view.area; - let line_decoration = move |renderer: &mut TextRenderer, pos: LinePos| { - let area = Rect::new(viewport.x, viewport.y + pos.visual_line, viewport.width, 1); + move |renderer: &mut TextRenderer, pos: LinePos| { + let area = Rect::new(viewport.x, pos.visual_line, viewport.width, 1); if primary_line == pos.doc_line { - renderer.surface.set_style(area, primary_style); + renderer.set_style(area, primary_style); } else if secondary_lines.binary_search(&pos.doc_line).is_ok() { - renderer.surface.set_style(area, secondary_style); + renderer.set_style(area, secondary_style); } - }; - - Box::new(line_decoration) + } } /// Apply the highlighting on the columns where a cursor is active diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index b5969818cf56..b8e96974d577 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -12,6 +12,7 @@ mod prompt; mod spinner; mod statusline; mod text; +mod text_decorations; use crate::compositor::{Component, Compositor}; use crate::filter_picker_entry; diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index c2728888a1c4..04c8e8489d65 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -6,7 +6,8 @@ use crate::{ key, shift, ui::{ self, - document::{render_document, LineDecoration, LinePos, TextRenderer}, + document::{render_document, LinePos, TextRenderer}, + text_decorations::DecorationManager, EditorView, }, }; @@ -761,7 +762,7 @@ impl Picker { } overlay_highlights = Box::new(helix_core::syntax::merge(overlay_highlights, spans)); } - let mut decorations: Vec> = Vec::new(); + let mut decorations = DecorationManager::default(); if let Some((start, end)) = range { let style = cx @@ -773,14 +774,14 @@ impl Picker { if (start..=end).contains(&pos.doc_line) { let area = Rect::new( renderer.viewport.x, - renderer.viewport.y + pos.visual_line, + pos.visual_line, renderer.viewport.width, 1, ); - renderer.surface.set_style(area, style) + renderer.set_style(area, style) } }; - decorations.push(Box::new(draw_highlight)) + decorations.add_decoration(draw_highlight); } render_document( @@ -793,8 +794,7 @@ impl Picker { syntax_highlights, overlay_highlights, &cx.editor.theme, - &mut decorations, - &mut [], + decorations, ); } } diff --git a/helix-term/src/ui/text_decorations.rs b/helix-term/src/ui/text_decorations.rs new file mode 100644 index 000000000000..cc4be2ddf91f --- /dev/null +++ b/helix-term/src/ui/text_decorations.rs @@ -0,0 +1,173 @@ +use std::cmp::Ordering; + +use helix_core::doc_formatter::FormattedGrapheme; +use helix_core::Position; +use helix_view::editor::CursorCache; + +use crate::ui::document::{LinePos, TextRenderer}; + +pub use diagnostics::InlineDiagnostics; + +mod diagnostics; + +/// Decorations are the primary mechanisim for extending the text rendering. +/// +/// Any on-screen element which is anchored to the rendered text in some form should +/// be implemented using this trait. Translating char positions to +/// on-screen positions can be expensive and should not be done during rendering. +/// Instead such translations are performed on the fly while the text is being rendered. +/// The results are provided to this trait +/// +/// To reserve space for virtual text lines (which is then filled by this trait) emit appropriate +/// [`LineAnnotation`](helix_core::text_annotations::LineAnnotation) in [`helix_view::View::text_annotations`] +pub trait Decoration { + /// Called **before** a **visual** line is rendered. A visual line does not + /// necessairly correspond to a single line in a document as soft wrapping can + /// spread a single document line across multiple visual lines. + /// + /// This function is called before text is rendered as any decorations should + /// never overlap the document text. That means that setting the forground color + /// here is (essentially) useless as the text color is overwritten by the + /// rendered text. This -ofcourse- doesn't apply when rendering inside virtual lines + /// below the line reserved by `LineAnnotation`s. e as no text will be rendered here. + fn decorate_line(&mut self, _renderer: &mut TextRenderer, _pos: LinePos) {} + + /// Called **after** a **visual** line is rendered. A visual line does not + /// necessairly correspond to a single line in a document as soft wrapping can + /// spread a single document line across multiple visual lines. + /// + /// This function is called after text is rendered so that decorations can collect + /// horizontal positions on the line (see [`Decoration::decorate_grapheme`]) first and + /// use those positions` while rendering + /// virtual text. + /// That means that setting the forground color + /// here is (essentially) useless as the text color is overwritten by the + /// rendered text. This -ofcourse- doesn't apply when rendering inside virtual lines + /// below the line reserved by `LineAnnotation`s. e as no text will be rendered here. + /// **Note**: To avoid overlapping decorations in the virtual lines, each decoration + /// must return the number of virtual text lines it has taken up. Each `Decoration` recieves + /// an offset `virt_off` based on these return values where it can render virtual text: + /// + /// That means that a `render_line` implementation that returns `X` can render virtual text + /// in the following area: + /// ``` no-compile + /// let start = inner.y + pos.virtual_line + virt_off; + /// start .. start + X + /// ```` + fn render_virt_lines( + &mut self, + _renderer: &mut TextRenderer, + _pos: LinePos, + _virt_off: Position, + ) -> Position { + Position::new(0, 0) + } + + fn reset_pos(&mut self, _pos: usize) -> usize { + usize::MAX + } + + fn skip_concealed_anchor(&mut self, conceal_end_char_idx: usize) -> usize { + self.reset_pos(conceal_end_char_idx) + } + + /// This function is called **before** the grapheme at `char_idx` is rendered. + /// + /// # Returns + /// The next + fn decorate_grapheme( + &mut self, + _renderer: &mut TextRenderer, + _grapheme: &FormattedGrapheme, + ) -> usize { + usize::MAX + } +} + +impl Decoration for F { + fn decorate_line(&mut self, renderer: &mut TextRenderer, pos: LinePos) { + self(renderer, pos); + } +} + +#[derive(Default)] +pub struct DecorationManager<'a> { + decorations: Vec<(Box, usize)>, +} + +impl<'a> DecorationManager<'a> { + pub fn add_decoration(&mut self, decoration: impl Decoration + 'a) { + self.decorations.push((Box::new(decoration), 0)); + } + + pub fn prepare_for_rendering(&mut self, first_visible_char: usize) { + for (decoration, next_position) in &mut self.decorations { + *next_position = decoration.reset_pos(first_visible_char) + } + } + + pub fn decorate_grapheme(&mut self, renderer: &mut TextRenderer, grapheme: &FormattedGrapheme) { + for (decoration, hook_char_idx) in &mut self.decorations { + loop { + match (*hook_char_idx).cmp(&grapheme.char_idx) { + // this grapheme has been concealed or we are at the first grapheme + Ordering::Less => { + *hook_char_idx = decoration.skip_concealed_anchor(grapheme.char_idx) + } + Ordering::Equal => { + *hook_char_idx = decoration.decorate_grapheme(renderer, grapheme) + } + Ordering::Greater => break, + } + } + } + } + + pub fn decorate_line(&mut self, renderer: &mut TextRenderer, pos: LinePos) { + for (decoration, _) in &mut self.decorations { + decoration.decorate_line(renderer, pos); + } + } + + pub fn render_virtual_lines( + &mut self, + renderer: &mut TextRenderer, + pos: LinePos, + line_width: usize, + ) { + let mut virt_off = Position::new(1, line_width); // start at 1 so the line is never overwritten + for (decoration, _) in &mut self.decorations { + virt_off += decoration.render_virt_lines(renderer, pos, virt_off); + } + } +} + +/// Cursor rendering is done externally so all the cursor decoration +/// does is save the position of primary cursor +pub struct Cursor<'a> { + pub cache: &'a CursorCache, + pub primary_cursor: usize, +} +impl Decoration for Cursor<'_> { + fn reset_pos(&mut self, pos: usize) -> usize { + if pos <= self.primary_cursor { + self.primary_cursor + } else { + usize::MAX + } + } + + fn decorate_grapheme( + &mut self, + renderer: &mut TextRenderer, + grapheme: &FormattedGrapheme, + ) -> usize { + if renderer.column_in_bounds(grapheme.visual_pos.col) + && renderer.offset.row < grapheme.visual_pos.row + { + let position = grapheme.visual_pos - renderer.offset; + self.cache.set(Some(position)); + } + usize::MAX + } +} diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index dd360a78ebdd..87cdb9ca2d7c 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -997,10 +997,10 @@ pub struct Editor { /// This cache is only a performance optimization to /// avoid calculating the cursor position multiple /// times during rendering and should not be set by other functions. - pub cursor_cache: Cell>>, pub handlers: Handlers, pub mouse_down_range: Option, + pub cursor_cache: CursorCache, } pub type Motion = Box; @@ -1115,9 +1115,9 @@ impl Editor { exit_code: 0, config_events: unbounded_channel(), needs_redraw: false, - cursor_cache: Cell::new(None), handlers, mouse_down_range: None, + cursor_cache: CursorCache::default(), } } @@ -1905,15 +1905,7 @@ impl Editor { pub fn cursor(&self) -> (Option, CursorKind) { let config = self.config(); let (view, doc) = current_ref!(self); - let cursor = doc - .selection(view.id) - .primary() - .cursor(doc.text().slice(..)); - let pos = self - .cursor_cache - .get() - .unwrap_or_else(|| view.screen_coords_at_pos(doc, doc.text().slice(..), cursor)); - if let Some(mut pos) = pos { + if let Some(mut pos) = self.cursor_cache.get(view, doc) { let inner = view.inner_area(doc); pos.col += inner.x as usize; pos.row += inner.y as usize; @@ -2108,3 +2100,28 @@ fn try_restore_indent(doc: &mut Document, view: &mut View) { doc.apply(&transaction, view.id); } } + +#[derive(Default)] +pub struct CursorCache(Cell>>); + +impl CursorCache { + pub fn get(&self, view: &View, doc: &Document) -> Option { + if let Some(pos) = self.0.get() { + return pos; + } + + let text = doc.text().slice(..); + let cursor = doc.selection(view.id).primary().cursor(text); + let res = view.screen_coords_at_pos(doc, text, cursor); + self.set(res); + res + } + + pub fn set(&self, cursor_pos: Option) { + self.0.set(Some(cursor_pos)) + } + + pub fn reset(&self) { + self.0.set(None) + } +} From a241c529d6b8ff0b0dbb52886199ac7464e9bdc8 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:07 +0100 Subject: [PATCH 07/13] fix typo in doc_formatter.rs --- helix-core/src/doc_formatter.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index 030bfe1f60a1..7bb00938016b 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -182,7 +182,7 @@ pub struct DocumentFormatter<'t> { line_pos: usize, exhausted: bool, - inline_anntoation_graphemes: Option<(Graphemes<'t>, Option)>, + inline_annotation_graphemes: Option<(Graphemes<'t>, Option)>, // softwrap specific /// The indentation of the current line @@ -227,7 +227,7 @@ impl<'t> DocumentFormatter<'t> { word_buf: Vec::with_capacity(64), word_i: 0, line_pos: block_line_idx, - inline_anntoation_graphemes: None, + inline_annotation_graphemes: None, } } @@ -237,7 +237,7 @@ impl<'t> DocumentFormatter<'t> { ) -> Option<(&'t str, Option)> { loop { if let Some(&mut (ref mut annotation, highlight)) = - self.inline_anntoation_graphemes.as_mut() + self.inline_annotation_graphemes.as_mut() { if let Some(grapheme) = annotation.next() { return Some((grapheme, highlight)); @@ -247,7 +247,7 @@ impl<'t> DocumentFormatter<'t> { if let Some((annotation, highlight)) = self.annotations.next_inline_annotation_at(char_pos) { - self.inline_anntoation_graphemes = Some(( + self.inline_annotation_graphemes = Some(( UnicodeSegmentation::graphemes(&*annotation.text, true), highlight, )) From 960b67f3896b214141d6d21427d82c52cbcc310e Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 29 Jan 2024 17:07:16 +0100 Subject: [PATCH 08/13] stable sort diagnostics to avoid flickering --- helix-term/src/application.rs | 7 +++---- helix-view/src/document.rs | 5 ++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 2697948dc2b8..9eb5a966d7ed 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -759,7 +759,7 @@ impl Application { // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order params .diagnostics - .sort_unstable_by_key(|d| (d.severity, d.range.start)); + .sort_by_key(|d| (d.severity, d.range.start)); } for source in &lang_conf.persistent_diagnostic_sources { let new_diagnostics = params @@ -800,9 +800,8 @@ impl Application { // Sort diagnostics first by severity and then by line numbers. // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order - diagnostics.sort_unstable_by_key(|(d, server_id)| { - (d.severity, d.range.start, *server_id) - }); + diagnostics + .sort_by_key(|(d, server_id)| (d.severity, d.range.start, *server_id)); if let Some(doc) = doc { let diagnostic_of_language_server_and_not_in_unchanged_sources = diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 7090963ef3b8..68d2a7148d8b 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -97,7 +97,6 @@ impl Serialize for Mode { serializer.collect_str(self) } } - /// A snapshot of the text of a document that we want to write out to disk #[derive(Debug, Clone)] pub struct DocumentSavedEvent { @@ -1292,7 +1291,7 @@ impl Document { true }); - self.diagnostics.sort_unstable_by_key(|diagnostic| { + self.diagnostics.sort_by_key(|diagnostic| { ( diagnostic.range, diagnostic.severity, @@ -1872,7 +1871,7 @@ impl Document { }); } self.diagnostics.extend(diagnostics); - self.diagnostics.sort_unstable_by_key(|diagnostic| { + self.diagnostics.sort_by_key(|diagnostic| { ( diagnostic.range, diagnostic.severity, From 139c73169dc3212ba085b757f9052a07ca08a69f Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 29 Jan 2024 17:11:00 +0100 Subject: [PATCH 09/13] render diagnostic inline --- book/src/configuration.md | 1 + book/src/themes.md | 1 + helix-core/src/diagnostic.rs | 16 +- helix-core/src/graphemes.rs | 5 + helix-core/src/lib.rs | 4 +- helix-core/src/position.rs | 11 + helix-term/src/commands.rs | 1 + helix-term/src/ui/document.rs | 42 ++- helix-term/src/ui/editor.rs | 10 +- .../src/ui/text_decorations/diagnostics.rs | 291 ++++++++++++++++++ helix-view/src/annotations.rs | 1 + helix-view/src/annotations/diagnostics.rs | 267 ++++++++++++++++ helix-view/src/editor.rs | 7 + helix-view/src/lib.rs | 1 + helix-view/src/view.rs | 63 ++-- 15 files changed, 692 insertions(+), 29 deletions(-) create mode 100644 helix-term/src/ui/text_decorations/diagnostics.rs create mode 100644 helix-view/src/annotations.rs create mode 100644 helix-view/src/annotations/diagnostics.rs diff --git a/book/src/configuration.md b/book/src/configuration.md index 5e22cebf8f66..69137d7432bf 100644 --- a/book/src/configuration.md +++ b/book/src/configuration.md @@ -139,6 +139,7 @@ The following statusline elements can be configured: | `display-signature-help-docs` | Display docs under signature help popup | `true` | | `snippets` | Enables snippet completions. Requires a server restart (`:lsp-restart`) to take effect after `:config-reload`/`:set`. | `true` | | `goto-reference-include-declaration` | Include declaration in the goto references popup. | `true` | +| `display-inline-diagnostics` | Display diagnostics under their starting line | `true` | [^1]: By default, a progress spinner is shown in the statusline beside the file path. diff --git a/book/src/themes.md b/book/src/themes.md index 0a49053f5921..5108d9169171 100644 --- a/book/src/themes.md +++ b/book/src/themes.md @@ -309,6 +309,7 @@ These scopes are used for theming the editor interface: | `ui.text.inactive` | Same as `ui.text` but when the text is inactive (e.g. suggestions) | | `ui.text.info` | The key: command text in `ui.popup.info` boxes | | `ui.virtual.ruler` | Ruler columns (see the [`editor.rulers` config][editor-section]) | +| `ui.virtual.diagnostics` | Default style for inline diagnostics lines (notably control the background) | | `ui.virtual.whitespace` | Visible whitespace characters | | `ui.virtual.indent-guide` | Vertical indent width guides | | `ui.virtual.inlay-hint` | Default style for inlay hints of all kinds | diff --git a/helix-core/src/diagnostic.rs b/helix-core/src/diagnostic.rs index 52d77a9aace5..b65fc5ee23eb 100644 --- a/helix-core/src/diagnostic.rs +++ b/helix-core/src/diagnostic.rs @@ -2,7 +2,8 @@ use serde::{Deserialize, Serialize}; /// Describes the severity level of a [`Diagnostic`]. -#[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] +#[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] pub enum Severity { Hint, Info, @@ -23,6 +24,12 @@ pub struct Range { pub end: usize, } +impl Range { + pub fn contains(self, pos: usize) -> bool { + (self.start..self.end).contains(&pos) + } +} + #[derive(Debug, Eq, Hash, PartialEq, Clone, Deserialize, Serialize)] pub enum NumberOrString { Number(i32), @@ -52,3 +59,10 @@ pub struct Diagnostic { pub source: Option, pub data: Option, } + +impl Diagnostic { + #[inline] + pub fn severity(&self) -> Severity { + self.severity.unwrap_or(Severity::Warning) + } +} diff --git a/helix-core/src/graphemes.rs b/helix-core/src/graphemes.rs index 7cb5cd0625d5..e5ae13738676 100644 --- a/helix-core/src/graphemes.rs +++ b/helix-core/src/graphemes.rs @@ -28,6 +28,11 @@ pub enum Grapheme<'a> { } impl<'a> Grapheme<'a> { + pub fn new_decoration(g: &'static str) -> Grapheme<'a> { + assert_ne!(g, "\t"); + Grapheme::new(g.into(), 0, 0) + } + pub fn new(g: GraphemeStr<'a>, visual_x: usize, tab_width: u16) -> Grapheme<'a> { match g { g if g == "\t" => Grapheme::Tab { diff --git a/helix-core/src/lib.rs b/helix-core/src/lib.rs index 1abd90d10b21..eaec4b28b427 100644 --- a/helix-core/src/lib.rs +++ b/helix-core/src/lib.rs @@ -52,8 +52,8 @@ pub use {regex, tree_sitter}; pub use graphemes::RopeGraphemes; pub use position::{ - char_idx_at_visual_offset, coords_at_pos, pos_at_coords, visual_offset_from_anchor, - visual_offset_from_block, Position, VisualOffsetError, + char_idx_at_visual_offset, coords_at_pos, pos_at_coords, softwrapped_dimensions, + visual_offset_from_anchor, visual_offset_from_block, Position, VisualOffsetError, }; #[allow(deprecated)] pub use position::{pos_at_visual_coords, visual_coords_at_pos}; diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs index 0860da12f1df..3719abb0b1ef 100644 --- a/helix-core/src/position.rs +++ b/helix-core/src/position.rs @@ -171,6 +171,17 @@ pub fn visual_offset_from_block( (last_pos, block_start) } +/// Returns the height of the given text when softwrapping +pub fn softwrapped_dimensions(text: RopeSlice, text_fmt: &TextFormat) -> (usize, u16) { + let last_pos = + visual_offset_from_block(text, 0, usize::MAX, text_fmt, &TextAnnotations::default()).0; + if last_pos.row == 0 { + (1, last_pos.col as u16) + } else { + (last_pos.row + 1, text_fmt.viewport_width) + } +} + #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum VisualOffsetError { PosBeforeAnchorRow, diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index d0b9047c8ad6..9c069cf01309 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1681,6 +1681,7 @@ pub fn scroll(cx: &mut Context, offset: usize, direction: Direction, sync_cursor &mut annotations, ) }); + drop(annotations); doc.set_selection(view.id, selection); return; } diff --git a/helix-term/src/ui/document.rs b/helix-term/src/ui/document.rs index f7f20a2d9a82..218f2562ed42 100644 --- a/helix-term/src/ui/document.rs +++ b/helix-term/src/ui/document.rs @@ -97,7 +97,7 @@ pub fn render_document( } #[allow(clippy::too_many_arguments)] -pub fn render_text<'t>( +pub fn render_text( renderer: &mut TextRenderer, text: RopeSlice<'_>, anchor: usize, @@ -327,6 +327,46 @@ impl<'a> TextRenderer<'a> { offset, } } + /// Draws a single `grapheme` at the current render position with a specified `style`. + pub fn draw_decoration_grapheme( + &mut self, + grapheme: Grapheme, + mut style: Style, + mut row: u16, + col: u16, + ) -> bool { + if (row as usize) < self.offset.row + || row >= self.viewport.height + || col >= self.viewport.width + { + return false; + } + row -= self.offset.row as u16; + let is_whitespace = grapheme.is_whitespace(); + + // TODO is it correct to apply the whitspace style to all unicode white spaces? + if is_whitespace { + style = style.patch(self.whitespace_style); + } + + let grapheme = match grapheme { + Grapheme::Tab { width } => { + let grapheme_tab_width = char_to_byte_idx(&self.virtual_tab, width); + &self.virtual_tab[..grapheme_tab_width] + } + Grapheme::Other { ref g } if g == "\u{00A0}" => " ", + Grapheme::Other { ref g } => g, + Grapheme::Newline => " ", + }; + + self.surface.set_string( + self.viewport.x + col, + self.viewport.y + row, + grapheme, + style, + ); + true + } /// Draws a single `grapheme` at the current render position with a specified `style`. pub fn draw_grapheme( diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index e77a1628fab7..46228bb17d7d 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -187,6 +187,12 @@ impl EditorView { primary_cursor, }); } + decorations.add_decoration(InlineDiagnostics::new( + doc, + theme, + primary_cursor, + config.lsp.inline_diagnostics.clone(), + )); render_document( surface, inner, @@ -212,7 +218,9 @@ impl EditorView { } } - Self::render_diagnostics(doc, view, inner, surface, theme); + if config.lsp.display_diagnostic_message { + Self::render_diagnostics(doc, view, inner, surface, theme); + } let statusline_area = view .area diff --git a/helix-term/src/ui/text_decorations/diagnostics.rs b/helix-term/src/ui/text_decorations/diagnostics.rs new file mode 100644 index 000000000000..d24e4ce776d2 --- /dev/null +++ b/helix-term/src/ui/text_decorations/diagnostics.rs @@ -0,0 +1,291 @@ +use std::cmp::Ordering; + +use helix_core::diagnostic::Severity; +use helix_core::doc_formatter::{DocumentFormatter, FormattedGrapheme}; +use helix_core::graphemes::Grapheme; +use helix_core::text_annotations::TextAnnotations; +use helix_core::{Diagnostic, Position}; +use helix_view::annotations::diagnostics::{InlineDiagnosticAccumulator, InlineDiagnosticsConfig}; + +use helix_view::theme::Style; +use helix_view::{Document, Theme}; + +use crate::ui::document::{LinePos, TextRenderer}; +use crate::ui::text_decorations::Decoration; + +#[derive(Debug)] +struct Styles { + hint: Style, + info: Style, + warning: Style, + error: Style, +} + +impl Styles { + fn new(theme: &Theme) -> Styles { + Styles { + hint: theme.get("hint"), + info: theme.get("info"), + warning: theme.get("warning"), + error: theme.get("error"), + } + } + + fn severity_style(&self, severity: Severity) -> Style { + match severity { + Severity::Hint => self.hint, + Severity::Info => self.info, + Severity::Warning => self.warning, + Severity::Error => self.error, + } + } +} + +pub struct InlineDiagnostics<'a> { + state: InlineDiagnosticAccumulator<'a>, + styles: Styles, +} + +impl<'a> InlineDiagnostics<'a> { + pub fn new( + doc: &'a Document, + theme: &Theme, + cursor: usize, + config: InlineDiagnosticsConfig, + ) -> Self { + InlineDiagnostics { + state: InlineDiagnosticAccumulator::new(cursor, doc, config), + styles: Styles::new(theme), + } + } +} + +const BL_CORNER: &str = "┘"; +const TR_CORNER: &str = "┌"; +const BR_CORNER: &str = "└"; +const STACK: &str = "├"; +const MULTI: &str = "┴"; +const HOR_BAR: &str = "─"; +const VER_BAR: &str = "│"; + +struct Renderer<'a, 'b> { + renderer: &'a mut TextRenderer<'b>, + first_row: u16, + row: u16, + config: &'a InlineDiagnosticsConfig, + styles: &'a Styles, +} + +impl Renderer<'_, '_> { + fn draw_decoration(&mut self, g: &'static str, severity: Severity, col: u16) { + self.draw_decoration_at(g, severity, col, self.row) + } + + fn draw_decoration_at(&mut self, g: &'static str, severity: Severity, col: u16, row: u16) { + self.renderer.draw_decoration_grapheme( + Grapheme::new_decoration(g), + self.styles.severity_style(severity), + row, + col, + ); + } + + fn draw_eol_diagnostic(&mut self, diag: &Diagnostic, row: u16, col: usize) -> u16 { + let style = self.styles.severity_style(diag.severity()); + let width = self.renderer.viewport.width; + if !self.renderer.column_in_bounds(col + 1) { + return 0; + } + let col = (col - self.renderer.offset.col) as u16; + let (new_col, _) = self.renderer.set_string_truncated( + self.renderer.viewport.x + col + 1, + row, + &diag.message, + width.saturating_sub(col + 1) as usize, + |_| style, + true, + false, + ); + new_col - col + } + + fn draw_diagnostic(&mut self, diag: &Diagnostic, col: u16, next_severity: Option) { + let severity = diag.severity(); + let (sym, sym_severity) = if let Some(next_severity) = next_severity { + (STACK, next_severity.max(severity)) + } else { + (BR_CORNER, severity) + }; + self.draw_decoration(sym, sym_severity, col); + for i in 0..self.config.prefix_len { + self.draw_decoration(HOR_BAR, severity, col + i + 1); + } + + let text_col = col + self.config.prefix_len + 1; + let text_fmt = self.config.text_fmt(text_col, self.renderer.viewport.width); + let annotations = TextAnnotations::default(); + let formatter = DocumentFormatter::new_at_prev_checkpoint( + diag.message.as_str().trim().into(), + &text_fmt, + &annotations, + 0, + ); + let mut last_row = 0; + let style = self.styles.severity_style(severity); + for grapheme in formatter { + last_row = grapheme.visual_pos.row; + self.renderer.draw_decoration_grapheme( + grapheme.raw, + style, + self.row + grapheme.visual_pos.row as u16, + text_col + grapheme.visual_pos.col as u16, + ); + } + self.row += 1; + // height is last_row + 1 and extra_rows is height - 1 + let extra_lines = last_row; + if let Some(next_severity) = next_severity { + for _ in 0..extra_lines { + self.draw_decoration(VER_BAR, next_severity, col); + self.row += 1; + } + } else { + self.row += extra_lines as u16; + } + } + + fn draw_multi_diagnostics(&mut self, stack: &mut Vec<(&Diagnostic, u16)>) { + let Some(&(last_diag, last_anchor)) = stack.last() else { + return; + }; + let start = self + .config + .max_diagnostic_start(self.renderer.viewport.width); + + if last_anchor <= start { + return; + } + let mut severity = last_diag.severity(); + let mut last_anchor = last_anchor; + self.draw_decoration(BL_CORNER, severity, last_anchor); + let mut stacked_diagnostics = 1; + for &(diag, anchor) in stack.iter().rev().skip(1) { + let sym = match anchor.cmp(&start) { + Ordering::Less => break, + Ordering::Equal => STACK, + Ordering::Greater => MULTI, + }; + stacked_diagnostics += 1; + severity = severity.max(diag.severity()); + let old_severity = severity; + if anchor == last_anchor && severity == old_severity { + continue; + } + for col in (anchor + 1)..last_anchor { + self.draw_decoration(HOR_BAR, old_severity, col) + } + self.draw_decoration(sym, severity, anchor); + last_anchor = anchor; + } + + // if no diagnostic anchor was found exactly at the start of the + // diagnostic text draw an upwards corner and ensure the last piece + // of the line is not missing + if last_anchor != start { + for col in (start + 1)..last_anchor { + self.draw_decoration(HOR_BAR, severity, col) + } + self.draw_decoration(TR_CORNER, severity, start) + } + self.row += 1; + let stacked_diagnostics = &stack[stack.len() - stacked_diagnostics..]; + + for (i, (diag, _)) in stacked_diagnostics.iter().rev().enumerate() { + let next_severity = stacked_diagnostics[..stacked_diagnostics.len() - i - 1] + .iter() + .map(|(diag, _)| diag.severity()) + .max(); + self.draw_diagnostic(diag, start, next_severity); + } + + stack.truncate(stack.len() - stacked_diagnostics.len()); + } + + fn draw_diagnostics(&mut self, stack: &mut Vec<(&Diagnostic, u16)>) { + let mut stack = stack.drain(..).rev().peekable(); + let mut last_anchor = self.renderer.viewport.width; + while let Some((diag, anchor)) = stack.next() { + if anchor != last_anchor { + for row in self.first_row..self.row { + self.draw_decoration_at(VER_BAR, diag.severity(), anchor, row); + } + } + let next_severity = stack.peek().and_then(|&(diag, next_anchor)| { + (next_anchor == anchor).then_some(diag.severity()) + }); + self.draw_diagnostic(diag, anchor, next_severity); + last_anchor = anchor; + } + } +} + +impl Decoration for InlineDiagnostics<'_> { + fn render_virt_lines( + &mut self, + renderer: &mut TextRenderer, + pos: LinePos, + virt_off: Position, + ) -> Position { + let mut col_off = 0; + let filter = self.state.filter(); + let eol_diagnostic = match filter { + Some(filter) => self + .state + .stack + .iter() + .filter(|(diag, _)| filter < diag.severity()) + .max_by_key(|(diagnostic, _)| diagnostic.severity), + None => self.state.stack.last(), + }; + if let Some((eol_diagnostic, _)) = eol_diagnostic { + let mut renderer = Renderer { + renderer, + first_row: pos.visual_line, + row: pos.visual_line, + config: &self.state.config, + styles: &self.styles, + }; + col_off = renderer.draw_eol_diagnostic(eol_diagnostic, pos.visual_line, virt_off.col); + } + + self.state.compute_line_diagnostics(); + let mut renderer = Renderer { + renderer, + first_row: pos.visual_line + virt_off.row as u16, + row: pos.visual_line + virt_off.row as u16, + config: &self.state.config, + styles: &self.styles, + }; + renderer.draw_multi_diagnostics(&mut self.state.stack); + renderer.draw_diagnostics(&mut self.state.stack); + let horizontal_off = renderer.row - renderer.first_row; + Position::new(horizontal_off as usize, col_off as usize) + } + + fn reset_pos(&mut self, pos: usize) -> usize { + self.state.reset_pos(pos) + } + + fn skip_concealed_anchor(&mut self, conceal_end_char_idx: usize) -> usize { + self.state.skip_concealed(conceal_end_char_idx) + } + + fn decorate_grapheme( + &mut self, + renderer: &mut TextRenderer, + grapheme: &FormattedGrapheme, + ) -> usize { + self.state + .proccess_anchor(grapheme, renderer.viewport.width, renderer.offset.col) + } +} diff --git a/helix-view/src/annotations.rs b/helix-view/src/annotations.rs new file mode 100644 index 000000000000..4c630487f1cb --- /dev/null +++ b/helix-view/src/annotations.rs @@ -0,0 +1 @@ +pub mod diagnostics; diff --git a/helix-view/src/annotations/diagnostics.rs b/helix-view/src/annotations/diagnostics.rs new file mode 100644 index 000000000000..503fbd3da2ef --- /dev/null +++ b/helix-view/src/annotations/diagnostics.rs @@ -0,0 +1,267 @@ +use helix_core::diagnostic::Severity; +use helix_core::doc_formatter::{FormattedGrapheme, TextFormat}; +use helix_core::text_annotations::LineAnnotation; +use helix_core::{softwrapped_dimensions, Diagnostic, Position}; +use serde::{Deserialize, Serialize}; + +use crate::Document; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(default, rename_all = "kebab-case", deny_unknown_fields)] +pub struct InlineDiagnosticsConfig { + pub cursor_line: Option, + pub other_lines: Option, + pub min_diagnostic_width: u16, + pub prefix_len: u16, + pub max_wrap: u16, + pub max_diagnostics: usize, +} + +impl InlineDiagnosticsConfig { + // last column where to start diagnostics + // every diagnostics that start afterwards will be displayed with a "backwards + // line" to ensure they are still rendered with `min_diagnostic_widht`. If `width` + // it too small to display diagnostics with atleast `min_diagnostic_width` space + // (or inline diagnostics are displed) `None` is returned. In that case inline + // diagnostics should not be shown + pub fn enable(&self, width: u16) -> bool { + let disabled = matches!( + self, + Self { + cursor_line: None, + other_lines: None, + .. + } + ); + !disabled && width >= self.min_diagnostic_width + self.prefix_len + } + + pub fn max_diagnostic_start(&self, width: u16) -> u16 { + width - self.min_diagnostic_width - self.prefix_len + } + + pub fn text_fmt(&self, anchor_col: u16, width: u16) -> TextFormat { + let width = if anchor_col > self.max_diagnostic_start(width) { + self.min_diagnostic_width + } else { + width - anchor_col - self.prefix_len + }; + + TextFormat { + soft_wrap: true, + tab_width: 4, + max_wrap: self.max_wrap.min(width / 4), + max_indent_retain: 0, + wrap_indicator: "".into(), + wrap_indicator_highlight: None, + viewport_width: width, + soft_wrap_at_text_width: true, + } + } +} + +impl Default for InlineDiagnosticsConfig { + fn default() -> Self { + InlineDiagnosticsConfig { + cursor_line: Some(Severity::Warning), + other_lines: None, + min_diagnostic_width: 40, + prefix_len: 1, + max_wrap: 20, + max_diagnostics: 20, + } + } +} + +pub struct InlineDiagnosticAccumulator<'a> { + idx: usize, + doc: &'a Document, + pub stack: Vec<(&'a Diagnostic, u16)>, + pub config: InlineDiagnosticsConfig, + cursor: usize, + cursor_line: bool, +} + +impl<'a> InlineDiagnosticAccumulator<'a> { + pub fn new(cursor: usize, doc: &'a Document, config: InlineDiagnosticsConfig) -> Self { + InlineDiagnosticAccumulator { + idx: 0, + doc, + stack: Vec::new(), + config, + cursor, + cursor_line: false, + } + } + + pub fn reset_pos(&mut self, char_idx: usize) -> usize { + self.idx = 0; + self.clear(); + self.skip_concealed(char_idx) + } + + pub fn skip_concealed(&mut self, conceal_end_char_idx: usize) -> usize { + let diagnostics = &self.doc.diagnostics[self.idx..]; + let idx = diagnostics.partition_point(|diag| diag.range.start < conceal_end_char_idx); + self.idx += idx; + self.next_anchor(conceal_end_char_idx) + } + + pub fn next_anchor(&self, current_char_idx: usize) -> usize { + let next_diag_start = self + .doc + .diagnostics + .get(self.idx) + .map_or(usize::MAX, |diag| diag.range.start); + if (current_char_idx..next_diag_start).contains(&self.cursor) { + self.cursor + } else { + next_diag_start + } + } + + pub fn clear(&mut self) { + self.cursor_line = false; + self.stack.clear(); + } + + fn process_anchor_impl( + &mut self, + grapheme: &FormattedGrapheme, + width: u16, + horizontal_off: usize, + ) -> bool { + // TODO: doing the cursor tracking here works well but is somewhat + // duplicate effort/tedious maybe centrilize this somehwere? + // In the DocFormatter? + if grapheme.char_idx == self.cursor { + self.cursor_line = true; + if self + .doc + .diagnostics + .get(self.idx) + .map_or(true, |diag| diag.range.start != grapheme.char_idx) + { + return false; + } + } + + let Some(anchor_col) = grapheme.visual_pos.col.checked_sub(horizontal_off) else { + return true; + }; + if anchor_col >= width as usize { + return true; + } + + for diag in &self.doc.diagnostics[self.idx..] { + if diag.range.start != grapheme.char_idx { + break; + } + self.stack.push((diag, anchor_col as u16)); + self.idx += 1; + } + false + } + + pub fn proccess_anchor( + &mut self, + grapheme: &FormattedGrapheme, + width: u16, + horizontal_off: usize, + ) -> usize { + if self.process_anchor_impl(grapheme, width, horizontal_off) { + self.idx += self.doc.diagnostics[self.idx..] + .iter() + .take_while(|diag| diag.range.start == grapheme.char_idx) + .count(); + } + self.next_anchor(grapheme.char_idx + 1) + } + + pub fn filter(&self) -> Option { + if self.cursor_line { + self.config.cursor_line + } else { + self.config.other_lines + } + } + + pub fn compute_line_diagnostics(&mut self) { + let filter = if self.cursor_line { + self.cursor_line = false; + self.config.cursor_line + } else { + self.config.other_lines + }; + let Some(filter) = filter else { + self.stack.clear(); + return; + }; + self.stack.retain(|(diag, _)| diag.severity() >= filter); + self.stack.truncate(self.config.max_diagnostics) + } + + pub fn has_multi(&self, width: u16) -> bool { + self.stack.last().map_or(false, |&(_, anchor)| { + anchor > self.config.max_diagnostic_start(width) + }) + } +} + +pub(crate) struct InlineDiagnostics<'a> { + state: InlineDiagnosticAccumulator<'a>, + width: u16, + horizontal_off: usize, +} + +impl<'a> InlineDiagnostics<'a> { + #[allow(clippy::new_ret_no_self)] + pub(crate) fn new( + doc: &'a Document, + cursor: usize, + width: u16, + horizontal_off: usize, + config: InlineDiagnosticsConfig, + ) -> Box { + Box::new(InlineDiagnostics { + state: InlineDiagnosticAccumulator::new(cursor, doc, config), + width, + horizontal_off, + }) + } +} + +impl LineAnnotation for InlineDiagnostics<'_> { + fn reset_pos(&mut self, char_idx: usize) -> usize { + self.state.reset_pos(char_idx) + } + + fn skip_concealed_anchors(&mut self, conceal_end_char_idx: usize) -> usize { + self.state.skip_concealed(conceal_end_char_idx) + } + + fn process_anchor(&mut self, grapheme: &FormattedGrapheme) -> usize { + self.state + .proccess_anchor(grapheme, self.width, self.horizontal_off) + } + + fn insert_virtual_lines( + &mut self, + _line_end_char_idx: usize, + _line_end_visual_pos: Position, + _doc_line: usize, + ) -> Position { + self.state.compute_line_diagnostics(); + let multi = self.state.has_multi(self.width); + let diagostic_height: usize = self + .state + .stack + .drain(..) + .map(|(diag, anchor)| { + let text_fmt = self.state.config.text_fmt(anchor, self.width); + softwrapped_dimensions(diag.message.as_str().trim().into(), &text_fmt).0 + }) + .sum(); + Position::new(multi as usize + diagostic_height, 0) + } +} diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 87cdb9ca2d7c..90d9e23f0bda 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1,5 +1,6 @@ use crate::{ align_view, + annotations::diagnostics::InlineDiagnosticsConfig, document::{DocumentSavedEventFuture, DocumentSavedEventResult, Mode, SavePoint}, graphics::{CursorKind, Rect}, handlers::Handlers, @@ -413,6 +414,10 @@ pub struct LspConfig { pub snippets: bool, /// Whether to include declaration in the goto reference query pub goto_reference_include_declaration: bool, + /// Display diagnostic on the same line they occur automatically. + /// Also called "error lens"-style diagnostics, in reference to the popular VSCode extension. + pub inline_diagnostics: InlineDiagnosticsConfig, + pub display_diagnostic_message: bool, } impl Default for LspConfig { @@ -425,6 +430,8 @@ impl Default for LspConfig { display_inlay_hints: false, snippets: true, goto_reference_include_declaration: true, + inline_diagnostics: InlineDiagnosticsConfig::default(), + display_diagnostic_message: false, } } } diff --git a/helix-view/src/lib.rs b/helix-view/src/lib.rs index 14b6e1ce8138..5628c830cfb7 100644 --- a/helix-view/src/lib.rs +++ b/helix-view/src/lib.rs @@ -1,6 +1,7 @@ #[macro_use] pub mod macros; +pub mod annotations; pub mod base64; pub mod clipboard; pub mod document; diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index b5dc0615a5af..03cf08bd3e95 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -1,5 +1,6 @@ use crate::{ align_view, + annotations::diagnostics::InlineDiagnostics, document::DocumentInlayHints, editor::{GutterConfig, GutterType}, graphics::Rect, @@ -422,37 +423,51 @@ impl View { text_annotations.add_overlay(labels, style); } - let DocumentInlayHints { + if let Some(DocumentInlayHints { id: _, type_inlay_hints, parameter_inlay_hints, other_inlay_hints, padding_before_inlay_hints, padding_after_inlay_hints, - } = match doc.inlay_hints.get(&self.id) { - Some(doc_inlay_hints) => doc_inlay_hints, - None => return text_annotations, - }; + }) = doc.inlay_hints.get(&self.id) + { + let type_style = theme + .and_then(|t| t.find_scope_index("ui.virtual.inlay-hint.type")) + .map(Highlight); + let parameter_style = theme + .and_then(|t| t.find_scope_index("ui.virtual.inlay-hint.parameter")) + .map(Highlight); + let other_style = theme + .and_then(|t| t.find_scope_index("ui.virtual.inlay-hint")) + .map(Highlight); - let type_style = theme - .and_then(|t| t.find_scope_index("ui.virtual.inlay-hint.type")) - .map(Highlight); - let parameter_style = theme - .and_then(|t| t.find_scope_index("ui.virtual.inlay-hint.parameter")) - .map(Highlight); - let other_style = theme - .and_then(|t| t.find_scope_index("ui.virtual.inlay-hint")) - .map(Highlight); - - // Overlapping annotations are ignored apart from the first so the order here is not random: - // types -> parameters -> others should hopefully be the "correct" order for most use cases, - // with the padding coming before and after as expected. - text_annotations - .add_inline_annotations(padding_before_inlay_hints, None) - .add_inline_annotations(type_inlay_hints, type_style) - .add_inline_annotations(parameter_inlay_hints, parameter_style) - .add_inline_annotations(other_inlay_hints, other_style) - .add_inline_annotations(padding_after_inlay_hints, None); + // Overlapping annotations are ignored apart from the first so the order here is not random: + // types -> parameters -> others should hopefully be the "correct" order for most use cases, + // with the padding coming before and after as expected. + text_annotations + .add_inline_annotations(padding_before_inlay_hints, None) + .add_inline_annotations(type_inlay_hints, type_style) + .add_inline_annotations(parameter_inlay_hints, parameter_style) + .add_inline_annotations(other_inlay_hints, other_style) + .add_inline_annotations(padding_after_inlay_hints, None); + }; + let width = self.inner_width(doc); + let config = doc.config.load(); + if config.lsp.inline_diagnostics.enable(width) { + let config = config.lsp.inline_diagnostics.clone(); + let cursor = doc + .selection(self.id) + .primary() + .cursor(doc.text().slice(..)); + text_annotations.add_line_annotation(InlineDiagnostics::new( + doc, + cursor, + width, + self.offset.horizontal_offset, + config, + )); + } text_annotations } From 55da3e48ea4629b7abbecf4527443913ea0d6a88 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:08 +0100 Subject: [PATCH 10/13] remove redudant/incorrect view bound check --- helix-view/src/view.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index 03cf08bd3e95..5c1ff2d0d6ec 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -377,11 +377,6 @@ impl View { text: RopeSlice, pos: usize, ) -> Option { - if pos < self.offset.anchor { - // Line is not visible on screen - return None; - } - let viewport = self.inner_area(doc); let text_fmt = doc.text_format(viewport.width, None); let annotations = self.text_annotations(doc, None); From bea18547078f464fd08954b05582af392730398a Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:08 +0100 Subject: [PATCH 11/13] use correct position for cursor in doc popup --- helix-term/src/ui/completion.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index 6cbb5b1095f4..74b8e361b09e 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -456,14 +456,12 @@ impl Component for Completion { // --- // option.documentation - let (view, doc) = current!(cx.editor); - let language = doc.language_name().unwrap_or(""); - let text = doc.text().slice(..); - let cursor_pos = doc.selection(view.id).primary().cursor(text); - let coords = view - .screen_coords_at_pos(doc, text, cursor_pos) - .expect("cursor must be in view"); + let Some(coords) = cx.editor.cursor().0 else { + return; + }; let cursor_pos = coords.row as u16; + let (_, doc) = current!(cx.editor); + let language = doc.language_name().unwrap_or(""); let markdowned = |lang: &str, detail: Option<&str>, doc: Option<&str>| { let md = match (detail, doc) { From b14b5818f9a2662bda7cefbaf30956b7e35f5a43 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:09 +0100 Subject: [PATCH 12/13] ignore empty virtual text layers --- helix-core/src/text_annotations.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/helix-core/src/text_annotations.rs b/helix-core/src/text_annotations.rs index 1efd2d679cbc..342e61f33191 100644 --- a/helix-core/src/text_annotations.rs +++ b/helix-core/src/text_annotations.rs @@ -333,7 +333,9 @@ impl<'a> TextAnnotations<'a> { layer: &'a [InlineAnnotation], highlight: Option, ) -> &mut Self { - self.inline_annotations.push((layer, highlight).into()); + if !layer.is_empty() { + self.inline_annotations.push((layer, highlight).into()); + } self } @@ -348,7 +350,9 @@ impl<'a> TextAnnotations<'a> { /// If multiple layers contain overlay at the same position /// the overlay from the layer added last will be show. pub fn add_overlay(&mut self, layer: &'a [Overlay], highlight: Option) -> &mut Self { - self.overlays.push((layer, highlight).into()); + if !layer.is_empty() { + self.overlays.push((layer, highlight).into()); + } self } From e86b77843b8756d53cc756837f91aa8d368495bc Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Tue, 2 Apr 2024 14:48:14 +0200 Subject: [PATCH 13/13] fix scrolling/movement for multiline virtual text --- helix-core/src/movement.rs | 10 ++++----- helix-core/src/position.rs | 45 +++++++++++++++++++++++++++++++------- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index 54eb02fd0b19..f5c2b2ed5882 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -79,19 +79,19 @@ pub fn move_vertically_visual( Direction::Backward => -(count as isize), }; - // TODO how to handle inline annotations that span an entire visual line (very unlikely). - // Compute visual offset relative to block start to avoid trasversing the block twice row_off += visual_pos.row as isize; - let new_pos = char_idx_at_visual_offset( + let (mut new_pos, virtual_rows) = char_idx_at_visual_offset( slice, block_off, row_off, new_col as usize, text_fmt, annotations, - ) - .0; + ); + if dir == Direction::Forward { + new_pos += (virtual_rows != 0) as usize; + } // Special-case to avoid moving to the end of the last non-empty line. if behaviour == Movement::Extend && slice.line(slice.char_to_line(new_pos)).len_chars() == 0 { diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs index 3719abb0b1ef..1b37891101dc 100644 --- a/helix-core/src/position.rs +++ b/helix-core/src/position.rs @@ -415,7 +415,7 @@ pub fn char_idx_at_visual_block_offset( let mut formatter = DocumentFormatter::new_at_prev_checkpoint(text, text_fmt, annotations, anchor); let mut last_char_idx = formatter.next_char_pos(); - let mut last_char_idx_on_line = None; + let mut found_non_virtual_on_row = false; let mut last_row = 0; for grapheme in &mut formatter { match grapheme.visual_pos.row.cmp(&row) { @@ -423,19 +423,23 @@ pub fn char_idx_at_visual_block_offset( if grapheme.visual_pos.col + grapheme.width() > column { if !grapheme.is_virtual() { return (grapheme.char_idx, 0); - } else if let Some(char_idx) = last_char_idx_on_line { - return (char_idx, 0); + } else if found_non_virtual_on_row { + return (last_char_idx, 0); } } else if !grapheme.is_virtual() { - last_char_idx_on_line = Some(grapheme.char_idx) + found_non_virtual_on_row = true; + last_char_idx = grapheme.char_idx; } } + Ordering::Greater if found_non_virtual_on_row => return (last_char_idx, 0), Ordering::Greater => return (last_char_idx, row - last_row), - _ => (), + Ordering::Less => { + if !grapheme.is_virtual() { + last_row = grapheme.visual_pos.row; + last_char_idx = grapheme.char_idx; + } + } } - - last_char_idx = grapheme.char_idx; - last_row = grapheme.visual_pos.row; } (formatter.next_char_pos(), 0) @@ -444,6 +448,7 @@ pub fn char_idx_at_visual_block_offset( #[cfg(test)] mod test { use super::*; + use crate::text_annotations::InlineAnnotation; use crate::Rope; #[test] @@ -804,6 +809,30 @@ mod test { assert_eq!(pos_at_visual_coords(slice, (10, 10).into(), 4), 0); } + #[test] + fn test_char_idx_at_visual_row_offset_inline_annotation() { + let text = Rope::from("foo\nbar"); + let slice = text.slice(..); + let mut text_fmt = TextFormat::default(); + let annotations = [InlineAnnotation { + text: "x".repeat(100).into(), + char_idx: 3, + }]; + text_fmt.soft_wrap = true; + + assert_eq!( + char_idx_at_visual_offset( + slice, + 0, + 1, + 0, + &text_fmt, + TextAnnotations::default().add_inline_annotations(&annotations, None) + ), + (2, 1) + ); + } + #[test] fn test_char_idx_at_visual_row_offset() { let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ\nfoo");