From 22dfad605a2cd429b0cb739fa3db7d0912fc9554 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:35:40 +0100 Subject: [PATCH 01/15] 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 ee764bc64..dba112121 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 2023445a08e7b67ccd0a772b5deb4c6a236a7843 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:05 +0100 Subject: [PATCH 02/15] 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 bcbaa3519..17695287c 100644 --- a/helix-term/src/ui/document.rs +++ b/helix-term/src/ui/document.rs @@ -68,10 +68,7 @@ impl> Iterator for StyleIter<'_, H> { HighlightEvent::HighlightEnd => { self.active_highlights.pop(); } - HighlightEvent::Source { start, mut end } => { - if start == end { - continue; - } + HighlightEvent::Source { mut end, .. } => { let style = self .active_highlights .iter() @@ -300,12 +297,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 e15626a00a5019e9fbc33e2f798ec7629faad4e9 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:06 +0100 Subject: [PATCH 03/15] 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 cbe2da3b6..f934b38a1 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 d2b6ddc74..b214ab944 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 dba112121..0860da12f 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 17695287c..34282b261 100644 --- a/helix-term/src/ui/document.rs +++ b/helix-term/src/ui/document.rs @@ -179,21 +179,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, @@ -226,19 +223,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, @@ -250,46 +247,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); @@ -297,26 +304,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) = @@ -332,27 +338,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 4c7cdb8feaf94e5f0ee993680fe9e26c0b9339eb Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:06 +0100 Subject: [PATCH 04/15] 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 | 221 ++++++++++++++++++++++++----- 2 files changed, 214 insertions(+), 55 deletions(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index f934b38a1..0f9a52b5d 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 1576914e3..785e38fd7 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 traversal 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` parameter indicates the visual vertical distance + /// from the start of block where the traversal starts. 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,68 @@ 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` are covariant 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 +/// special 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. That +/// means any reference to the pointer can create a valid multable reference +/// that is 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 +295,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 +357,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 +389,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 9a93240d27ac2cdd589526293d6901b2342c8576 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:06 +0100 Subject: [PATCH 05/15] 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 0f9a52b5d..65e1532f0 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 undesirable. 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 special case a word that ends exactly at line bounds 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 b214ab944..415ce8f6a 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 3314a243f..412f79fa2 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1953,7 +1953,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 @@ -1964,12 +1964,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; @@ -2006,6 +2007,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 2c0506aa9609033e3e61426d9658e3179107bb86 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:07 +0100 Subject: [PATCH 06/15] 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 | 175 ++++++++++++++ helix-view/src/editor.rs | 39 +++- 7 files changed, 379 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 9695703b7..3d10862d1 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -290,7 +290,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 34282b261..2da4d4b31 100644 --- a/helix-term/src/ui/document.rs +++ b/helix-term/src/ui/document.rs @@ -12,26 +12,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; #[derive(Debug, PartialEq, Eq, Clone, Copy)] enum StyleIterKind { @@ -95,15 +79,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, @@ -114,84 +91,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), @@ -213,8 +152,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 @@ -223,58 +162,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 { @@ -283,87 +186,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)] @@ -382,8 +265,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 { @@ -396,7 +279,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(); @@ -451,8 +334,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 @@ -462,7 +345,7 @@ impl<'a> TextRenderer<'a> { text_style, draw_indent_guides: editor_config.indent_guides.render, viewport, - col_offset, + offset, } } @@ -474,9 +357,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? @@ -508,12 +395,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, @@ -533,26 +419,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)); @@ -560,4 +453,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 a071bfaa8..49386b835 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, }, }; @@ -31,9 +33,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, @@ -94,11 +93,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 { @@ -113,13 +111,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 = @@ -176,22 +171,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, @@ -201,8 +194,7 @@ impl EditorView { syntax_highlights, overlay_highlights, theme, - &mut line_decorations, - &mut translated_positions, + decorations, ); Self::render_rulers(editor, doc, view, inner, surface, theme); @@ -637,7 +629,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 @@ -663,7 +655,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, @@ -675,11 +667,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, @@ -691,7 +681,7 @@ impl EditorView { } text.clear(); }; - line_decorations.push(Box::new(gutter_decoration)); + decoration_manager.add_decoration(gutter_decoration); offset += width as u16; } @@ -761,11 +751,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); @@ -786,16 +772,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 fae64062d..10f4104ab 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::Compositor; use crate::filter_picker_entry; diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 079012396..70be421ae 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -7,8 +7,9 @@ use crate::{ ctrl, key, shift, ui::{ self, - document::{render_document, LineDecoration, LinePos, TextRenderer}, + document::{render_document, LinePos, TextRenderer}, picker::query::PickerQuery, + text_decorations::DecorationManager, EditorView, }, }; @@ -895,7 +896,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 @@ -907,14 +908,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( @@ -927,8 +928,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 000000000..630af5817 --- /dev/null +++ b/helix-term/src/ui/text_decorations.rs @@ -0,0 +1,175 @@ +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 mechanism 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 manually in the +/// ui loop. Instead such translations are automatically performed on the fly +/// while the text is being rendered. The results are provided to this trait by +/// the rendering infrastructure. +/// +/// To reserve space for virtual text lines (which is then filled by this trait) emit appropriate +/// [`LineAnnotation`](helix_core::text_annotations::LineAnnotation)s in [`helix_view::View::text_annotations`] +pub trait Decoration { + /// Called **before** a **visual** line is rendered. A visual line does not + /// necessarily 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 _of course_ doesn't apply when rendering inside virtual lines + /// below the line reserved by `LineAnnotation`s 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 + /// necessarily 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 char idx of the next grapheme that this function should be called for + 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 290590950..fb8438be0 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1070,10 +1070,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; @@ -1188,9 +1188,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(), } } @@ -1985,15 +1985,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; @@ -2188,3 +2180,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 839f4d758df7793965a3c1c5c878d3b6a24a9919 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:07 +0100 Subject: [PATCH 07/15] 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 65e1532f0..3cfc15708 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 39b3d81abf4cc3aede4b739e9aaf6a83899cb22c Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 29 Jan 2024 17:07:16 +0100 Subject: [PATCH 08/15] stable sort diagnostics to avoid flickering --- helix-term/src/application.rs | 7 +++---- helix-view/src/document.rs | 8 +++----- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 3d10862d1..4c305f6ac 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -771,7 +771,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 @@ -812,9 +812,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 412f79fa2..f3ace89e5 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -99,7 +99,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 { @@ -1321,7 +1320,7 @@ impl Document { true }); - self.diagnostics.sort_unstable_by_key(|diagnostic| { + self.diagnostics.sort_by_key(|diagnostic| { (diagnostic.range, diagnostic.severity, diagnostic.provider) }); @@ -1911,9 +1910,8 @@ impl Document { }); } self.diagnostics.extend(diagnostics); - self.diagnostics.sort_unstable_by_key(|diagnostic| { - (diagnostic.range, diagnostic.severity, diagnostic.provider) - }); + self.diagnostics + .sort_by_key(|diagnostic| (diagnostic.range, diagnostic.severity, diagnostic.provider)); } /// clears diagnostics for a given language server id if set, otherwise all diagnostics are cleared From 6d051d7084ab4d75df7bb3e77cc64767c1d9b98e Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 29 Jan 2024 17:11:00 +0100 Subject: [PATCH 09/15] render diagnostic inline --- book/src/editor.md | 40 +++ 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 | 40 ++- helix-term/src/ui/editor.rs | 13 +- .../src/ui/text_decorations/diagnostics.rs | 305 +++++++++++++++++ helix-view/src/annotations.rs | 1 + helix-view/src/annotations/diagnostics.rs | 309 ++++++++++++++++++ helix-view/src/editor.rs | 6 + helix-view/src/lib.rs | 1 + helix-view/src/view.rs | 63 ++-- 14 files changed, 786 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/editor.md b/book/src/editor.md index ba03e90e5..82d5f8461 100644 --- a/book/src/editor.md +++ b/book/src/editor.md @@ -16,6 +16,7 @@ - [`[editor.gutters.spacer]` Section](#editorguttersspacer-section) - [`[editor.soft-wrap]` Section](#editorsoft-wrap-section) - [`[editor.smart-tab]` Section](#editorsmart-tab-section) +- [`[editor.inline-diagnostics]` Section](#editorinline-diagnostics-section) ### `[editor]` Section @@ -50,6 +51,7 @@ | `popup-border` | Draw border around `popup`, `menu`, `all`, or `none` | `none` | | `indent-heuristic` | How the indentation for a newly inserted line is computed: `simple` just copies the indentation level from the previous line, `tree-sitter` computes the indentation based on the syntax tree and `hybrid` combines both approaches. If the chosen heuristic is not available, a different one will be used as a fallback (the fallback order being `hybrid` -> `tree-sitter` -> `simple`). | `hybrid` | `jump-label-alphabet` | The characters that are used to generate two character jump labels. Characters at the start of the alphabet are used first. | `"abcdefghijklmnopqrstuvwxyz"` +| `end-of-line-diagnostics` | Minimum severity of diagnostics to render at the end of the line. Set to `disable` to disable entirely. Refer to the setting about `inline-diagnostics` for more details | "disable" ### `[editor.statusline]` Section @@ -393,3 +395,41 @@ S-tab = "move_parent_node_start" tab = "extend_parent_node_end" S-tab = "extend_parent_node_start" ``` + +### `[editor.inline-diagnostics]` Section + +Options for rendering diagnostics inside the text like shown below + +``` +fn main() { + let foo = bar; + └─ no such value in this scope +} +```` + +| Key | Description | Default | +|------------|-------------|---------| +| `cursor-line` | The minimum severity that a diagnostic must have to be shown inline on the line that contains the primary cursor. Set to `disable` to not show any diagnostics inline. This option does not have any effect when in insert-mode and will only take effect 350ms after moving the cursor to a different line. | `"disable"` | +| `other-lines` | The minimum severity that a diagnostic must have to be shown inline on a line that does not contain the cursor-line. Set to `disable` to not show any diagnostics inline. | `"disable"` | +| `prefix-len` | How many horizontal bars `─` are rendered before the diagnostic text. | `1` | +| `max-wrap` | Equivalent of the `editor.soft-wrap.max-wrap` option for diagnostics. | `20` | +| `max-diagnostics` | Maximum number of diagnostics to render inline for a given line | `10` | + +The (first) diagnostic with the highest severity that is not shown inline is rendered at the end of the line (as long as its severity is higher than the `end-of-line-diagnostics` config option): + +``` +fn main() { + let baz = 1; + let foo = bar; a local variable with a similar name exists: baz + └─ no such value in this scope +} +``` + + +The new diagnostic rendering is not yet enabled by default. As soon as end of line or inline diagnostics are enabled the old diagnostics rendering is automatically disabled. The recommended default setting are: + +``` +end-of-line-diagnostics = "hint" +[editor.inline-diagnostics] +cursor-line = "warning" # show warnings and errors on the cursorline inline +``` diff --git a/helix-core/src/diagnostic.rs b/helix-core/src/diagnostic.rs index d41119d38..4e89361d2 100644 --- a/helix-core/src/diagnostic.rs +++ b/helix-core/src/diagnostic.rs @@ -4,7 +4,8 @@ use std::fmt; 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, @@ -25,6 +26,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), @@ -71,3 +78,10 @@ impl fmt::Display for LanguageServerId { write!(f, "{:?}", self.0) } } + +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 a02d6e4dd..91f11e620 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 681d3456d..9165560d0 100644 --- a/helix-core/src/lib.rs +++ b/helix-core/src/lib.rs @@ -53,8 +53,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 0860da12f..3719abb0b 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 097c3493c..d205f234c 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1711,6 +1711,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 2da4d4b31..79145ba04 100644 --- a/helix-term/src/ui/document.rs +++ b/helix-term/src/ui/document.rs @@ -114,7 +114,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, @@ -348,6 +348,44 @@ 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; + // TODO is it correct to apply the whitspace style to all unicode white spaces? + if grapheme.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 49386b835..e934659cd 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -22,6 +22,7 @@ use helix_core::{ visual_offset_from_block, Change, Position, Range, Selection, Transaction, }; use helix_view::{ + annotations::diagnostics::DiagnosticFilter, document::{Mode, SavePoint, SCRATCH_BUFFER_NAME}, editor::{CompleteAction, CursorShapeConfig}, graphics::{Color, CursorKind, Modifier, Rect, Style}, @@ -185,6 +186,12 @@ impl EditorView { primary_cursor, }); } + decorations.add_decoration(InlineDiagnostics::new( + doc, + theme, + primary_cursor, + config.lsp.inline_diagnostics.clone(), + )); render_document( surface, inner, @@ -210,7 +217,11 @@ impl EditorView { } } - Self::render_diagnostics(doc, view, inner, surface, theme); + if config.inline_diagnostics.disabled() + && config.end_of_line_diagnostics == DiagnosticFilter::Disable + { + 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 000000000..2d9e83700 --- /dev/null +++ b/helix-term/src/ui/text_decorations/diagnostics.rs @@ -0,0 +1,305 @@ +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::{ + DiagnosticFilter, 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>, + eol_diagnostics: DiagnosticFilter, + styles: Styles, +} + +impl<'a> InlineDiagnostics<'a> { + pub fn new( + doc: &'a Document, + theme: &Theme, + cursor: usize, + config: InlineDiagnosticsConfig, + eol_diagnostics: DiagnosticFilter, + ) -> Self { + InlineDiagnostics { + state: InlineDiagnosticAccumulator::new(cursor, doc, config), + styles: Styles::new(theme), + eol_diagnostics, + } + } +} + +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 self.eol_diagnostics { + DiagnosticFilter::Enable(eol_filter) => { + let eol_diganogistcs = self + .state + .stack + .iter() + .filter(|(diag, _)| eol_filter <= diag.severity()); + match filter { + DiagnosticFilter::Enable(filter) => eol_diganogistcs + .filter(|(diag, _)| filter > diag.severity()) + .max_by_key(|(diagnostic, _)| diagnostic.severity), + DiagnosticFilter::Disable => { + eol_diganogistcs.max_by_key(|(diagnostic, _)| diagnostic.severity) + } + } + } + DiagnosticFilter::Disable => None, + }; + 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 000000000..4c630487f --- /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 000000000..afe0685a5 --- /dev/null +++ b/helix-view/src/annotations/diagnostics.rs @@ -0,0 +1,309 @@ +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; + +/// Describes the severity level of a [`Diagnostic`]. +#[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord)] +pub enum DiagnosticFilter { + Disable, + Enable(Severity), +} + +impl<'de> Deserialize<'de> for DiagnosticFilter { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + match &*String::deserialize(deserializer)? { + "disable" => Ok(DiagnosticFilter::Disable), + "hint" => Ok(DiagnosticFilter::Enable(Severity::Hint)), + "info" => Ok(DiagnosticFilter::Enable(Severity::Info)), + "warning" => Ok(DiagnosticFilter::Enable(Severity::Warning)), + "error" => Ok(DiagnosticFilter::Enable(Severity::Error)), + variant => Err(serde::de::Error::unknown_variant( + variant, + &["disable", "hint", "info", "warning", "error"], + )), + } + } +} + +impl Serialize for DiagnosticFilter { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + let filter = match self { + DiagnosticFilter::Disable => "disable", + DiagnosticFilter::Enable(Severity::Hint) => "hint", + DiagnosticFilter::Enable(Severity::Info) => "info", + DiagnosticFilter::Enable(Severity::Warning) => "warning", + DiagnosticFilter::Enable(Severity::Error) => "error", + }; + filter.serialize(serializer) + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(default, rename_all = "kebab-case", deny_unknown_fields)] +pub struct InlineDiagnosticsConfig { + pub cursor_line: DiagnosticFilter, + pub other_lines: DiagnosticFilter, + 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: DiagnosticFilter::Disable, + other_lines: DiagnosticFilter::Disable, + .. + } + ); + !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: DiagnosticFilter::Disable, + other_lines: DiagnosticFilter::Disable, + min_diagnostic_width: 40, + prefix_len: 1, + max_wrap: 20, + max_diagnostics: 10, + } + } +} + +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 centralize 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) -> DiagnosticFilter { + 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 DiagnosticFilter::Enable(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 fb8438be0..cead30d7c 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1,5 +1,6 @@ use crate::{ align_view, + annotations::diagnostics::{DiagnosticFilter, InlineDiagnosticsConfig}, document::{ DocumentOpenError, DocumentSavedEventFuture, DocumentSavedEventResult, Mode, SavePoint, }, @@ -343,6 +344,9 @@ pub struct Config { deserialize_with = "deserialize_alphabet" )] pub jump_label_alphabet: Vec, + /// Display diagnostic below the line they occur. + pub inline_diagnostics: InlineDiagnosticsConfig, + pub end_of_line_diagnostics: DiagnosticFilter, } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)] @@ -975,6 +979,8 @@ impl Default for Config { popup_border: PopupBorderConfig::None, indent_heuristic: IndentationHeuristic::default(), jump_label_alphabet: ('a'..='z').collect(), + inline_diagnostics: InlineDiagnosticsConfig::default(), + end_of_line_diagnostics: DiagnosticFilter::Disable, } } } diff --git a/helix-view/src/lib.rs b/helix-view/src/lib.rs index 14b6e1ce8..5628c830c 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 4c0674949..4aea98a70 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, @@ -438,37 +439,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 7e133167ca18e126d52939a717f6f34511068b3c Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:08 +0100 Subject: [PATCH 10/15] 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 4aea98a70..a5654759e 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -393,11 +393,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 3abc07a79e6ddd6a146977648031a9322492f445 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:08 +0100 Subject: [PATCH 11/15] 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 372e3e5ef..14397bb5c 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -448,14 +448,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 = doc!(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 a17b008b42698ecf2b9f64fb5676768b1a7f8652 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:09 +0100 Subject: [PATCH 12/15] 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 785e38fd7..ff28a8dd2 100644 --- a/helix-core/src/text_annotations.rs +++ b/helix-core/src/text_annotations.rs @@ -334,7 +334,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 } @@ -349,7 +351,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 d8a115641d7e6738473aba158be6efc2ad3493a7 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Tue, 2 Apr 2024 14:48:14 +0200 Subject: [PATCH 13/15] 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 54eb02fd0..f5c2b2ed5 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 3719abb0b..1b3789110 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"); From 7283ef881f2855ef94b67eca88799fa07157df9c Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Fri, 5 Apr 2024 03:17:06 +0200 Subject: [PATCH 14/15] only show inline diagnostics after a delay --- helix-term/src/application.rs | 7 ++ helix-term/src/commands.rs | 8 ++ helix-term/src/commands/lsp.rs | 5 +- helix-term/src/events.rs | 3 +- helix-term/src/handlers.rs | 2 + helix-term/src/handlers/diagnostics.rs | 24 ++++ helix-term/src/ui/editor.rs | 9 +- helix-view/src/annotations/diagnostics.rs | 24 ++-- helix-view/src/events.rs | 3 +- helix-view/src/handlers.rs | 1 + helix-view/src/handlers/diagnostics.rs | 131 ++++++++++++++++++++++ helix-view/src/view.rs | 19 +++- 12 files changed, 219 insertions(+), 17 deletions(-) create mode 100644 helix-term/src/handlers/diagnostics.rs create mode 100644 helix-view/src/handlers/diagnostics.rs diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 4c305f6ac..b7123e972 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -11,6 +11,7 @@ use helix_view::{ align_view, document::{DocumentOpenError, DocumentSavedEventResult}, editor::{ConfigEvent, EditorEvent}, + events::DiagnosticsDidChange, graphics::Rect, theme, tree::Layout, @@ -834,6 +835,12 @@ impl Application { &unchanged_diag_sources, Some(server_id), ); + + let doc = doc.id(); + helix_event::dispatch(DiagnosticsDidChange { + editor: &mut self.editor, + doc, + }); } } Notification::ShowMessage(params) => { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index d205f234c..234902886 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3550,6 +3550,8 @@ fn goto_first_diag(cx: &mut Context) { None => return, }; doc.set_selection(view.id, selection); + view.diagnostics_handler + .immediately_show_diagnostic(doc, view.id); } fn goto_last_diag(cx: &mut Context) { @@ -3559,6 +3561,8 @@ fn goto_last_diag(cx: &mut Context) { None => return, }; doc.set_selection(view.id, selection); + view.diagnostics_handler + .immediately_show_diagnostic(doc, view.id); } fn goto_next_diag(cx: &mut Context) { @@ -3581,6 +3585,8 @@ fn goto_next_diag(cx: &mut Context) { None => return, }; doc.set_selection(view.id, selection); + view.diagnostics_handler + .immediately_show_diagnostic(doc, view.id); }; cx.editor.apply_motion(motion); @@ -3609,6 +3615,8 @@ fn goto_prev_diag(cx: &mut Context) { None => return, }; doc.set_selection(view.id, selection); + view.diagnostics_handler + .immediately_show_diagnostic(doc, view.id); }; cx.editor.apply_motion(motion) } diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 103d1df24..3b9efb431 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -271,7 +271,10 @@ fn diag_picker( let Some(path) = uri.as_path() else { return; }; - jump_to_position(cx.editor, path, diag.range, *offset_encoding, action) + jump_to_position(cx.editor, path, diag.range, *offset_encoding, action); + let (view, doc) = current!(cx.editor); + view.diagnostics_handler + .immediately_show_diagnostic(doc, view.id); }, ) .with_preview(move |_editor, PickerDiagnostic { uri, diag, .. }| { diff --git a/helix-term/src/events.rs b/helix-term/src/events.rs index 49b44f775..415213c22 100644 --- a/helix-term/src/events.rs +++ b/helix-term/src/events.rs @@ -1,6 +1,6 @@ use helix_event::{events, register_event}; use helix_view::document::Mode; -use helix_view::events::{DocumentDidChange, SelectionDidChange}; +use helix_view::events::{DiagnosticsDidChange, DocumentDidChange, SelectionDidChange}; use crate::commands; use crate::keymap::MappableCommand; @@ -17,4 +17,5 @@ pub fn register() { register_event::(); register_event::(); register_event::(); + register_event::(); } diff --git a/helix-term/src/handlers.rs b/helix-term/src/handlers.rs index fc9273137..b27e34e29 100644 --- a/helix-term/src/handlers.rs +++ b/helix-term/src/handlers.rs @@ -14,6 +14,7 @@ pub use helix_view::handlers::Handlers; mod auto_save; pub mod completion; +mod diagnostics; mod signature_help; pub fn setup(config: Arc>) -> Handlers { @@ -32,5 +33,6 @@ pub fn setup(config: Arc>) -> Handlers { completion::register_hooks(&handlers); signature_help::register_hooks(&handlers); auto_save::register_hooks(&handlers); + diagnostics::register_hooks(&handlers); handlers } diff --git a/helix-term/src/handlers/diagnostics.rs b/helix-term/src/handlers/diagnostics.rs new file mode 100644 index 000000000..3e44d416d --- /dev/null +++ b/helix-term/src/handlers/diagnostics.rs @@ -0,0 +1,24 @@ +use helix_event::{register_hook, send_blocking}; +use helix_view::document::Mode; +use helix_view::events::DiagnosticsDidChange; +use helix_view::handlers::diagnostics::DiagnosticEvent; +use helix_view::handlers::Handlers; + +use crate::events::OnModeSwitch; + +pub(super) fn register_hooks(_handlers: &Handlers) { + register_hook!(move |event: &mut DiagnosticsDidChange<'_>| { + if event.editor.mode != Mode::Insert { + for (view, _) in event.editor.tree.views_mut() { + send_blocking(&view.diagnostics_handler.events, DiagnosticEvent::Refresh) + } + } + Ok(()) + }); + register_hook!(move |event: &mut OnModeSwitch<'_, '_>| { + for (view, _) in event.cx.editor.tree.views_mut() { + view.diagnostics_handler.active = event.new_mode != Mode::Insert; + } + Ok(()) + }); +} diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index e934659cd..c151a7dd5 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -186,11 +186,18 @@ impl EditorView { primary_cursor, }); } + let width = view.inner_width(doc); + let config = doc.config.load(); + let enable_cursor_line = view + .diagnostics_handler + .show_cursorline_diagnostics(doc, view.id); + let inline_diagnostic_config = config.inline_diagnostics.prepare(width, enable_cursor_line); decorations.add_decoration(InlineDiagnostics::new( doc, theme, primary_cursor, - config.lsp.inline_diagnostics.clone(), + inline_diagnostic_config, + config.end_of_line_diagnostics, )); render_document( surface, diff --git a/helix-view/src/annotations/diagnostics.rs b/helix-view/src/annotations/diagnostics.rs index afe0685a5..09085d3fe 100644 --- a/helix-view/src/annotations/diagnostics.rs +++ b/helix-view/src/annotations/diagnostics.rs @@ -60,22 +60,26 @@ pub struct InlineDiagnosticsConfig { } 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!( + pub fn disabled(&self) -> bool { + matches!( self, Self { cursor_line: DiagnosticFilter::Disable, other_lines: DiagnosticFilter::Disable, .. } - ); - !disabled && width >= self.min_diagnostic_width + self.prefix_len + ) + } + + pub fn prepare(&self, width: u16, enable_cursor_line: bool) -> Self { + let mut config = self.clone(); + if width < self.min_diagnostic_width + self.prefix_len { + config.cursor_line = DiagnosticFilter::Disable; + config.other_lines = DiagnosticFilter::Disable; + } else if !enable_cursor_line { + config.cursor_line = self.cursor_line.min(self.other_lines); + } + config } pub fn max_diagnostic_start(&self, width: u16) -> u16 { diff --git a/helix-view/src/events.rs b/helix-view/src/events.rs index 8b789cc0d..881412495 100644 --- a/helix-view/src/events.rs +++ b/helix-view/src/events.rs @@ -1,9 +1,10 @@ use helix_core::Rope; use helix_event::events; -use crate::{Document, ViewId}; +use crate::{Document, DocumentId, Editor, ViewId}; events! { DocumentDidChange<'a> { doc: &'a mut Document, view: ViewId, old_text: &'a Rope } SelectionDidChange<'a> { doc: &'a mut Document, view: ViewId } + DiagnosticsDidChange<'a> { editor: &'a mut Editor, doc: DocumentId } } diff --git a/helix-view/src/handlers.rs b/helix-view/src/handlers.rs index e2848f264..93336beb5 100644 --- a/helix-view/src/handlers.rs +++ b/helix-view/src/handlers.rs @@ -5,6 +5,7 @@ use crate::handlers::lsp::SignatureHelpInvoked; use crate::{DocumentId, Editor, ViewId}; pub mod dap; +pub mod diagnostics; pub mod lsp; #[derive(Debug)] diff --git a/helix-view/src/handlers/diagnostics.rs b/helix-view/src/handlers/diagnostics.rs new file mode 100644 index 000000000..2b8ff6325 --- /dev/null +++ b/helix-view/src/handlers/diagnostics.rs @@ -0,0 +1,131 @@ +use std::cell::Cell; +use std::num::NonZeroUsize; +use std::sync::atomic::{self, AtomicUsize}; +use std::sync::Arc; +use std::time::Duration; + +use helix_event::{request_redraw, send_blocking, AsyncHook}; +use tokio::sync::mpsc::Sender; +use tokio::time::Instant; + +use crate::{Document, DocumentId, ViewId}; + +#[derive(Debug)] +pub enum DiagnosticEvent { + CursorLineChanged { generation: usize }, + Refresh, +} + +struct DiagnosticTimeout { + active_generation: Arc, + generation: usize, +} + +const TIMEOUT: Duration = Duration::from_millis(350); + +impl AsyncHook for DiagnosticTimeout { + type Event = DiagnosticEvent; + + fn handle_event( + &mut self, + event: DiagnosticEvent, + timeout: Option, + ) -> Option { + match event { + DiagnosticEvent::CursorLineChanged { generation } => { + if generation > self.generation { + self.generation = generation; + Some(Instant::now() + TIMEOUT) + } else { + timeout + } + } + DiagnosticEvent::Refresh if timeout.is_some() => Some(Instant::now() + TIMEOUT), + DiagnosticEvent::Refresh => None, + } + } + + fn finish_debounce(&mut self) { + if self.active_generation.load(atomic::Ordering::Relaxed) < self.generation { + self.active_generation + .store(self.generation, atomic::Ordering::Relaxed); + request_redraw(); + } + } +} + +pub struct DiagnosticsHandler { + active_generation: Arc, + generation: Cell, + last_doc: Cell, + last_cursor_line: Cell, + pub active: bool, + pub events: Sender, +} + +// make sure we never share handlers across multiple views this is a stop +// gap solution. We just shouldn't be cloneing a view to begin with (we do +// for :hsplit/vsplit) and really this should not be view specific to begin with +// but to fix that larger architecutre changes are needed +impl Clone for DiagnosticsHandler { + fn clone(&self) -> Self { + Self::new() + } +} + +impl DiagnosticsHandler { + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + let active_generation = Arc::new(AtomicUsize::new(0)); + let events = DiagnosticTimeout { + active_generation: active_generation.clone(), + generation: 0, + } + .spawn(); + Self { + active_generation, + generation: Cell::new(0), + events, + last_doc: Cell::new(DocumentId(NonZeroUsize::new(usize::MAX).unwrap())), + last_cursor_line: Cell::new(usize::MAX), + active: true, + } + } +} + +impl DiagnosticsHandler { + pub fn immediately_show_diagnostic(&self, doc: &Document, view: ViewId) { + self.last_doc.set(doc.id()); + let cursor_line = doc + .selection(view) + .primary() + .cursor_line(doc.text().slice(..)); + self.last_cursor_line.set(cursor_line); + self.active_generation + .store(self.generation.get(), atomic::Ordering::Relaxed); + } + pub fn show_cursorline_diagnostics(&self, doc: &Document, view: ViewId) -> bool { + if !self.active { + return false; + } + let cursor_line = doc + .selection(view) + .primary() + .cursor_line(doc.text().slice(..)); + if self.last_cursor_line.get() == cursor_line && self.last_doc.get() == doc.id() { + let active_generation = self.active_generation.load(atomic::Ordering::Relaxed); + self.generation.get() == active_generation + } else { + self.last_doc.set(doc.id()); + self.last_cursor_line.set(cursor_line); + self.generation.set(self.generation.get() + 1); + send_blocking( + &self.events, + DiagnosticEvent::CursorLineChanged { + generation: self.generation.get(), + }, + ); + false + } + } +} diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index a5654759e..af4fdfe4e 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -4,6 +4,7 @@ use crate::{ document::DocumentInlayHints, editor::{GutterConfig, GutterType}, graphics::Rect, + handlers::diagnostics::DiagnosticsHandler, Align, Document, DocumentId, Theme, ViewId, }; @@ -147,6 +148,14 @@ pub struct View { /// mapping keeps track of the last applied history revision so that only new changes /// are applied. doc_revisions: HashMap, + // HACKS: there should really only be a global diagnostics handler (the + // non-focused views should just not have different handling for the cursor + // line). For that we would need accces to editor everywhere (we want to use + // the positioning code) so this can only happen by refactoring View and + // Document into entity component like structure. That is a huge refactor + // left to future work. For now we treat all views as focused and give them + // each their own handler. + pub diagnostics_handler: DiagnosticsHandler, } impl fmt::Debug for View { @@ -176,6 +185,7 @@ impl View { object_selections: Vec::new(), gutters, doc_revisions: HashMap::new(), + diagnostics_handler: DiagnosticsHandler::new(), } } @@ -463,10 +473,13 @@ impl View { .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 width = self.inner_width(doc); + let enable_cursor_line = self + .diagnostics_handler + .show_cursorline_diagnostics(doc, self.id); + let config = config.inline_diagnostics.prepare(width, enable_cursor_line); + if !config.disabled() { let cursor = doc .selection(self.id) .primary() From 386fa371d708f8e91a83762dfc7a58971681e091 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Fri, 5 Apr 2024 03:25:41 +0200 Subject: [PATCH 15/15] gracefully handle lack of tokio runtime --- helix-event/src/debounce.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/helix-event/src/debounce.rs b/helix-event/src/debounce.rs index 30b6f671b..5971f72b3 100644 --- a/helix-event/src/debounce.rs +++ b/helix-event/src/debounce.rs @@ -28,7 +28,10 @@ pub trait AsyncHook: Sync + Send + 'static + Sized { // so it should only be reached in case of total CPU overload. // However, a bounded channel is much more efficient so it's nice to use here let (tx, rx) = mpsc::channel(128); - tokio::spawn(run(self, rx)); + // only spawn worker if we are inside runtime to avoid having to spawn a runtime for unrelated unit tests + if tokio::runtime::Handle::try_current().is_ok() { + tokio::spawn(run(self, rx)); + } tx } }