-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add virtual text support #417
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,10 +12,11 @@ use helix_core::{ | |
}, | ||
movement::Direction, | ||
syntax::{self, HighlightEvent}, | ||
unicode::width::UnicodeWidthStr, | ||
unicode::{segmentation::UnicodeSegmentation, width::UnicodeWidthStr}, | ||
LineEnding, Position, Range, Selection, Transaction, | ||
}; | ||
use helix_view::{ | ||
decorations::{TextAnnotation, TextAnnotationKind}, | ||
document::{Mode, SCRATCH_BUFFER_NAME}, | ||
editor::{CompleteAction, CursorShapeConfig}, | ||
graphics::{Color, CursorKind, Modifier, Rect, Style}, | ||
|
@@ -395,6 +396,11 @@ impl EditorView { | |
// It's slightly more efficient to produce a full RopeSlice from the Rope, then slice that a bunch | ||
// of times than it is to always call Rope::slice/get_slice (it will internally always hit RSEnum::Light). | ||
let text = doc.text().slice(..); | ||
let last_line = std::cmp::min( | ||
// Saturating subs to make it inclusive zero indexing. | ||
(offset.row + viewport.height as usize).saturating_sub(1), | ||
doc.text().len_lines().saturating_sub(1), | ||
); | ||
|
||
let characters = &whitespace.characters; | ||
|
||
|
@@ -450,6 +456,37 @@ impl EditorView { | |
} | ||
}; | ||
|
||
// It's slightly more efficient to produce a full RopeSlice from the Rope, then slice that a bunch | ||
// of times than it is to always call Rope::slice/get_slice (it will internally always hit RSEnum::Light). | ||
let text = text.slice(..); | ||
let out_of_bounds = |visual_x: u16| { | ||
visual_x < offset.col as u16 || visual_x >= viewport.width + offset.col as u16 | ||
}; | ||
|
||
let render_annotation = | ||
|annot: &TextAnnotation, line: u16, pos: u16, surface: &mut Surface| { | ||
let mut visual_x = pos; | ||
for grapheme in annot.text.graphemes(true) { | ||
if out_of_bounds(visual_x) { | ||
break; | ||
} | ||
surface.set_string( | ||
viewport.x + visual_x - offset.col as u16, | ||
viewport.y + line, | ||
grapheme, | ||
annot.style, | ||
); | ||
visual_x = visual_x.saturating_add(grapheme.width() as u16); | ||
} | ||
}; | ||
|
||
let text_annotations = doc | ||
.text_annotations() | ||
.iter() | ||
.flat_map(|(_, annots)| annots) | ||
.filter(|annot| (offset.row..last_line).contains(&annot.line)) | ||
.collect::<Vec<_>>(); | ||
|
||
'outer: for event in highlights { | ||
match event { | ||
HighlightEvent::HighlightStart(span) => { | ||
|
@@ -488,22 +525,27 @@ impl EditorView { | |
use helix_core::graphemes::{grapheme_width, RopeGraphemes}; | ||
|
||
for grapheme in RopeGraphemes::new(text) { | ||
let out_of_bounds = visual_x < offset.col as u16 | ||
|| visual_x >= viewport.width + offset.col as u16; | ||
|
||
if LineEnding::from_rope_slice(&grapheme).is_some() { | ||
if !out_of_bounds { | ||
if !out_of_bounds(visual_x) { | ||
// we still want to render an empty cell with the style | ||
surface.set_string( | ||
viewport.x + visual_x - offset.col as u16, | ||
viewport.y + line, | ||
&newline, | ||
style.patch(whitespace_style), | ||
); | ||
visual_x += 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this not necessary before, but is now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe because it is due to the addition of virtual text. But this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this works as intended in all cases.
This was not required before this PR because |
||
} | ||
|
||
draw_indent_guides(last_line_indent_level, line, surface); | ||
|
||
if let Some(annot) = text_annotations | ||
.iter() | ||
.find(|t| t.kind.is_eol() && t.line == offset.row + line as usize) | ||
{ | ||
render_annotation(annot, line, visual_x, surface); | ||
} | ||
|
||
visual_x = 0; | ||
line += 1; | ||
is_in_indent_area = true; | ||
|
@@ -540,7 +582,7 @@ impl EditorView { | |
|
||
let cut_off_start = offset.col.saturating_sub(visual_x as usize); | ||
|
||
if !out_of_bounds { | ||
if !out_of_bounds(visual_x) { | ||
// if we're offscreen just keep going until we hit a new line | ||
surface.set_string( | ||
viewport.x + visual_x - offset.col as u16, | ||
|
@@ -582,6 +624,13 @@ impl EditorView { | |
} | ||
} | ||
} | ||
|
||
for annot in &text_annotations { | ||
if let TextAnnotationKind::Overlay(visual_x) = annot.kind { | ||
let line = (annot.line - offset.row) as u16; | ||
render_annotation(annot, line, visual_x as u16, surface); | ||
} | ||
} | ||
} | ||
|
||
/// Render brace match, etc (meant for the focused view only) | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,32 @@ | ||||||
use std::borrow::Cow; | ||||||
|
||||||
use crate::graphics::Style; | ||||||
|
||||||
#[derive(Clone, Copy, PartialEq)] | ||||||
pub enum TextAnnotationKind { | ||||||
/// Add to end of line | ||||||
Eol, | ||||||
/// Replace actual text or arbitary cells with annotations. | ||||||
/// Specifies an offset from the 0th column. | ||||||
Overlay(usize), | ||||||
sudormrfbin marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
|
||||||
impl TextAnnotationKind { | ||||||
pub fn is_eol(&self) -> bool { | ||||||
*self == Self::Eol | ||||||
} | ||||||
|
||||||
pub fn is_overlay(&self) -> bool { | ||||||
matches!(*self, Self::Overlay(_)) | ||||||
} | ||||||
} | ||||||
|
||||||
/// Namespaces and identifes similar annotations | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
pub type TextAnnotationGroup = &'static str; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this could be an enum instead of a string, since it will be known at compile time anyway. pub enum TextAnnotationGroup {
Diagnostic,
JumpMode,
} Then hashing or b-tree look up & filtering will be fast, or possibly even faster would be to store annotations directly into a named field: struct DocumentAnnotations {
diagnostics: Vec<TextAnnotation>,
jump_mode: Vec<TextAnnotation>,
}
pub struct Document {
// -----8<-----
text_annotations: DocumentAnnotations,
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, another review note: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think making a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's a fair point! it's certainly not a blocking issue. to reap benefits (which may be slight?) of the enum approach and allow plugins to add annotations, something like this would work: /// first-party annotation groups (only used to differentiate access to the Vecs below)
pub enum TextAnnotationGroup {
Diagnostic,
JumpMode,
}
/// plugin annotation groups
/// (might have to reserve "diagnostic" and "jump_mode" strs?
pub type TextAnnotationPluginGroup = &'static str;
struct DocumentAnnotations {
diagnostics: Vec<TextAnnotation>,
jump_mode: Vec<TextAnnotation>,
from_plugins: BTreeMap<&'static str, TextAnnotation>,
}
pub struct Document {
// -----8<-----
text_annotations: DocumentAnnotations,
}
This comment was marked as off-topic.
Sorry, something went wrong. |
||||||
|
||||||
pub struct TextAnnotation { | ||||||
pub text: Cow<'static, str>, | ||||||
pub style: Style, | ||||||
pub line: usize, | ||||||
pub kind: TextAnnotationKind, | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,9 @@ use helix_core::{ | |
DEFAULT_LINE_ENDING, | ||
}; | ||
|
||
use crate::{DocumentId, Editor, ViewId}; | ||
use crate::decorations::TextAnnotationGroup; | ||
use crate::Editor; | ||
use crate::{decorations::TextAnnotation, DocumentId, ViewId}; | ||
|
||
/// 8kB of buffer space for encoding and decoding `Rope`s. | ||
const BUF_SIZE: usize = 8192; | ||
|
@@ -119,6 +121,7 @@ pub struct Document { | |
pub(crate) modified_since_accessed: bool, | ||
|
||
diagnostics: Vec<Diagnostic>, | ||
text_annotations: HashMap<TextAnnotationGroup, Vec<TextAnnotation>>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be more performant to implement this as a |
||
language_server: Option<Arc<helix_lsp::Client>>, | ||
} | ||
|
||
|
@@ -351,6 +354,7 @@ impl Document { | |
language: None, | ||
changes, | ||
old_state, | ||
text_annotations: HashMap::new(), | ||
diagnostics: Vec::new(), | ||
version: 0, | ||
history: Cell::new(History::default()), | ||
|
@@ -1038,6 +1042,31 @@ impl Document { | |
.map(helix_core::path::get_relative_path) | ||
} | ||
|
||
pub fn text_annotations(&self) -> &HashMap<TextAnnotationGroup, Vec<TextAnnotation>> { | ||
&self.text_annotations | ||
} | ||
|
||
pub fn push_text_annotations<I: Iterator<Item = TextAnnotation>>( | ||
&mut self, | ||
group: TextAnnotationGroup, | ||
annots: I, | ||
) { | ||
self.text_annotations | ||
.entry(group) | ||
.or_default() | ||
.extend(annots); | ||
} | ||
|
||
pub fn clear_text_annotations(&mut self, group: TextAnnotationGroup) { | ||
if let Some(annots) = self.text_annotations.get_mut(group) { | ||
annots.clear() | ||
} | ||
} | ||
|
||
// pub fn slice<R>(&self, range: R) -> RopeSlice where R: RangeBounds { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still needed? |
||
// self.state.doc.slice | ||
// } | ||
|
||
// transact(Fn) ? | ||
|
||
// -- LSP methods | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a noop (text is already a
RopeSlice