From 69e07ab61e51598ba6e31bdd79608091fdbba254 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:03 +0100 Subject: [PATCH] use slices instead of Rc for virtual text --- helix-core/src/doc_formatter.rs | 2 +- helix-core/src/doc_formatter/test.rs | 19 +++++++++---- helix-core/src/text_annotations.rs | 31 ++++++++++---------- helix-term/src/commands.rs | 4 ++- helix-term/src/commands/lsp.rs | 10 +++---- helix-view/src/document.rs | 42 +++++++++++----------------- helix-view/src/view.rs | 28 ++++++++----------- 7 files changed, 66 insertions(+), 70 deletions(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index c7dc9081f..cbe2da3b6 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -116,7 +116,7 @@ impl Default for TextFormat { #[derive(Debug)] pub struct DocumentFormatter<'t> { text_fmt: &'t TextFormat, - annotations: &'t TextAnnotations, + annotations: &'t TextAnnotations<'t>, /// The visual position at the end of the last yielded word boundary visual_pos: Position, diff --git a/helix-core/src/doc_formatter/test.rs b/helix-core/src/doc_formatter/test.rs index ac8918bb7..d2b6ddc74 100644 --- a/helix-core/src/doc_formatter/test.rs +++ b/helix-core/src/doc_formatter/test.rs @@ -1,5 +1,3 @@ -use std::rc::Rc; - use crate::doc_formatter::{DocumentFormatter, TextFormat}; use crate::text_annotations::{InlineAnnotation, Overlay, TextAnnotations}; @@ -105,7 +103,7 @@ fn overlay_text(text: &str, char_pos: usize, softwrap: bool, overlays: &[Overlay DocumentFormatter::new_at_prev_checkpoint( text.into(), &TextFormat::new_test(softwrap), - TextAnnotations::default().add_overlay(overlays.into(), None), + TextAnnotations::default().add_overlay(overlays, None), char_pos, ) .0 @@ -142,7 +140,7 @@ fn annotate_text(text: &str, softwrap: bool, annotations: &[InlineAnnotation]) - DocumentFormatter::new_at_prev_checkpoint( text.into(), &TextFormat::new_test(softwrap), - TextAnnotations::default().add_inline_annotations(annotations.into(), None), + TextAnnotations::default().add_inline_annotations(annotations, None), 0, ) .0 @@ -164,15 +162,24 @@ fn annotation() { "foo foo foo foo \n.foo foo foo foo \n.foo foo foo " ); } + #[test] fn annotation_and_overlay() { + let annotations = [InlineAnnotation { + char_idx: 0, + text: "fooo".into(), + }]; + let overlay = [Overlay { + char_idx: 0, + grapheme: "\t".into(), + }]; assert_eq!( DocumentFormatter::new_at_prev_checkpoint( "bbar".into(), &TextFormat::new_test(false), TextAnnotations::default() - .add_inline_annotations(Rc::new([InlineAnnotation::new(0, "fooo")]), None) - .add_overlay(Rc::new([Overlay::new(0, "\t")]), None), + .add_inline_annotations(annotations.as_slice(), None) + .add_overlay(overlay.as_slice(), None), 0, ) .0 diff --git a/helix-core/src/text_annotations.rs b/helix-core/src/text_annotations.rs index 11d19d485..1576914e3 100644 --- a/helix-core/src/text_annotations.rs +++ b/helix-core/src/text_annotations.rs @@ -1,6 +1,5 @@ use std::cell::Cell; use std::ops::Range; -use std::rc::Rc; use crate::syntax::Highlight; use crate::Tendril; @@ -92,23 +91,23 @@ pub struct LineAnnotation { } #[derive(Debug)] -struct Layer { - annotations: Rc<[A]>, +struct Layer<'a, A, M> { + annotations: &'a [A], current_index: Cell, metadata: M, } -impl Clone for Layer { +impl Clone for Layer<'_, A, M> { fn clone(&self) -> Self { Layer { - annotations: self.annotations.clone(), + annotations: self.annotations, current_index: self.current_index.clone(), metadata: self.metadata.clone(), } } } -impl Layer { +impl Layer<'_, A, M> { pub fn reset_pos(&self, char_idx: usize, get_char_idx: impl Fn(&A) -> usize) { let new_index = self .annotations @@ -128,8 +127,8 @@ impl Layer { } } -impl From<(Rc<[A]>, M)> for Layer { - fn from((annotations, metadata): (Rc<[A]>, M)) -> Layer { +impl<'a, A, M> From<(&'a [A], M)> for Layer<'a, A, M> { + fn from((annotations, metadata): (&'a [A], M)) -> Layer { Layer { annotations, current_index: Cell::new(0), @@ -147,13 +146,13 @@ fn reset_pos(layers: &[Layer], pos: usize, get_pos: impl Fn(&A) -> u /// Annotations that change that is displayed when the document is render. /// Also commonly called virtual text. #[derive(Default, Debug, Clone)] -pub struct TextAnnotations { - inline_annotations: Vec>>, - overlays: Vec>>, - line_annotations: Vec>, +pub struct TextAnnotations<'a> { + inline_annotations: Vec>>, + overlays: Vec>>, + line_annotations: Vec>, } -impl TextAnnotations { +impl<'a> TextAnnotations<'a> { /// Prepare the TextAnnotations for iteration starting at char_idx pub fn reset_pos(&self, char_idx: usize) { reset_pos(&self.inline_annotations, char_idx, |annot| annot.char_idx); @@ -194,7 +193,7 @@ impl TextAnnotations { /// the annotations that belong to the layers added first will be shown first. pub fn add_inline_annotations( &mut self, - layer: Rc<[InlineAnnotation]>, + layer: &'a [InlineAnnotation], highlight: Option, ) -> &mut Self { self.inline_annotations.push((layer, highlight).into()); @@ -211,7 +210,7 @@ impl TextAnnotations { /// /// 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: Rc<[Overlay]>, highlight: Option) -> &mut Self { + pub fn add_overlay(&mut self, layer: &'a [Overlay], highlight: Option) -> &mut Self { self.overlays.push((layer, highlight).into()); self } @@ -220,7 +219,7 @@ impl TextAnnotations { /// /// 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: Rc<[LineAnnotation]>) -> &mut Self { + pub fn add_line_annotation(&mut self, layer: &'a [LineAnnotation]) -> &mut Self { self.line_annotations.push((layer, ()).into()); self } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index ff72cee1f..6de056518 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -617,6 +617,7 @@ fn move_impl(cx: &mut Context, move_fn: MoveFn, dir: Direction, behaviour: Movem &mut annotations, ) }); + drop(annotations); doc.set_selection(view.id, selection); } @@ -1637,7 +1638,7 @@ pub fn scroll(cx: &mut Context, offset: usize, direction: Direction, sync_cursor let doc_text = doc.text().slice(..); let viewport = view.inner_area(doc); let text_fmt = doc.text_format(viewport.width, None); - let mut annotations = view.text_annotations(doc, None); + let mut annotations = view.text_annotations(&*doc, None); (view.offset.anchor, view.offset.vertical_offset) = char_idx_at_visual_offset( doc_text, view.offset.anchor, @@ -1715,6 +1716,7 @@ pub fn scroll(cx: &mut Context, offset: usize, direction: Direction, sync_cursor let mut sel = doc.selection(view.id).clone(); let idx = sel.primary_index(); sel = sel.replace(idx, prim_sel); + drop(annotations); doc.set_selection(view.id, sel); } diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index a3168dc2d..63d1608f9 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -1315,11 +1315,11 @@ fn compute_inlay_hints_for_view( view_id, DocumentInlayHints { id: new_doc_inlay_hints_id, - type_inlay_hints: type_inlay_hints.into(), - parameter_inlay_hints: parameter_inlay_hints.into(), - other_inlay_hints: other_inlay_hints.into(), - padding_before_inlay_hints: padding_before_inlay_hints.into(), - padding_after_inlay_hints: padding_after_inlay_hints.into(), + type_inlay_hints, + parameter_inlay_hints, + other_inlay_hints, + padding_before_inlay_hints, + padding_after_inlay_hints, }, ); doc.inlay_hints_oudated = false; diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index f813c7424..090e4dd5c 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -8,7 +8,7 @@ use helix_core::chars::char_is_word; use helix_core::doc_formatter::TextFormat; use helix_core::encoding::Encoding; use helix_core::syntax::{Highlight, LanguageServerFeature}; -use helix_core::text_annotations::{InlineAnnotation, TextAnnotations}; +use helix_core::text_annotations::InlineAnnotation; use helix_lsp::util::lsp_pos_to_pos; use helix_vcs::{DiffHandle, DiffProviderRegistry}; @@ -21,7 +21,6 @@ use std::collections::HashMap; use std::fmt::Display; use std::future::Future; use std::path::{Path, PathBuf}; -use std::rc::Rc; use std::str::FromStr; use std::sync::{Arc, Weak}; use std::time::SystemTime; @@ -200,22 +199,22 @@ pub struct DocumentInlayHints { pub id: DocumentInlayHintsId, /// Inlay hints of `TYPE` kind, if any. - pub type_inlay_hints: Rc<[InlineAnnotation]>, + pub type_inlay_hints: Vec, /// Inlay hints of `PARAMETER` kind, if any. - pub parameter_inlay_hints: Rc<[InlineAnnotation]>, + pub parameter_inlay_hints: Vec, /// Inlay hints that are neither `TYPE` nor `PARAMETER`. /// /// LSPs are not required to associate a kind to their inlay hints, for example Rust-Analyzer /// currently never does (February 2023) and the LSP spec may add new kinds in the future that /// we want to display even if we don't have some special highlighting for them. - pub other_inlay_hints: Rc<[InlineAnnotation]>, + pub other_inlay_hints: Vec, /// Inlay hint padding. When creating the final `TextAnnotations`, the `before` padding must be /// added first, then the regular inlay hints, then the `after` padding. - pub padding_before_inlay_hints: Rc<[InlineAnnotation]>, - pub padding_after_inlay_hints: Rc<[InlineAnnotation]>, + pub padding_before_inlay_hints: Vec, + pub padding_after_inlay_hints: Vec, } impl DocumentInlayHints { @@ -223,11 +222,11 @@ impl DocumentInlayHints { pub fn empty_with_id(id: DocumentInlayHintsId) -> Self { Self { id, - type_inlay_hints: Rc::new([]), - parameter_inlay_hints: Rc::new([]), - other_inlay_hints: Rc::new([]), - padding_before_inlay_hints: Rc::new([]), - padding_after_inlay_hints: Rc::new([]), + type_inlay_hints: Vec::new(), + parameter_inlay_hints: Vec::new(), + other_inlay_hints: Vec::new(), + padding_before_inlay_hints: Vec::new(), + padding_after_inlay_hints: Vec::new(), } } } @@ -1266,13 +1265,12 @@ impl Document { }); // Update the inlay hint annotations' positions, helping ensure they are displayed in the proper place - let apply_inlay_hint_changes = |annotations: &mut Rc<[InlineAnnotation]>| { - if let Some(data) = Rc::get_mut(annotations) { - changes.update_positions( - data.iter_mut() - .map(|annotation| (&mut annotation.char_idx, Assoc::After)), - ); - } + let apply_inlay_hint_changes = |annotations: &mut Vec| { + changes.update_positions( + annotations + .iter_mut() + .map(|annotation| (&mut annotation.char_idx, Assoc::After)), + ); }; self.inlay_hints_oudated = true; @@ -1940,12 +1938,6 @@ impl Document { } } - /// Get the text annotations that apply to the whole document, those that do not apply to any - /// specific view. - pub fn text_annotations(&self, _theme: Option<&Theme>) -> TextAnnotations { - TextAnnotations::default() - } - /// Set the inlay hints for this document and `view_id`. pub fn set_inlay_hints(&mut self, view_id: ViewId, inlay_hints: DocumentInlayHints) { self.inlay_hints.insert(view_id, inlay_hints); diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index e5e2641a8..bbdc74a74 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -19,7 +19,6 @@ use helix_core::{ use std::{ collections::{HashMap, VecDeque}, fmt, - rc::Rc, }; const JUMP_LIST_CAPACITY: usize = 30; @@ -409,10 +408,12 @@ impl View { } /// Get the text annotations to display in the current view for the given document and theme. - pub fn text_annotations(&self, doc: &Document, theme: Option<&Theme>) -> TextAnnotations { - // TODO custom annotations for custom views like side by side diffs - - let mut text_annotations = doc.text_annotations(theme); + pub fn text_annotations<'a>( + &self, + doc: &'a Document, + theme: Option<&Theme>, + ) -> TextAnnotations<'a> { + let mut text_annotations = TextAnnotations::default(); let DocumentInlayHints { id: _, @@ -436,20 +437,15 @@ impl View { .and_then(|t| t.find_scope_index("ui.virtual.inlay-hint")) .map(Highlight); - let mut add_annotations = |annotations: &Rc<[_]>, style| { - if !annotations.is_empty() { - text_annotations.add_inline_annotations(Rc::clone(annotations), style); - } - }; - // 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. - add_annotations(padding_before_inlay_hints, None); - add_annotations(type_inlay_hints, type_style); - add_annotations(parameter_inlay_hints, parameter_style); - add_annotations(other_inlay_hints, other_style); - add_annotations(padding_after_inlay_hints, None); + 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); text_annotations }