From 5b098c3fd8212cee632b9e18a8667f8b59b78365 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Thu, 29 Sep 2022 17:05:10 +0200 Subject: [PATCH 1/9] implement consistent behaviour for highlight overlays --- helix-core/src/syntax.rs | 173 ++++-------------- helix-core/src/syntax/overlay.rs | 254 ++++++++++++++++++++++++++ helix-core/src/syntax/overlay/test.rs | 32 ++++ helix-term/src/ui/editor.rs | 218 +++++++++++++--------- helix-term/src/ui/lsp.rs | 17 +- helix-term/src/ui/markdown.rs | 14 +- helix-term/src/ui/picker.rs | 33 ++-- 7 files changed, 487 insertions(+), 254 deletions(-) create mode 100644 helix-core/src/syntax/overlay.rs create mode 100644 helix-core/src/syntax/overlay/test.rs diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index e0a984d20d29..d3f4e7a2d0fa 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -2,10 +2,12 @@ use crate::{ auto_pairs::AutoPairs, chars::char_is_line_ending, diagnostic::Severity, + graphemes::ensure_grapheme_boundary_next_byte, regex::Regex, transaction::{ChangeSet, Operation}, Rope, RopeSlice, Tendril, }; +pub use overlay::{monotonic_overlay, overlapping_overlay, Span}; use arc_swap::{ArcSwap, Guard}; use slotmap::{DefaultKey as LayerId, HopSlotMap}; @@ -25,6 +27,8 @@ use serde::{Deserialize, Serialize}; use helix_loader::grammar::{get_language, load_runtime_file}; +mod overlay; + fn deserialize_regex<'de, D>(deserializer: D) -> Result, D::Error> where D: serde::Deserializer<'de>, @@ -885,7 +889,7 @@ impl Syntax { source: RopeSlice<'a>, range: Option>, cancellation_flag: Option<&'a AtomicUsize>, - ) -> impl Iterator> + 'a { + ) -> HighlightIter<'a> { let mut layers = self .layers .iter() @@ -1142,7 +1146,7 @@ pub enum Error { } /// Represents a single step in rendering a syntax-highlighted document. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum HighlightEvent { Source { start: usize, end: usize }, HighlightStart(Highlight), @@ -1184,7 +1188,36 @@ struct LocalScope<'a> { } #[derive(Debug)] -struct HighlightIter<'a> { +pub struct CharacterHighlightIter<'a>(HighlightIter<'a>); + +impl Iterator for CharacterHighlightIter<'_> { + type Item = HighlightEvent; + + fn next(&mut self) -> Option { + let res = match self.0.next()?.unwrap() { + // TODO: use byte slices directly + // convert byte offsets to char offset + HighlightEvent::Source { start, end } => { + let text = self.0.source; + let start = text.byte_to_char(ensure_grapheme_boundary_next_byte(text, start)); + let end = text.byte_to_char(ensure_grapheme_boundary_next_byte(text, end)); + HighlightEvent::Source { start, end } + } + event => event, + }; + + Some(res) + } +} + +impl<'a> HighlightIter<'a> { + pub fn to_chars(self) -> CharacterHighlightIter<'a> { + CharacterHighlightIter(self) + } +} + +#[derive(Debug)] +pub struct HighlightIter<'a> { source: RopeSlice<'a>, byte_offset: usize, cancellation_flag: Option<&'a AtomicUsize>, @@ -1859,140 +1892,6 @@ fn injection_for_match<'a>( (language_name, content_node, included_children) } -pub struct Merge { - iter: I, - spans: Box)>>, - - next_event: Option, - next_span: Option<(usize, std::ops::Range)>, - - queue: Vec, -} - -/// Merge a list of spans into the highlight event stream. -pub fn merge>( - iter: I, - spans: Vec<(usize, std::ops::Range)>, -) -> Merge { - let spans = Box::new(spans.into_iter()); - let mut merge = Merge { - iter, - spans, - next_event: None, - next_span: None, - queue: Vec::new(), - }; - merge.next_event = merge.iter.next(); - merge.next_span = merge.spans.next(); - merge -} - -impl> Iterator for Merge { - type Item = HighlightEvent; - fn next(&mut self) -> Option { - use HighlightEvent::*; - if let Some(event) = self.queue.pop() { - return Some(event); - } - - loop { - match (self.next_event, &self.next_span) { - // this happens when range is partially or fully offscreen - (Some(Source { start, .. }), Some((span, range))) if start > range.start => { - if start > range.end { - self.next_span = self.spans.next(); - } else { - self.next_span = Some((*span, start..range.end)); - }; - } - _ => break, - } - } - - match (self.next_event, &self.next_span) { - (Some(HighlightStart(i)), _) => { - self.next_event = self.iter.next(); - Some(HighlightStart(i)) - } - (Some(HighlightEnd), _) => { - self.next_event = self.iter.next(); - Some(HighlightEnd) - } - (Some(Source { start, end }), Some((_, range))) if start < range.start => { - let intersect = range.start.min(end); - let event = Source { - start, - end: intersect, - }; - - if end == intersect { - // the event is complete - self.next_event = self.iter.next(); - } else { - // subslice the event - self.next_event = Some(Source { - start: intersect, - end, - }); - }; - - Some(event) - } - (Some(Source { start, end }), Some((span, range))) if start == range.start => { - let intersect = range.end.min(end); - let event = HighlightStart(Highlight(*span)); - - // enqueue in reverse order - self.queue.push(HighlightEnd); - self.queue.push(Source { - start, - end: intersect, - }); - - if end == intersect { - // the event is complete - self.next_event = self.iter.next(); - } else { - // subslice the event - self.next_event = Some(Source { - start: intersect, - end, - }); - }; - - if intersect == range.end { - self.next_span = self.spans.next(); - } else { - self.next_span = Some((*span, intersect..range.end)); - } - - Some(event) - } - (Some(event), None) => { - self.next_event = self.iter.next(); - Some(event) - } - // Can happen if cursor at EOF and/or diagnostic reaches past the end. - // We need to actually emit events for the cursor-at-EOF situation, - // even though the range is past the end of the text. This needs to be - // handled appropriately by the drawing code by not assuming that - // all `Source` events point to valid indices in the rope. - (None, Some((span, range))) => { - let event = HighlightStart(Highlight(*span)); - self.queue.push(HighlightEnd); - self.queue.push(Source { - start: range.start, - end: range.end, - }); - self.next_span = self.spans.next(); - Some(event) - } - (None, None) => None, - e => unreachable!("{:?}", e), - } - } -} - #[cfg(test)] mod test { use super::*; diff --git a/helix-core/src/syntax/overlay.rs b/helix-core/src/syntax/overlay.rs new file mode 100644 index 000000000000..04c771ceae6a --- /dev/null +++ b/helix-core/src/syntax/overlay.rs @@ -0,0 +1,254 @@ +use std::iter::{self, Peekable}; +use std::mem::replace; +use std::ops::Range; + +use crate::syntax::{Highlight, HighlightEvent}; +use HighlightEvent::*; + +#[cfg(test)] +mod test; + +/// Overlays multiple different highlights from `spans` onto the `HighlightEvent` stream `events`. +/// +/// The [`Span`]s yielded by `spans` **must never overlap* or the iterator will produce incorrect results. +/// The [`Span`]s **must be sorted** in ascending order by their start. +/// If multiple [`Span`]s share the same start, the ordering is arbitrary. +/// +/// Together these two properties mean that `spans` must prduce monotonically increasing [`Span`]s. +/// That means that the next span must always start after the last span ended: +/// `next_span.end <= span.start` +pub fn monotonic_overlay( + events: Events, + spans: Spans, +) -> Overlay +where + Events: Iterator, + Spans: Iterator, +{ + let mut overlay = Overlay { + events, + spans: spans.peekable(), + next_event: None, + current_span: None, + queue: EventQueue::new(), + }; + overlay.next_event = overlay.events.next(); + overlay.current_span = overlay.spans.next(); + overlay +} + +/// Overlays a `scope` highlight onto the `HighlightEvent` stream `events` +/// at the ranges specified in `ranges`. +/// +/// Multiple `ranges` **may overlap**, the iterator will merge these ranges into a single range +/// This is possible because all `ranges` use the **same highlighting scope**. +/// +/// The `ranges` **must be sorted** in ascending order by their start. +/// If multiple `ranges` share the same start, the ordering is arbitrary. +pub fn overlapping_overlay( + events: Events, + ranges: Ranges, + scope: Highlight, +) -> Overlay) -> Span>, true> +where + Events: Iterator, + Ranges: Iterator>, +{ + let mut overlay = Overlay { + events, + spans: ranges + .map(move |span| Span { + start: span.start, + end: span.end, + scope, + }) + .peekable(), + next_event: None, + current_span: None, + queue: EventQueue::new(), + }; + overlay.next_event = overlay.events.next(); + overlay.current_span = overlay.spans.next(); + overlay +} + +struct EventQueue { + data: [HighlightEvent; 2], + len: u32, +} + +impl EventQueue { + fn new() -> EventQueue { + EventQueue { + data: [HighlightEnd; 2], + len: 0, + } + } + fn pop(&mut self) -> Option { + if self.len > 0 { + self.len -= 1; + let res = self.data[self.len as usize]; + Some(res) + } else { + None + } + } + + fn push(&mut self, event: HighlightEvent) { + self.data[self.len as usize] = event; + self.len += 1; + } +} + +#[derive(Clone, Copy)] +pub struct Span { + pub scope: Highlight, + pub start: usize, + pub end: usize, +} + +pub struct Overlay +where + Events: Iterator, + Spans: Iterator, +{ + events: Events, + spans: Peekable, + + next_event: Option, + current_span: Option, + + queue: EventQueue, +} + +/// merge spans from `self.spans` that overlap `span` into one larger span +/// this function assumes that `span` and all spans yielded by `spans` +/// have the same scope +fn merge_spans(span: &mut Span, spans: &mut Peekable) +where + I: Iterator, +{ + while let Some(next_span) = spans.peek() { + if next_span.start > span.end { + break; + } + + if next_span.end > span.end { + span.end = next_span.end + } + spans.next(); + } +} + +impl Overlay +where + Events: Iterator, + Spans: Iterator, +{ + fn partition_source_event( + &mut self, + start: usize, + end: usize, + partition_point: usize, + ) -> HighlightEvent { + debug_assert!(start < partition_point && partition_point < end); + let source_1 = Source { + start, + end: partition_point, + }; + let source_2 = Source { + start: partition_point, + end, + }; + self.next_event = Some(source_2); + source_1 + } +} + +impl Iterator for Overlay +where + Events: Iterator, + Spans: Iterator, +{ + type Item = HighlightEvent; + fn next(&mut self) -> Option { + if let Some(event) = self.queue.pop() { + return Some(event); + } + + while let Some(Source { start, end }) = self.next_event { + if start == end { + self.next_event = self.events.next(); + continue; + } + // skip empty spans and spans that end before this source + while matches!(&self.current_span, Some(span) if span.end <= start || span.start == span.end) + { + self.current_span = self.spans.next(); + } + + if let Some(span) = &mut self.current_span { + // only process the span if it actually covers this source (so starts before) + if span.start < end { + // if the span starts inside the source, + // split off the start of the source that is not highlighted + // and emit this sourcespan first + if start < span.start { + let partition_point = span.start; + let unhighlighted = + self.partition_source_event(start, end, partition_point); + return Some(unhighlighted); + } + + // overlapping spans (with the same scope) must be merged into a single span + if MERGE { + merge_spans(span, &mut self.spans); + } + + // copy out the span to satisfy the borrow checker + let span = *span; + + // push `HighlightEnd` and `Source` to queue and return `HighlightStart` right now + self.queue.push(HighlightEnd); + + // advance the span as the current one has been fully processed + if span.end <= end { + self.current_span = self.spans.next(); + } + let event = if span.end < end { + // the span ends before the current source event. + // Add the highlighted part to the queue and process the remainder of the event later + let partition_point = span.end; + self.partition_source_event(start, end, partition_point) + } else { + // advance to the next event as the current one has been fully processed + self.next_event = self.events.next(); + // the source event is fully contained within the span + // just emit the source event to the que and process the next event + Source { start, end } + }; + + self.queue.push(event); + return Some(HighlightStart(span.scope)); + } + } + + break; + } + + match replace(&mut self.next_event, self.events.next()) { + Some(event) => Some(event), + None => { + // Unfinished span at EOF is allowed to finish. + // Special case for cursor past EOF + let span = self.current_span.take()?; + self.queue.push(HighlightEnd); + self.queue.push(Source { + start: span.start, + end: span.end, + }); + Some(HighlightStart(span.scope)) + } + } + } +} diff --git a/helix-core/src/syntax/overlay/test.rs b/helix-core/src/syntax/overlay/test.rs new file mode 100644 index 000000000000..fe0b290c375c --- /dev/null +++ b/helix-core/src/syntax/overlay/test.rs @@ -0,0 +1,32 @@ +use crate::syntax::{overlapping_overlay, Highlight, HighlightEvent::*}; +use std::iter; + +/// this tests checks that merging two overlapping ranges onto each other +/// correctly preveres the order of merges. +/// that is the highlight that is merged in last, gets applied last and overwrites the other layers +/// In this test a range of lower priority (like a hint) starts at 2 +/// and another range of a high priority range (like an error) starts earlier +/// with the old span implementation the hint would always overwrite the error. +/// The new implementation (tested here) forces the +#[test] +fn overlay_long_hint() { + let base = iter::once(Source { start: 0, end: 31 }); + let highlights = overlapping_overlay(base, [2..10].into_iter(), Highlight(1)); + let highlights = overlapping_overlay(highlights, [0..4].into_iter(), Highlight(2)); + let res: Vec<_> = highlights.collect(); + assert_eq!( + &*res, + &[ + HighlightStart(Highlight(2)), + Source { start: 0, end: 2 }, + HighlightEnd, + HighlightStart(Highlight(1)), + HighlightStart(Highlight(2)), + Source { start: 2, end: 4 }, + HighlightEnd, + Source { start: 4, end: 10 }, + HighlightEnd, + Source { start: 10, end: 31 }, + ] + ); +} diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 7cb29c3b1ecb..5db5db84d5d8 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -7,11 +7,10 @@ use crate::{ }; use helix_core::{ - graphemes::{ - ensure_grapheme_boundary_next_byte, next_grapheme_boundary, prev_grapheme_boundary, - }, + diagnostic::Severity, + graphemes::{next_grapheme_boundary, prev_grapheme_boundary}, movement::Direction, - syntax::{self, HighlightEvent}, + syntax::{self, CharacterHighlightIter, Highlight, HighlightEvent}, unicode::width::UnicodeWidthStr, LineEnding, Position, Range, Selection, Transaction, }; @@ -23,7 +22,7 @@ use helix_view::{ keyboard::{KeyCode, KeyModifiers}, Document, Editor, Theme, View, }; -use std::{borrow::Cow, path::PathBuf}; +use std::{borrow::Cow, iter, ops, path::PathBuf}; use tui::buffer::Buffer as Surface; @@ -66,6 +65,52 @@ impl EditorView { &mut self.spinners } + #[allow(clippy::too_many_arguments)] + fn render_text( + base: impl Iterator, + editor: &Editor, + doc: &Document, + view: &View, + text_area: Rect, + theme: &Theme, + surface: &mut Surface, + is_focused: bool, + ) { + let highlights = + Self::overlay_diagnostics_highlights(base, doc, theme, Some(Severity::Hint)); + let highlights = + Self::overlay_diagnostics_highlights(highlights, doc, theme, Some(Severity::Info)); + let highlights = Self::overlay_diagnostics_highlights(highlights, doc, theme, None); + let highlights = + Self::overlay_diagnostics_highlights(highlights, doc, theme, Some(Severity::Warning)); + let highlights = + Self::overlay_diagnostics_highlights(highlights, doc, theme, Some(Severity::Error)); + let doc_selection_highlights = if is_focused { + Self::doc_selection_highlights( + editor.mode(), + doc, + view, + theme, + &editor.config().cursor_shape, + ) + } else { + Vec::new() + }; + + let highlights = + syntax::monotonic_overlay(highlights, doc_selection_highlights.into_iter()); + + Self::render_text_highlights( + doc, + view.offset, + text_area, + surface, + theme, + highlights, + &editor.config(), + ); + } + pub fn render_view( &self, editor: &Editor, @@ -117,32 +162,14 @@ impl EditorView { Self::highlight_cursorline(doc, view, surface, theme); } - let highlights = Self::doc_syntax_highlights(doc, view.offset, inner.height, theme); - let highlights = syntax::merge(highlights, Self::doc_diagnostics_highlights(doc, theme)); - let highlights: Box> = if is_focused { - Box::new(syntax::merge( - highlights, - Self::doc_selection_highlights( - editor.mode(), - doc, - view, - theme, - &editor.config().cursor_shape, - ), - )) - } else { - Box::new(highlights) - }; - - Self::render_text_highlights( - doc, - view.offset, - inner, - surface, - theme, - highlights, - &editor.config(), - ); + match Self::doc_syntax_highlights(doc, view.offset, inner.height, theme) { + DocSyntaxHighlight::TreeSitter(base) => { + Self::render_text(base, editor, doc, view, inner, theme, surface, is_focused) + } + DocSyntaxHighlight::Default(base) => { + Self::render_text(base, editor, doc, view, inner, theme, surface, is_focused) + } + } Self::render_gutter(editor, doc, view, view.area, surface, theme, is_focused); Self::render_rulers(editor, doc, view, inner, surface, theme); @@ -211,7 +238,7 @@ impl EditorView { offset: Position, height: u16, _theme: &Theme, - ) -> Box + 'doc> { + ) -> DocSyntaxHighlight<'doc> { let text = doc.text().slice(..); let last_line = std::cmp::min( // Saturating subs to make it inclusive zero indexing. @@ -229,41 +256,30 @@ impl EditorView { match doc.syntax() { Some(syntax) => { + // TODO: range doesn't actually restrict source, just highlight range + // TODO: use byte slices directly + // convert byte offsets to char offset let iter = syntax - // TODO: range doesn't actually restrict source, just highlight range .highlight_iter(text.slice(..), Some(range), None) - .map(|event| event.unwrap()) - .map(move |event| match event { - // TODO: use byte slices directly - // convert byte offsets to char offset - HighlightEvent::Source { start, end } => { - let start = - text.byte_to_char(ensure_grapheme_boundary_next_byte(text, start)); - let end = - text.byte_to_char(ensure_grapheme_boundary_next_byte(text, end)); - HighlightEvent::Source { start, end } - } - event => event, - }); - - Box::new(iter) + .to_chars(); + DocSyntaxHighlight::TreeSitter(iter) } - None => Box::new( - [HighlightEvent::Source { + None => { + let event = HighlightEvent::Source { start: text.byte_to_char(range.start), end: text.byte_to_char(range.end), - }] - .into_iter(), - ), + }; + DocSyntaxHighlight::Default(iter::once(event)) + } } } /// Get highlight spans for document diagnostics - pub fn doc_diagnostics_highlights( - doc: &Document, + pub fn doc_diagnostics_highlights<'d>( + doc: &'d Document, theme: &Theme, - ) -> Vec<(usize, std::ops::Range)> { - use helix_core::diagnostic::Severity; + severity: Option, + ) -> (Highlight, impl Iterator> + 'd) { let get_scope_of = |scope| { theme .find_scope_index(scope) @@ -276,29 +292,32 @@ impl EditorView { ) }; - // basically just queries the theme color defined in the config - let hint = get_scope_of("diagnostic.hint"); - let info = get_scope_of("diagnostic.info"); - let warning = get_scope_of("diagnostic.warning"); - let error = get_scope_of("diagnostic.error"); - let r#default = get_scope_of("diagnostic"); // this is a bit redundant but should be fine + let highlight = match severity { + Some(Severity::Error) => get_scope_of("diagnostic.error"), + Some(Severity::Warning) => get_scope_of("diagnostic.warning"), + Some(Severity::Info) => get_scope_of("diagnostic.info"), + Some(Severity::Hint) => get_scope_of("diagnostic.hint"), + None => get_scope_of("diagnostic"), + }; - doc.diagnostics() - .iter() - .map(|diagnostic| { - let diagnostic_scope = match diagnostic.severity { - Some(Severity::Info) => info, - Some(Severity::Hint) => hint, - Some(Severity::Warning) => warning, - Some(Severity::Error) => error, - _ => r#default, - }; - ( - diagnostic_scope, - diagnostic.range.start..diagnostic.range.end, - ) - }) - .collect() + let diagnostic_ranges = doc.diagnostics().iter().filter_map(move |diagnostic| { + if diagnostic.severity != severity { + return None; + } + Some(diagnostic.range.start..diagnostic.range.end) + }); + + (Highlight(highlight), diagnostic_ranges) + } + + pub fn overlay_diagnostics_highlights<'d>( + events: impl Iterator + 'd, + doc: &'d Document, + theme: &Theme, + severity: Option, + ) -> impl Iterator + 'd { + let (highlight, ranges) = Self::doc_diagnostics_highlights(doc, theme, severity); + syntax::overlapping_overlay(events, ranges, highlight) } /// Get highlight spans for selections in a document view. @@ -308,7 +327,7 @@ impl EditorView { view: &View, theme: &Theme, cursor_shape_config: &CursorShapeConfig, - ) -> Vec<(usize, std::ops::Range)> { + ) -> Vec { let text = doc.text().slice(..); let selection = doc.selection(view.id); let primary_idx = selection.primary_index(); @@ -337,7 +356,7 @@ impl EditorView { .find_scope_index("ui.selection.primary") .unwrap_or(selection_scope); - let mut spans: Vec<(usize, std::ops::Range)> = Vec::new(); + let mut spans: Vec = Vec::new(); for (i, range) in selection.iter().enumerate() { let selection_is_primary = i == primary_idx; let (cursor_scope, selection_scope) = if selection_is_primary { @@ -354,7 +373,11 @@ impl EditorView { // underline cursor (eg. when a regex prompt has focus) then // the primary cursor will be invisible. This doesn't happen // with block cursors since we manually draw *all* cursors. - spans.push((cursor_scope, range.head..range.head + 1)); + spans.push(syntax::Span { + scope: Highlight(cursor_scope), + start: range.head, + end: range.head + 1, + }); } continue; } @@ -363,17 +386,33 @@ impl EditorView { if range.head > range.anchor { // Standard case. let cursor_start = prev_grapheme_boundary(text, range.head); - spans.push((selection_scope, range.anchor..cursor_start)); + spans.push(syntax::Span { + scope: Highlight(selection_scope), + start: range.anchor, + end: cursor_start, + }); if !selection_is_primary || cursor_is_block { - spans.push((cursor_scope, cursor_start..range.head)); + spans.push(syntax::Span { + scope: Highlight(cursor_scope), + start: cursor_start, + end: range.head, + }); } } else { // Reverse case. let cursor_end = next_grapheme_boundary(text, range.head); if !selection_is_primary || cursor_is_block { - spans.push((cursor_scope, range.head..cursor_end)); + spans.push(syntax::Span { + scope: Highlight(cursor_scope), + start: range.head, + end: cursor_end, + }); } - spans.push((selection_scope, cursor_end..range.anchor)); + spans.push(syntax::Span { + scope: Highlight(selection_scope), + start: cursor_end, + end: range.anchor, + }); } } @@ -736,7 +775,6 @@ impl EditorView { surface: &mut Surface, theme: &Theme, ) { - use helix_core::diagnostic::Severity; use tui::{ layout::Alignment, text::Text, @@ -1385,7 +1423,6 @@ impl Component for EditorView { // render status msg if let Some((status_msg, severity)) = &cx.editor.status_msg { status_msg_width = status_msg.width(); - use helix_view::editor::Severity; let style = if *severity == Severity::Error { cx.editor.theme.get("error") } else { @@ -1461,3 +1498,8 @@ fn canonicalize_key(key: &mut KeyEvent) { key.modifiers.remove(KeyModifiers::SHIFT) } } + +pub enum DocSyntaxHighlight<'d> { + TreeSitter(CharacterHighlightIter<'d>), + Default(iter::Once), +} diff --git a/helix-term/src/ui/lsp.rs b/helix-term/src/ui/lsp.rs index f2854551ded0..3b790b1f45ec 100644 --- a/helix-term/src/ui/lsp.rs +++ b/helix-term/src/ui/lsp.rs @@ -1,6 +1,7 @@ +use std::iter; use std::sync::Arc; -use helix_core::syntax; +use helix_core::syntax::{self, Highlight}; use helix_view::graphics::{Margin, Rect, Style}; use tui::buffer::Buffer; use tui::widgets::{BorderType, Paragraph, Widget, Wrap}; @@ -52,10 +53,12 @@ impl Component for SignatureHelp { let margin = Margin::horizontal(1); let active_param_span = self.active_param_range.map(|(start, end)| { - vec![( - cx.editor.theme.find_scope_index("ui.selection").unwrap(), - start..end, - )] + let highlight = cx.editor.theme.find_scope_index("ui.selection").unwrap(); + syntax::Span { + scope: Highlight(highlight), + start, + end, + } }); let sig_text = crate::ui::markdown::highlighted_code_block( @@ -63,7 +66,7 @@ impl Component for SignatureHelp { &self.language, Some(&cx.editor.theme), Arc::clone(&self.config_loader), - active_param_span, + active_param_span.into_iter(), ); let (_, sig_text_height) = crate::ui::text::required_size(&sig_text, area.width); @@ -109,7 +112,7 @@ impl Component for SignatureHelp { &self.language, None, Arc::clone(&self.config_loader), - None, + iter::empty(), ); let (sig_width, sig_height) = crate::ui::text::required_size(&signature_text, max_text_width); diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index 923dd73a16ab..5bb92fbc8d5e 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -4,7 +4,7 @@ use tui::{ text::{Span, Spans, Text}, }; -use std::sync::Arc; +use std::{iter, sync::Arc}; use pulldown_cmark::{CodeBlockKind, Event, HeadingLevel, Options, Parser, Tag}; @@ -31,7 +31,7 @@ pub fn highlighted_code_block<'a>( language: &str, theme: Option<&Theme>, config_loader: Arc, - additional_highlight_spans: Option)>>, + additional_highlight_spans: impl Iterator, ) -> Text<'a> { let mut spans = Vec::new(); let mut lines = Vec::new(); @@ -59,13 +59,7 @@ pub fn highlighted_code_block<'a>( let highlight_iter = syntax .highlight_iter(rope.slice(..), None, None) .map(|e| e.unwrap()); - let highlight_iter: Box> = - if let Some(spans) = additional_highlight_spans { - Box::new(helix_core::syntax::merge(highlight_iter, spans)) - } else { - Box::new(highlight_iter) - }; - + let highlight_iter = syntax::monotonic_overlay(highlight_iter, additional_highlight_spans); let mut highlights = Vec::new(); for event in highlight_iter { match event { @@ -269,7 +263,7 @@ impl Markdown { language, theme, Arc::clone(&self.config_loader), - None, + iter::empty(), ); lines.extend(tui_text.lines.into_iter()); } else { diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index a56455d7d569..e02922951698 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -1,7 +1,7 @@ use crate::{ compositor::{Component, Compositor, Context, Event, EventResult}, ctrl, key, shift, - ui::{self, EditorView}, + ui::{self, editor::DocSyntaxHighlight, EditorView}, }; use tui::{ buffer::Buffer as Surface, @@ -228,17 +228,26 @@ impl Component for FilePicker { let offset = Position::new(first_line, 0); - let highlights = - EditorView::doc_syntax_highlights(doc, offset, area.height, &cx.editor.theme); - EditorView::render_text_highlights( - doc, - offset, - inner, - surface, - &cx.editor.theme, - highlights, - &cx.editor.config(), - ); + match EditorView::doc_syntax_highlights(doc, offset, area.height, &cx.editor.theme) { + DocSyntaxHighlight::Default(highlights) => EditorView::render_text_highlights( + doc, + offset, + inner, + surface, + &cx.editor.theme, + highlights, + &cx.editor.config(), + ), + DocSyntaxHighlight::TreeSitter(highlights) => EditorView::render_text_highlights( + doc, + offset, + inner, + surface, + &cx.editor.theme, + highlights, + &cx.editor.config(), + ), + } // highlight the line if let Some((start, end)) = range { From ed3cdd31064f63c7b1f8f306938cafe0f06bcfd9 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 3 Oct 2022 20:51:21 +0200 Subject: [PATCH 2/9] Apply stylistic suggestions from code review Co-authored-by: Michael Davis --- helix-core/src/syntax/overlay.rs | 9 ++++----- helix-core/src/syntax/overlay/test.rs | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/helix-core/src/syntax/overlay.rs b/helix-core/src/syntax/overlay.rs index 04c771ceae6a..5b0bfe6f068a 100644 --- a/helix-core/src/syntax/overlay.rs +++ b/helix-core/src/syntax/overlay.rs @@ -10,13 +10,13 @@ mod test; /// Overlays multiple different highlights from `spans` onto the `HighlightEvent` stream `events`. /// -/// The [`Span`]s yielded by `spans` **must never overlap* or the iterator will produce incorrect results. +/// The [`Span`]s yielded by `spans` **must never overlap** or the iterator will produce incorrect results. /// The [`Span`]s **must be sorted** in ascending order by their start. /// If multiple [`Span`]s share the same start, the ordering is arbitrary. /// /// Together these two properties mean that `spans` must prduce monotonically increasing [`Span`]s. /// That means that the next span must always start after the last span ended: -/// `next_span.end <= span.start` +/// `span.end <= next_span.start` pub fn monotonic_overlay( events: Events, spans: Spans, @@ -192,7 +192,7 @@ where if span.start < end { // if the span starts inside the source, // split off the start of the source that is not highlighted - // and emit this sourcespan first + // and emit this source span first if start < span.start { let partition_point = span.start; let unhighlighted = @@ -239,8 +239,7 @@ where match replace(&mut self.next_event, self.events.next()) { Some(event) => Some(event), None => { - // Unfinished span at EOF is allowed to finish. - // Special case for cursor past EOF + // Unfinished span at EOF is allowed to finish. let span = self.current_span.take()?; self.queue.push(HighlightEnd); self.queue.push(Source { diff --git a/helix-core/src/syntax/overlay/test.rs b/helix-core/src/syntax/overlay/test.rs index fe0b290c375c..d78999f564ff 100644 --- a/helix-core/src/syntax/overlay/test.rs +++ b/helix-core/src/syntax/overlay/test.rs @@ -15,7 +15,7 @@ fn overlay_long_hint() { let highlights = overlapping_overlay(highlights, [0..4].into_iter(), Highlight(2)); let res: Vec<_> = highlights.collect(); assert_eq!( - &*res, + res, &[ HighlightStart(Highlight(2)), Source { start: 0, end: 2 }, From 2c6f681ffcf1662b7ff2d868bf090f3fc13fb185 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 3 Oct 2022 20:48:47 +0200 Subject: [PATCH 3/9] more elegant static dispatch --- helix-core/src/syntax.rs | 4 +- helix-core/src/syntax/overlay.rs | 35 ++- helix-term/src/ui/editor.rs | 292 +++++++------------------ helix-term/src/ui/editor/highlights.rs | 281 ++++++++++++++++++++++++ helix-term/src/ui/picker.rs | 30 +-- 5 files changed, 392 insertions(+), 250 deletions(-) create mode 100644 helix-term/src/ui/editor/highlights.rs diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index d3f4e7a2d0fa..24fd92460476 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -7,7 +7,9 @@ use crate::{ transaction::{ChangeSet, Operation}, Rope, RopeSlice, Tendril, }; -pub use overlay::{monotonic_overlay, overlapping_overlay, Span}; +pub use overlay::{ + monotonic_overlay, overlapping_overlay, MonotonicOverlay, OverlappingOverlay, Span, +}; use arc_swap::{ArcSwap, Guard}; use slotmap::{DefaultKey as LayerId, HopSlotMap}; diff --git a/helix-core/src/syntax/overlay.rs b/helix-core/src/syntax/overlay.rs index 5b0bfe6f068a..e975aca58960 100644 --- a/helix-core/src/syntax/overlay.rs +++ b/helix-core/src/syntax/overlay.rs @@ -1,4 +1,4 @@ -use std::iter::{self, Peekable}; +use std::iter::Peekable; use std::mem::replace; use std::ops::Range; @@ -8,6 +8,8 @@ use HighlightEvent::*; #[cfg(test)] mod test; +pub type MonotonicOverlay = Overlay; + /// Overlays multiple different highlights from `spans` onto the `HighlightEvent` stream `events`. /// /// The [`Span`]s yielded by `spans` **must never overlap** or the iterator will produce incorrect results. @@ -20,7 +22,7 @@ mod test; pub fn monotonic_overlay( events: Events, spans: Spans, -) -> Overlay +) -> MonotonicOverlay where Events: Iterator, Spans: Iterator, @@ -37,6 +39,25 @@ where overlay } +pub struct RangeToSpan>> { + scope: Highlight, + ranges: I, +} + +impl>> Iterator for RangeToSpan { + type Item = Span; + + fn next(&mut self) -> Option { + self.ranges.next().map(|range| Span { + start: range.start, + end: range.end, + scope: self.scope, + }) + } +} + +pub type OverlappingOverlay = Overlay, true>; + /// Overlays a `scope` highlight onto the `HighlightEvent` stream `events` /// at the ranges specified in `ranges`. /// @@ -49,20 +70,14 @@ pub fn overlapping_overlay( events: Events, ranges: Ranges, scope: Highlight, -) -> Overlay) -> Span>, true> +) -> OverlappingOverlay where Events: Iterator, Ranges: Iterator>, { let mut overlay = Overlay { events, - spans: ranges - .map(move |span| Span { - start: span.start, - end: span.end, - scope, - }) - .peekable(), + spans: RangeToSpan { scope, ranges }.peekable(), next_event: None, current_span: None, queue: EventQueue::new(), diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 5db5db84d5d8..4506f240505b 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -3,32 +3,36 @@ use crate::{ compositor::{Component, Context, Event, EventResult}, job, key, keymap::{KeymapResult, Keymaps}, - ui::{Completion, ProgressSpinners}, + ui::{ + editor::highlights::{all_diganostic_overlays, HighlightOverlay, SelectionOverlay}, + Completion, ProgressSpinners, + }, }; use helix_core::{ diagnostic::Severity, - graphemes::{next_grapheme_boundary, prev_grapheme_boundary}, movement::Direction, - syntax::{self, CharacterHighlightIter, Highlight, HighlightEvent}, + syntax::{CharacterHighlightIter, HighlightEvent}, unicode::width::UnicodeWidthStr, LineEnding, Position, Range, Selection, Transaction, }; use helix_view::{ document::{Mode, SCRATCH_BUFFER_NAME}, - editor::{CompleteAction, CursorShapeConfig}, + editor::CompleteAction, graphics::{Color, CursorKind, Modifier, Rect, Style}, input::{KeyEvent, MouseButton, MouseEvent, MouseEventKind}, keyboard::{KeyCode, KeyModifiers}, Document, Editor, Theme, View, }; -use std::{borrow::Cow, iter, ops, path::PathBuf}; +use std::{borrow::Cow, iter, path::PathBuf}; use tui::buffer::Buffer as Surface; use super::lsp::SignatureHelp; use super::statusline; +mod highlights; + pub struct EditorView { pub keymaps: Keymaps, on_next_key: Option>, @@ -65,52 +69,6 @@ impl EditorView { &mut self.spinners } - #[allow(clippy::too_many_arguments)] - fn render_text( - base: impl Iterator, - editor: &Editor, - doc: &Document, - view: &View, - text_area: Rect, - theme: &Theme, - surface: &mut Surface, - is_focused: bool, - ) { - let highlights = - Self::overlay_diagnostics_highlights(base, doc, theme, Some(Severity::Hint)); - let highlights = - Self::overlay_diagnostics_highlights(highlights, doc, theme, Some(Severity::Info)); - let highlights = Self::overlay_diagnostics_highlights(highlights, doc, theme, None); - let highlights = - Self::overlay_diagnostics_highlights(highlights, doc, theme, Some(Severity::Warning)); - let highlights = - Self::overlay_diagnostics_highlights(highlights, doc, theme, Some(Severity::Error)); - let doc_selection_highlights = if is_focused { - Self::doc_selection_highlights( - editor.mode(), - doc, - view, - theme, - &editor.config().cursor_shape, - ) - } else { - Vec::new() - }; - - let highlights = - syntax::monotonic_overlay(highlights, doc_selection_highlights.into_iter()); - - Self::render_text_highlights( - doc, - view.offset, - text_area, - surface, - theme, - highlights, - &editor.config(), - ); - } - pub fn render_view( &self, editor: &Editor, @@ -162,14 +120,24 @@ impl EditorView { Self::highlight_cursorline(doc, view, surface, theme); } - match Self::doc_syntax_highlights(doc, view.offset, inner.height, theme) { - DocSyntaxHighlight::TreeSitter(base) => { - Self::render_text(base, editor, doc, view, inner, theme, surface, is_focused) - } - DocSyntaxHighlight::Default(base) => { - Self::render_text(base, editor, doc, view, inner, theme, surface, is_focused) - } - } + let diagnostics_overlay = all_diganostic_overlays(doc, theme); + let selection_overlay = SelectionOverlay { + focused: is_focused, + editor, + doc, + theme, + view, + }; + + Self::render_document_with_overlay( + doc, + inner, + surface, + view.offset, + theme, + &*editor.config(), + (diagnostics_overlay, selection_overlay), + ); Self::render_gutter(editor, doc, view, view.area, surface, theme, is_focused); Self::render_rulers(editor, doc, view, inner, surface, theme); @@ -230,15 +198,37 @@ impl EditorView { .for_each(|area| surface.set_style(area, ruler_theme)) } - /// Get syntax highlights for a document in a view represented by the first line - /// and column (`offset`) and the last line. This is done instead of using a view - /// directly to enable rendering syntax highlighted docs anywhere (eg. picker preview) - pub fn doc_syntax_highlights<'doc>( + /// Get syntax highlights for a document in `viewport` and renders the result onto `surface`. + /// This is done instead of using a view directly to enable rendering + /// syntax highlighted docs anywhere (eg. picker preview) + pub fn render_document<'doc>( + doc: &'doc Document, + viewport: Rect, + surface: &mut Surface, + offset: Position, + theme: &Theme, + config: &helix_view::editor::Config, + ) { + Self::render_document_with_overlay(doc, viewport, surface, offset, theme, config, ()) + } + + /// Get syntax highlights for a document in `viewport`, + /// applies `overlay` and renders the result onto `surface`. + /// This is done instead of using a view directly to enable rendering + /// syntax highlighted docs anywhere (eg. picker preview) + pub fn render_document_with_overlay<'doc, O>( doc: &'doc Document, + viewport: Rect, + surface: &mut Surface, offset: Position, - height: u16, - _theme: &Theme, - ) -> DocSyntaxHighlight<'doc> { + theme: &Theme, + config: &helix_view::editor::Config, + overlay: O, + ) where + O: HighlightOverlay> + + HighlightOverlay>, + { + let height = viewport.height; let text = doc.text().slice(..); let last_line = std::cmp::min( // Saturating subs to make it inclusive zero indexing. @@ -262,161 +252,32 @@ impl EditorView { let iter = syntax .highlight_iter(text.slice(..), Some(range), None) .to_chars(); - DocSyntaxHighlight::TreeSitter(iter) + Self::render_text_highlights( + doc, + offset, + viewport, + surface, + theme, + overlay.apply(iter), + config, + ); } None => { let event = HighlightEvent::Source { start: text.byte_to_char(range.start), end: text.byte_to_char(range.end), }; - DocSyntaxHighlight::Default(iter::once(event)) - } - } - } - - /// Get highlight spans for document diagnostics - pub fn doc_diagnostics_highlights<'d>( - doc: &'d Document, - theme: &Theme, - severity: Option, - ) -> (Highlight, impl Iterator> + 'd) { - let get_scope_of = |scope| { - theme - .find_scope_index(scope) - // get one of the themes below as fallback values - .or_else(|| theme.find_scope_index("diagnostic")) - .or_else(|| theme.find_scope_index("ui.cursor")) - .or_else(|| theme.find_scope_index("ui.selection")) - .expect( - "at least one of the following scopes must be defined in the theme: `diagnostic`, `ui.cursor`, or `ui.selection`", - ) - }; - - let highlight = match severity { - Some(Severity::Error) => get_scope_of("diagnostic.error"), - Some(Severity::Warning) => get_scope_of("diagnostic.warning"), - Some(Severity::Info) => get_scope_of("diagnostic.info"), - Some(Severity::Hint) => get_scope_of("diagnostic.hint"), - None => get_scope_of("diagnostic"), - }; - - let diagnostic_ranges = doc.diagnostics().iter().filter_map(move |diagnostic| { - if diagnostic.severity != severity { - return None; - } - Some(diagnostic.range.start..diagnostic.range.end) - }); - - (Highlight(highlight), diagnostic_ranges) - } - - pub fn overlay_diagnostics_highlights<'d>( - events: impl Iterator + 'd, - doc: &'d Document, - theme: &Theme, - severity: Option, - ) -> impl Iterator + 'd { - let (highlight, ranges) = Self::doc_diagnostics_highlights(doc, theme, severity); - syntax::overlapping_overlay(events, ranges, highlight) - } - - /// Get highlight spans for selections in a document view. - pub fn doc_selection_highlights( - mode: Mode, - doc: &Document, - view: &View, - theme: &Theme, - cursor_shape_config: &CursorShapeConfig, - ) -> Vec { - let text = doc.text().slice(..); - let selection = doc.selection(view.id); - let primary_idx = selection.primary_index(); - - let cursorkind = cursor_shape_config.from_mode(mode); - let cursor_is_block = cursorkind == CursorKind::Block; - - let selection_scope = theme - .find_scope_index("ui.selection") - .expect("could not find `ui.selection` scope in the theme!"); - let base_cursor_scope = theme - .find_scope_index("ui.cursor") - .unwrap_or(selection_scope); - - let cursor_scope = match mode { - Mode::Insert => theme.find_scope_index("ui.cursor.insert"), - Mode::Select => theme.find_scope_index("ui.cursor.select"), - Mode::Normal => Some(base_cursor_scope), - } - .unwrap_or(base_cursor_scope); - - let primary_cursor_scope = theme - .find_scope_index("ui.cursor.primary") - .unwrap_or(cursor_scope); - let primary_selection_scope = theme - .find_scope_index("ui.selection.primary") - .unwrap_or(selection_scope); - - let mut spans: Vec = Vec::new(); - for (i, range) in selection.iter().enumerate() { - let selection_is_primary = i == primary_idx; - let (cursor_scope, selection_scope) = if selection_is_primary { - (primary_cursor_scope, primary_selection_scope) - } else { - (cursor_scope, selection_scope) - }; - - // Special-case: cursor at end of the rope. - if range.head == range.anchor && range.head == text.len_chars() { - if !selection_is_primary || cursor_is_block { - // Bar and underline cursors are drawn by the terminal - // BUG: If the editor area loses focus while having a bar or - // underline cursor (eg. when a regex prompt has focus) then - // the primary cursor will be invisible. This doesn't happen - // with block cursors since we manually draw *all* cursors. - spans.push(syntax::Span { - scope: Highlight(cursor_scope), - start: range.head, - end: range.head + 1, - }); - } - continue; - } - - let range = range.min_width_1(text); - if range.head > range.anchor { - // Standard case. - let cursor_start = prev_grapheme_boundary(text, range.head); - spans.push(syntax::Span { - scope: Highlight(selection_scope), - start: range.anchor, - end: cursor_start, - }); - if !selection_is_primary || cursor_is_block { - spans.push(syntax::Span { - scope: Highlight(cursor_scope), - start: cursor_start, - end: range.head, - }); - } - } else { - // Reverse case. - let cursor_end = next_grapheme_boundary(text, range.head); - if !selection_is_primary || cursor_is_block { - spans.push(syntax::Span { - scope: Highlight(cursor_scope), - start: range.head, - end: cursor_end, - }); - } - spans.push(syntax::Span { - scope: Highlight(selection_scope), - start: cursor_end, - end: range.anchor, - }); + Self::render_text_highlights( + doc, + offset, + viewport, + surface, + theme, + overlay.apply(iter::once(event)), + config, + ); } } - - spans } pub fn render_text_highlights>( @@ -1498,8 +1359,3 @@ fn canonicalize_key(key: &mut KeyEvent) { key.modifiers.remove(KeyModifiers::SHIFT) } } - -pub enum DocSyntaxHighlight<'d> { - TreeSitter(CharacterHighlightIter<'d>), - Default(iter::Once), -} diff --git a/helix-term/src/ui/editor/highlights.rs b/helix-term/src/ui/editor/highlights.rs new file mode 100644 index 000000000000..72430ba548f5 --- /dev/null +++ b/helix-term/src/ui/editor/highlights.rs @@ -0,0 +1,281 @@ +use std::ops::Range; +use std::{slice, vec}; + +use helix_core::diagnostic::Severity; +use helix_core::graphemes::{next_grapheme_boundary, prev_grapheme_boundary}; +use helix_core::syntax::{ + self, monotonic_overlay, overlapping_overlay, Highlight, HighlightEvent, MonotonicOverlay, +}; +use helix_core::Diagnostic; +use helix_view::document::Mode; +use helix_view::graphics::CursorKind; +use helix_view::{Document, Editor, Theme, View}; + +pub trait HighlightOverlay> { + type Out: Iterator; + fn apply(self, highlights: In) -> Self::Out; +} + +macro_rules! impl_composition { + ($first: ident, $last: ident $(,$names: ident)*) => { + impl_composition!(@impl [$first, $($names,)* $last] [$first $(,$names)*] [$($names,)* $last] $first $last); + }; + + ($first: ident) => {}; + + (@impl [$($names: ident),+] [$($head: ident),*] [$($tail: ident),*] $first: ident $last: ident) => { + #[allow(non_snake_case)] + impl HighlightOverlay for ($($names),+) + where + In: Iterator, + $first: HighlightOverlay, + $($tail: HighlightOverlay<$head::Out>),+ + { + type Out = $last::Out; + + fn apply(self, highlights: In) -> Self::Out { + let ($($names),+) = self; + $(let highlights = $names.apply(highlights);)+ + highlights + } + } + impl_composition!($($head),*); + }; + (@impl [$name: ident] [$first: ident] [$last: ident]) => {}; +} + +impl_composition!(A, B, C, D, E, F, G, H, I, J, K); + +pub struct DiagnosticsOverlay<'a> { + pub theme: &'a Theme, + pub doc: &'a Document, + pub severity: Option, +} + +pub struct DiagnosticIter<'doc> { + diagnostics: slice::Iter<'doc, Diagnostic>, + severity: Option, +} + +impl<'doc> Iterator for DiagnosticIter<'doc> { + type Item = Range; + + fn next(&mut self) -> Option { + self.diagnostics.find_map(|diagnostic| { + if diagnostic.severity != self.severity { + return None; + } + Some(diagnostic.range.start..diagnostic.range.end) + }) + } +} + +impl<'a, In> HighlightOverlay for DiagnosticsOverlay<'a> +where + In: Iterator, +{ + type Out = syntax::OverlappingOverlay>; + fn apply(self, highlights: In) -> Self::Out { + let get_scope_of = |scope| { + self.theme + .find_scope_index(scope) + // get one of the themes below as fallback values + .or_else(|| self.theme.find_scope_index("diagnostic")) + .or_else(|| self.theme.find_scope_index("ui.cursor")) + .or_else(|| self.theme.find_scope_index("ui.selection")) + .expect( + "at least one of the following scopes must be defined in the theme: `diagnostic`, `ui.cursor`, or `ui.selection`", + ) + }; + + let highlight = match self.severity { + Some(Severity::Error) => get_scope_of("diagnostic.error"), + Some(Severity::Warning) => get_scope_of("diagnostic.warning"), + Some(Severity::Info) => get_scope_of("diagnostic.info"), + Some(Severity::Hint) => get_scope_of("diagnostic.hint"), + None => get_scope_of("diagnostic"), + }; + + overlapping_overlay( + highlights, + DiagnosticIter { + diagnostics: self.doc.diagnostics().iter(), + severity: self.severity, + }, + Highlight(highlight), + ) + } +} + +pub type AllDiagnosticsOverlay<'a> = ( + DiagnosticsOverlay<'a>, + DiagnosticsOverlay<'a>, + DiagnosticsOverlay<'a>, + DiagnosticsOverlay<'a>, + DiagnosticsOverlay<'a>, +); + +pub fn all_diganostic_overlays<'a>( + doc: &'a Document, + theme: &'a Theme, +) -> AllDiagnosticsOverlay<'a> { + ( + DiagnosticsOverlay { + theme, + doc, + severity: Some(Severity::Hint), + }, + DiagnosticsOverlay { + theme, + doc, + severity: Some(Severity::Info), + }, + DiagnosticsOverlay { + theme, + doc, + severity: None, + }, + DiagnosticsOverlay { + theme, + doc, + severity: Some(Severity::Warning), + }, + DiagnosticsOverlay { + theme, + doc, + severity: Some(Severity::Error), + }, + ) +} + +pub struct SelectionOverlay<'a> { + pub focused: bool, + pub editor: &'a Editor, + pub doc: &'a Document, + pub theme: &'a Theme, + pub view: &'a View, +} + +impl<'a, In> HighlightOverlay for SelectionOverlay<'a> +where + In: Iterator, +{ + type Out = MonotonicOverlay>; + + fn apply(self, highlights: In) -> Self::Out { + let SelectionOverlay { + focused, + editor, + doc, + theme, + view, + } = self; + + if !focused { + return monotonic_overlay(highlights, Vec::new().into_iter()); + } + + // TODO avoid collecting into a vec + let text = doc.text().slice(..); + let selection = doc.selection(view.id); + let primary_idx = selection.primary_index(); + let mode = editor.mode(); + + let cursorkind = editor.config().cursor_shape.from_mode(mode); + let cursor_is_block = cursorkind == CursorKind::Block; + + let selection_scope = theme + .find_scope_index("ui.selection") + .expect("could not find `ui.selection` scope in the theme!"); + let base_cursor_scope = theme + .find_scope_index("ui.cursor") + .unwrap_or(selection_scope); + + let cursor_scope = match mode { + Mode::Insert => theme.find_scope_index("ui.cursor.insert"), + Mode::Select => theme.find_scope_index("ui.cursor.select"), + Mode::Normal => Some(base_cursor_scope), + } + .unwrap_or(base_cursor_scope); + + let primary_cursor_scope = theme + .find_scope_index("ui.cursor.primary") + .unwrap_or(cursor_scope); + let primary_selection_scope = theme + .find_scope_index("ui.selection.primary") + .unwrap_or(selection_scope); + + let mut spans: Vec = Vec::new(); + for (i, range) in selection.iter().enumerate() { + let selection_is_primary = i == primary_idx; + let (cursor_scope, selection_scope) = if selection_is_primary { + (primary_cursor_scope, primary_selection_scope) + } else { + (cursor_scope, selection_scope) + }; + + // Special-case: cursor at end of the rope. + if range.head == range.anchor && range.head == text.len_chars() { + if !selection_is_primary || cursor_is_block { + // Bar and underline cursors are drawn by the terminal + // BUG: If the editor area loses focus while having a bar or + // underline cursor (eg. when a regex prompt has focus) then + // the primary cursor will be invisible. This doesn't happen + // with block cursors since we manually draw *all* cursors. + spans.push(syntax::Span { + scope: Highlight(cursor_scope), + start: range.head, + end: range.head + 1, + }); + } + continue; + } + + let range = range.min_width_1(text); + if range.head > range.anchor { + // Standard case. + let cursor_start = prev_grapheme_boundary(text, range.head); + spans.push(syntax::Span { + scope: Highlight(selection_scope), + start: range.anchor, + end: cursor_start, + }); + if !selection_is_primary || cursor_is_block { + spans.push(syntax::Span { + scope: Highlight(cursor_scope), + start: cursor_start, + end: range.head, + }); + } + } else { + // Reverse case. + let cursor_end = next_grapheme_boundary(text, range.head); + if !selection_is_primary || cursor_is_block { + spans.push(syntax::Span { + scope: Highlight(cursor_scope), + start: range.head, + end: cursor_end, + }); + } + spans.push(syntax::Span { + scope: Highlight(selection_scope), + start: cursor_end, + end: range.anchor, + }); + } + } + + monotonic_overlay(highlights, spans.into_iter()) + } +} + +impl<'a, In> HighlightOverlay for () +where + In: Iterator, +{ + type Out = In; + + fn apply(self, highlights: In) -> Self::Out { + highlights + } +} diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index e02922951698..ccd6b135d7eb 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -1,7 +1,7 @@ use crate::{ compositor::{Component, Compositor, Context, Event, EventResult}, ctrl, key, shift, - ui::{self, editor::DocSyntaxHighlight, EditorView}, + ui::{self, EditorView}, }; use tui::{ buffer::Buffer as Surface, @@ -228,26 +228,14 @@ impl Component for FilePicker { let offset = Position::new(first_line, 0); - match EditorView::doc_syntax_highlights(doc, offset, area.height, &cx.editor.theme) { - DocSyntaxHighlight::Default(highlights) => EditorView::render_text_highlights( - doc, - offset, - inner, - surface, - &cx.editor.theme, - highlights, - &cx.editor.config(), - ), - DocSyntaxHighlight::TreeSitter(highlights) => EditorView::render_text_highlights( - doc, - offset, - inner, - surface, - &cx.editor.theme, - highlights, - &cx.editor.config(), - ), - } + EditorView::render_document( + doc, + inner, + surface, + offset, + &cx.editor.theme, + &*cx.editor.config(), + ); // highlight the line if let Some((start, end)) = range { From c3f652fa5e7850729e47f4d16a364a4d352b0465 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 3 Oct 2022 21:04:04 +0200 Subject: [PATCH 4/9] remove unnecessary close from DiagnosticsOverlay --- helix-term/src/ui/editor/highlights.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/helix-term/src/ui/editor/highlights.rs b/helix-term/src/ui/editor/highlights.rs index 72430ba548f5..c4dcfd8a42e5 100644 --- a/helix-term/src/ui/editor/highlights.rs +++ b/helix-term/src/ui/editor/highlights.rs @@ -76,8 +76,15 @@ where { type Out = syntax::OverlappingOverlay>; fn apply(self, highlights: In) -> Self::Out { - let get_scope_of = |scope| { - self.theme + let scope = match self.severity { + Some(Severity::Error) => "diagnostic.error", + Some(Severity::Warning) => "diagnostic.warning", + Some(Severity::Info) => "diagnostic.info", + Some(Severity::Hint) => "diagnostic.hint", + None => "diagnostic", + }; + + let scope = self.theme .find_scope_index(scope) // get one of the themes below as fallback values .or_else(|| self.theme.find_scope_index("diagnostic")) @@ -85,16 +92,7 @@ where .or_else(|| self.theme.find_scope_index("ui.selection")) .expect( "at least one of the following scopes must be defined in the theme: `diagnostic`, `ui.cursor`, or `ui.selection`", - ) - }; - - let highlight = match self.severity { - Some(Severity::Error) => get_scope_of("diagnostic.error"), - Some(Severity::Warning) => get_scope_of("diagnostic.warning"), - Some(Severity::Info) => get_scope_of("diagnostic.info"), - Some(Severity::Hint) => get_scope_of("diagnostic.hint"), - None => get_scope_of("diagnostic"), - }; + ); overlapping_overlay( highlights, @@ -102,7 +100,7 @@ where diagnostics: self.doc.diagnostics().iter(), severity: self.severity, }, - Highlight(highlight), + Highlight(scope), ) } } From 25427ced840d1b319485cfadc704f350dadb86b8 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 3 Oct 2022 21:22:06 +0200 Subject: [PATCH 5/9] move unwrap from helix-core to helix-term --- helix-core/src/syntax.rs | 8 ++++---- helix-term/src/ui/editor.rs | 12 +++++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 24fd92460476..2514b5f0679b 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -1193,17 +1193,17 @@ struct LocalScope<'a> { pub struct CharacterHighlightIter<'a>(HighlightIter<'a>); impl Iterator for CharacterHighlightIter<'_> { - type Item = HighlightEvent; + type Item = Result; fn next(&mut self) -> Option { - let res = match self.0.next()?.unwrap() { + let res = match self.0.next()? { // TODO: use byte slices directly // convert byte offsets to char offset - HighlightEvent::Source { start, end } => { + Ok(HighlightEvent::Source { start, end }) => { let text = self.0.source; let start = text.byte_to_char(ensure_grapheme_boundary_next_byte(text, start)); let end = text.byte_to_char(ensure_grapheme_boundary_next_byte(text, end)); - HighlightEvent::Source { start, end } + Ok(HighlightEvent::Source { start, end }) } event => event, }; diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 4506f240505b..1a692bac19bb 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -12,7 +12,7 @@ use crate::{ use helix_core::{ diagnostic::Severity, movement::Direction, - syntax::{CharacterHighlightIter, HighlightEvent}, + syntax::{self, CharacterHighlightIter, HighlightEvent}, unicode::width::UnicodeWidthStr, LineEnding, Position, Range, Selection, Transaction, }; @@ -33,6 +33,8 @@ use super::statusline; mod highlights; +type UnwrapResult = fn(Result) -> HighlightEvent; + pub struct EditorView { pub keymaps: Keymaps, on_next_key: Option>, @@ -225,7 +227,7 @@ impl EditorView { config: &helix_view::editor::Config, overlay: O, ) where - O: HighlightOverlay> + O: HighlightOverlay, UnwrapResult>> + HighlightOverlay>, { let height = viewport.height; @@ -249,9 +251,13 @@ impl EditorView { // TODO: range doesn't actually restrict source, just highlight range // TODO: use byte slices directly // convert byte offsets to char offset - let iter = syntax + let iter: CharacterHighlightIter<'doc> = syntax .highlight_iter(text.slice(..), Some(range), None) .to_chars(); + + // we have to help type inference out here (even if turbofish is used) + // this is probably a rustc bug/shortcoming + let iter = iter.map(Result::unwrap as UnwrapResult); Self::render_text_highlights( doc, offset, From 1f26bca234cec27db13e880a731202184ad0d54d Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 3 Oct 2022 21:24:26 +0200 Subject: [PATCH 6/9] move contents of overlay/tests.rs to overlay.rs --- helix-core/src/syntax/overlay.rs | 39 ++++++++++++++++++++++++--- helix-core/src/syntax/overlay/test.rs | 32 ---------------------- 2 files changed, 36 insertions(+), 35 deletions(-) delete mode 100644 helix-core/src/syntax/overlay/test.rs diff --git a/helix-core/src/syntax/overlay.rs b/helix-core/src/syntax/overlay.rs index e975aca58960..780d522e3f60 100644 --- a/helix-core/src/syntax/overlay.rs +++ b/helix-core/src/syntax/overlay.rs @@ -5,9 +5,6 @@ use std::ops::Range; use crate::syntax::{Highlight, HighlightEvent}; use HighlightEvent::*; -#[cfg(test)] -mod test; - pub type MonotonicOverlay = Overlay; /// Overlays multiple different highlights from `spans` onto the `HighlightEvent` stream `events`. @@ -266,3 +263,39 @@ where } } } + +#[cfg(test)] +mod test { + use super::*; + use std::iter; + + /// this tests checks that merging two overlapping ranges onto each other + /// correctly preveres the order of merges. + /// that is the highlight that is merged in last, gets applied last and overwrites the other layers + /// In this test a range of lower priority (like a hint) starts at 2 + /// and another range of a high priority range (like an error) starts earlier + /// with the old span implementation the hint would always overwrite the error. + /// The new implementation (tested here) forces the + #[test] + fn overlay_long_hint() { + let base = iter::once(Source { start: 0, end: 31 }); + let highlights = overlapping_overlay(base, [2..10].into_iter(), Highlight(1)); + let highlights = overlapping_overlay(highlights, [0..4].into_iter(), Highlight(2)); + let res: Vec<_> = highlights.collect(); + assert_eq!( + res, + &[ + HighlightStart(Highlight(2)), + Source { start: 0, end: 2 }, + HighlightEnd, + HighlightStart(Highlight(1)), + HighlightStart(Highlight(2)), + Source { start: 2, end: 4 }, + HighlightEnd, + Source { start: 4, end: 10 }, + HighlightEnd, + Source { start: 10, end: 31 }, + ] + ); + } +} diff --git a/helix-core/src/syntax/overlay/test.rs b/helix-core/src/syntax/overlay/test.rs deleted file mode 100644 index d78999f564ff..000000000000 --- a/helix-core/src/syntax/overlay/test.rs +++ /dev/null @@ -1,32 +0,0 @@ -use crate::syntax::{overlapping_overlay, Highlight, HighlightEvent::*}; -use std::iter; - -/// this tests checks that merging two overlapping ranges onto each other -/// correctly preveres the order of merges. -/// that is the highlight that is merged in last, gets applied last and overwrites the other layers -/// In this test a range of lower priority (like a hint) starts at 2 -/// and another range of a high priority range (like an error) starts earlier -/// with the old span implementation the hint would always overwrite the error. -/// The new implementation (tested here) forces the -#[test] -fn overlay_long_hint() { - let base = iter::once(Source { start: 0, end: 31 }); - let highlights = overlapping_overlay(base, [2..10].into_iter(), Highlight(1)); - let highlights = overlapping_overlay(highlights, [0..4].into_iter(), Highlight(2)); - let res: Vec<_> = highlights.collect(); - assert_eq!( - res, - &[ - HighlightStart(Highlight(2)), - Source { start: 0, end: 2 }, - HighlightEnd, - HighlightStart(Highlight(1)), - HighlightStart(Highlight(2)), - Source { start: 2, end: 4 }, - HighlightEnd, - Source { start: 4, end: 10 }, - HighlightEnd, - Source { start: 10, end: 31 }, - ] - ); -} From 87f1a170eedc26123bedbcb5aeb4e807c39a65a3 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 3 Oct 2022 21:29:44 +0200 Subject: [PATCH 7/9] fixup overlay test documentation --- helix-core/src/syntax/overlay.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helix-core/src/syntax/overlay.rs b/helix-core/src/syntax/overlay.rs index 780d522e3f60..ff52a5215a71 100644 --- a/helix-core/src/syntax/overlay.rs +++ b/helix-core/src/syntax/overlay.rs @@ -275,7 +275,8 @@ mod test { /// In this test a range of lower priority (like a hint) starts at 2 /// and another range of a high priority range (like an error) starts earlier /// with the old span implementation the hint would always overwrite the error. - /// The new implementation (tested here) forces the + /// The new implementation (tested here) ensures that that high pripority diagnostic + /// always overlays the low priority diagnostic #[test] fn overlay_long_hint() { let base = iter::once(Source { start: 0, end: 31 }); From c2bdbbfe4b62a608c11e7fccbd764dd392b2e35b Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 3 Oct 2022 21:44:30 +0200 Subject: [PATCH 8/9] check overlay invariants --- helix-core/src/syntax/overlay.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/helix-core/src/syntax/overlay.rs b/helix-core/src/syntax/overlay.rs index ff52a5215a71..3f0a5661cde0 100644 --- a/helix-core/src/syntax/overlay.rs +++ b/helix-core/src/syntax/overlay.rs @@ -11,7 +11,6 @@ pub type MonotonicOverlay = Overlay; /// /// The [`Span`]s yielded by `spans` **must never overlap** or the iterator will produce incorrect results. /// The [`Span`]s **must be sorted** in ascending order by their start. -/// If multiple [`Span`]s share the same start, the ordering is arbitrary. /// /// Together these two properties mean that `spans` must prduce monotonically increasing [`Span`]s. /// That means that the next span must always start after the last span ended: @@ -62,7 +61,6 @@ pub type OverlappingOverlay = Overlay( events: Events, ranges: Ranges, @@ -226,6 +224,15 @@ where // advance the span as the current one has been fully processed if span.end <= end { self.current_span = self.spans.next(); + if cfg!(debug_assertions) + && matches!(self.current_span, Some(next_span) if next_span.start < span.end) + { + if MERGE { + unreachable!("spans must be sorted in ascending order",); + } else { + unreachable!("spans must be monotonically increasing",); + } + } } let event = if span.end < end { // the span ends before the current source event. @@ -269,14 +276,14 @@ mod test { use super::*; use std::iter; - /// this tests checks that merging two overlapping ranges onto each other + /// This tests checks that merging two overlapping ranges onto each other /// correctly preveres the order of merges. /// that is the highlight that is merged in last, gets applied last and overwrites the other layers /// In this test a range of lower priority (like a hint) starts at 2 /// and another range of a high priority range (like an error) starts earlier /// with the old span implementation the hint would always overwrite the error. /// The new implementation (tested here) ensures that that high pripority diagnostic - /// always overlays the low priority diagnostic + /// always overlays the low priority diagnostic. #[test] fn overlay_long_hint() { let base = iter::once(Source { start: 0, end: 31 }); From 7ed579dea1e1f00e97dde53ca7fc42fe014a542b Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 3 Oct 2022 23:54:31 +0200 Subject: [PATCH 9/9] add missing assert --- helix-core/src/syntax/overlay.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/helix-core/src/syntax/overlay.rs b/helix-core/src/syntax/overlay.rs index 3f0a5661cde0..bd3d9cf1140e 100644 --- a/helix-core/src/syntax/overlay.rs +++ b/helix-core/src/syntax/overlay.rs @@ -192,9 +192,16 @@ where continue; } // skip empty spans and spans that end before this source - while matches!(&self.current_span, Some(span) if span.end <= start || span.start == span.end) - { - self.current_span = self.spans.next(); + while let Some(span) = self.current_span { + if span.end <= start || span.start == span.end { + self.current_span = self.spans.next(); + debug_assert!( + !matches!(self.current_span, Some(next_span) if next_span.start < span.end), + "spans must be sorted in ascending order" + ); + } else { + break; + } } if let Some(span) = &mut self.current_span { @@ -224,15 +231,10 @@ where // advance the span as the current one has been fully processed if span.end <= end { self.current_span = self.spans.next(); - if cfg!(debug_assertions) - && matches!(self.current_span, Some(next_span) if next_span.start < span.end) - { - if MERGE { - unreachable!("spans must be sorted in ascending order",); - } else { - unreachable!("spans must be monotonically increasing",); - } - } + debug_assert!( + !matches!(self.current_span, Some(next_span) if next_span.start < span.end), + "spans must be sorted in ascending order" + ); } let event = if span.end < end { // the span ends before the current source event.