Skip to content
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

Fix precedence of ui.virtual.whitespace #8750

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 60 additions & 24 deletions helix-term/src/ui/document.rs
Original file line number Diff line number Diff line change
@@ -97,7 +97,8 @@ pub fn render_document(
doc: &Document,
offset: ViewPosition,
doc_annotations: &TextAnnotations,
highlight_iter: impl Iterator<Item = HighlightEvent>,
syntax_highlight_iter: impl Iterator<Item = HighlightEvent>,
overlay_highlight_iter: impl Iterator<Item = HighlightEvent>,
theme: &Theme,
line_decoration: &mut [Box<dyn LineDecoration + '_>],
translated_positions: &mut [TranslatedPosition],
@@ -109,7 +110,8 @@ pub fn render_document(
offset,
&doc.text_format(viewport.width, Some(theme)),
doc_annotations,
highlight_iter,
syntax_highlight_iter,
overlay_highlight_iter,
theme,
line_decoration,
translated_positions,
@@ -157,7 +159,8 @@ pub fn render_text<'t>(
offset: ViewPosition,
text_fmt: &TextFormat,
text_annotations: &TextAnnotations,
highlight_iter: impl Iterator<Item = HighlightEvent>,
syntax_highlight_iter: impl Iterator<Item = HighlightEvent>,
overlay_highlight_iter: impl Iterator<Item = HighlightEvent>,
theme: &Theme,
line_decorations: &mut [Box<dyn LineDecoration + '_>],
translated_positions: &mut [TranslatedPosition],
@@ -178,10 +181,16 @@ pub fn render_text<'t>(

let (mut formatter, mut first_visible_char_idx) =
DocumentFormatter::new_at_prev_checkpoint(text, text_fmt, text_annotations, offset.anchor);
let mut styles = StyleIter {
let mut syntax_styles = StyleIter {
text_style: renderer.text_style,
active_highlights: Vec::with_capacity(64),
highlight_iter,
highlight_iter: syntax_highlight_iter,
theme,
};
let mut overlay_styles = StyleIter {
text_style: renderer.text_style,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using render.text_style is wrong here it will set the whole document to the text colour as the overlay is patched last, this should just be Style::default so if there is no style here we don't override the past text highlighting

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see StyleIter for why it will highlight all the text the default text color:

.fold(self.text_style, |acc, span| {
acc.patch(self.theme.highlight(span.0))
});
as overlay_highlight_iter covers the whole viewport

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking into this more deeply, this text_style is set based on the given ui.text scope. The documentation about this says "Command prompts, popup text, etc.", but as this is really the default text style that is used, we should probably change that description.

active_highlights: Vec::with_capacity(64),
highlight_iter: overlay_highlight_iter,
theme,
};

@@ -193,7 +202,10 @@ pub fn render_text<'t>(
};
let mut is_in_indent_area = true;
let mut last_line_indent_level = 0;
let mut style_span = styles
let mut syntax_style_span = syntax_styles
.next()
.unwrap_or_else(|| (Style::default(), usize::MAX));
let mut overlay_style_span = overlay_styles
.next()
.unwrap_or_else(|| (Style::default(), usize::MAX));

@@ -221,9 +233,16 @@ pub fn render_text<'t>(

// skip any graphemes on visual lines before the block start
if pos.row < row_off {
if char_pos >= style_span.1 {
style_span = if let Some(style_span) = styles.next() {
style_span
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
} else {
break;
}
@@ -260,8 +279,15 @@ pub fn render_text<'t>(
}

// acquire the correct grapheme style
if char_pos >= style_span.1 {
style_span = styles.next().unwrap_or((Style::default(), usize::MAX));
if 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 {
overlay_style_span = overlay_styles
.next()
.unwrap_or((Style::default(), usize::MAX));
}
char_pos += grapheme.doc_chars();

@@ -275,22 +301,25 @@ pub fn render_text<'t>(
pos,
);

let grapheme_style = if let GraphemeSource::VirtualText { highlight } = grapheme.source {
let style = renderer.text_style;
if let Some(highlight) = highlight {
style.patch(theme.highlight(highlight.0))
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 {
style
}
} else {
style_span.0
};
(syntax_style_span.0, overlay_style_span.0)
};

let virt = grapheme.is_virtual();
let is_virtual = grapheme.is_virtual();
renderer.draw_grapheme(
grapheme.grapheme,
grapheme_style,
virt,
GraphemeStyle {
syntax_style,
overlay_style,
},
is_virtual,
&mut last_line_indent_level,
&mut is_in_indent_area,
pos,
@@ -322,6 +351,11 @@ pub struct TextRenderer<'a> {
pub viewport: Rect,
}

pub struct GraphemeStyle {
syntax_style: Style,
overlay_style: Style,
}

impl<'a> TextRenderer<'a> {
pub fn new(
surface: &'a mut Surface,
@@ -395,7 +429,7 @@ impl<'a> TextRenderer<'a> {
pub fn draw_grapheme(
&mut self,
grapheme: Grapheme,
mut style: Style,
grapheme_style: GraphemeStyle,
is_virtual: bool,
last_indent_level: &mut usize,
is_in_indent_area: &mut bool,
@@ -405,9 +439,11 @@ impl<'a> TextRenderer<'a> {
let is_whitespace = grapheme.is_whitespace();

// TODO is it correct to apply the whitespace style to all unicode white spaces?
let mut style = grapheme_style.syntax_style;
if is_whitespace {
style = style.patch(self.whitespace_style);
}
style = style.patch(grapheme_style.overlay_style);

let width = grapheme.width();
let space = if is_virtual { " " } else { &self.space };
96 changes: 58 additions & 38 deletions helix-term/src/ui/editor.rs
Original file line number Diff line number Diff line change
@@ -124,16 +124,20 @@ impl EditorView {
line_decorations.push(Box::new(line_decoration));
}

let mut highlights =
let syntax_highlights =
Self::doc_syntax_highlights(doc, view.offset.anchor, inner.height, theme);
let overlay_highlights = Self::overlay_syntax_highlights(

let mut overlay_highlights =
Self::empty_highlight_iter(doc, view.offset.anchor, inner.height);
let overlay_syntax_highlights = Self::overlay_syntax_highlights(
doc,
view.offset.anchor,
inner.height,
&text_annotations,
);
if !overlay_highlights.is_empty() {
highlights = Box::new(syntax::merge(highlights, overlay_highlights));
if !overlay_syntax_highlights.is_empty() {
overlay_highlights =
Box::new(syntax::merge(overlay_highlights, overlay_syntax_highlights));
}

for diagnostic in Self::doc_diagnostics_highlights(doc, theme) {
@@ -142,12 +146,12 @@ impl EditorView {
if diagnostic.is_empty() {
continue;
}
highlights = Box::new(syntax::merge(highlights, diagnostic));
overlay_highlights = Box::new(syntax::merge(overlay_highlights, diagnostic));
}

let highlights: Box<dyn Iterator<Item = HighlightEvent>> = if is_focused {
if is_focused {
let highlights = syntax::merge(
highlights,
overlay_highlights,
Self::doc_selection_highlights(
editor.mode(),
doc,
@@ -158,13 +162,11 @@ impl EditorView {
);
let focused_view_elements = Self::highlight_focused_view_elements(view, doc, theme);
if focused_view_elements.is_empty() {
Box::new(highlights)
overlay_highlights = Box::new(highlights)
pascalkuthe marked this conversation as resolved.
Show resolved Hide resolved
} else {
Box::new(syntax::merge(highlights, focused_view_elements))
overlay_highlights = Box::new(syntax::merge(highlights, focused_view_elements))
}
} else {
Box::new(highlights)
};
}

let gutter_overflow = view.gutter_offset(doc) == 0;
if !gutter_overflow {
@@ -197,7 +199,8 @@ impl EditorView {
doc,
view.offset,
&text_annotations,
highlights,
syntax_highlights,
overlay_highlights,
theme,
&mut line_decorations,
&mut translated_positions,
@@ -257,27 +260,39 @@ impl EditorView {
.for_each(|area| surface.set_style(area, ruler_theme))
}

pub fn overlay_syntax_highlights(
fn viewport_byte_range(
text: helix_core::RopeSlice,
row: usize,
height: u16,
) -> std::ops::Range<usize> {
// Calculate viewport byte ranges:
// Saturating subs to make it inclusive zero indexing.
let last_line = text.len_lines().saturating_sub(1);
let last_visible_line = (row + height as usize).saturating_sub(1).min(last_line);
let start = text.line_to_byte(row.min(last_line));
let end = text.line_to_byte(last_visible_line + 1);

start..end
}

pub fn empty_highlight_iter(
doc: &Document,
anchor: usize,
height: u16,
text_annotations: &TextAnnotations,
) -> Vec<(usize, std::ops::Range<usize>)> {
) -> Box<dyn Iterator<Item = HighlightEvent>> {
let text = doc.text().slice(..);
let row = text.char_to_line(anchor.min(text.len_chars()));

let range = {
// Calculate viewport byte ranges:
// Saturating subs to make it inclusive zero indexing.
let last_line = text.len_lines().saturating_sub(1);
let last_visible_line = (row + height as usize).saturating_sub(1).min(last_line);
let start = text.line_to_byte(row.min(last_line));
let end = text.line_to_byte(last_visible_line + 1);

start..end
};

text_annotations.collect_overlay_highlights(range)
// Calculate viewport byte ranges:
// Saturating subs to make it inclusive zero indexing.
let range = Self::viewport_byte_range(text, row, height);
Box::new(
[HighlightEvent::Source {
start: text.byte_to_char(range.start),
end: text.byte_to_char(range.end),
}]
.into_iter(),
)
}

/// Get syntax highlights for a document in a view represented by the first line
@@ -292,16 +307,7 @@ impl EditorView {
let text = doc.text().slice(..);
let row = text.char_to_line(anchor.min(text.len_chars()));

let range = {
// Calculate viewport byte ranges:
// Saturating subs to make it inclusive zero indexing.
let last_line = text.len_lines().saturating_sub(1);
let last_visible_line = (row + height as usize).saturating_sub(1).min(last_line);
let start = text.line_to_byte(row.min(last_line));
let end = text.line_to_byte(last_visible_line + 1);

start..end
};
let range = Self::viewport_byte_range(text, row, height);

match doc.syntax() {
Some(syntax) => {
@@ -334,6 +340,20 @@ impl EditorView {
}
}

pub fn overlay_syntax_highlights(
doc: &Document,
anchor: usize,
height: u16,
text_annotations: &TextAnnotations,
) -> Vec<(usize, std::ops::Range<usize>)> {
let text = doc.text().slice(..);
let row = text.char_to_line(anchor.min(text.len_chars()));

let range = Self::viewport_byte_range(text, row, height);

text_annotations.collect_overlay_highlights(range)
}

/// Get highlight spans for document diagnostics
pub fn doc_diagnostics_highlights(
doc: &Document,
10 changes: 7 additions & 3 deletions helix-term/src/ui/picker.rs
Original file line number Diff line number Diff line change
@@ -736,17 +736,20 @@ impl<T: Item + 'static> Picker<T> {
}
}

let mut highlights = EditorView::doc_syntax_highlights(
let syntax_highlights = EditorView::doc_syntax_highlights(
doc,
offset.anchor,
area.height,
&cx.editor.theme,
);

let mut overlay_highlights =
EditorView::empty_highlight_iter(doc, offset.anchor, area.height);
for spans in EditorView::doc_diagnostics_highlights(doc, &cx.editor.theme) {
if spans.is_empty() {
continue;
}
highlights = Box::new(helix_core::syntax::merge(highlights, spans));
overlay_highlights = Box::new(helix_core::syntax::merge(overlay_highlights, spans));
}
let mut decorations: Vec<Box<dyn LineDecoration>> = Vec::new();

@@ -777,7 +780,8 @@ impl<T: Item + 'static> Picker<T> {
offset,
// TODO: compute text annotations asynchronously here (like inlay hints)
&TextAnnotations::default(),
highlights,
syntax_highlights,
overlay_highlights,
&cx.editor.theme,
&mut decorations,
&mut [],