From 7283ef881f2855ef94b67eca88799fa07157df9c Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Fri, 5 Apr 2024 03:17:06 +0200 Subject: [PATCH] only show inline diagnostics after a delay --- helix-term/src/application.rs | 7 ++ helix-term/src/commands.rs | 8 ++ helix-term/src/commands/lsp.rs | 5 +- helix-term/src/events.rs | 3 +- helix-term/src/handlers.rs | 2 + helix-term/src/handlers/diagnostics.rs | 24 ++++ helix-term/src/ui/editor.rs | 9 +- helix-view/src/annotations/diagnostics.rs | 24 ++-- helix-view/src/events.rs | 3 +- helix-view/src/handlers.rs | 1 + helix-view/src/handlers/diagnostics.rs | 131 ++++++++++++++++++++++ helix-view/src/view.rs | 19 +++- 12 files changed, 219 insertions(+), 17 deletions(-) create mode 100644 helix-term/src/handlers/diagnostics.rs create mode 100644 helix-view/src/handlers/diagnostics.rs diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 4c305f6ac..b7123e972 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -11,6 +11,7 @@ use helix_view::{ align_view, document::{DocumentOpenError, DocumentSavedEventResult}, editor::{ConfigEvent, EditorEvent}, + events::DiagnosticsDidChange, graphics::Rect, theme, tree::Layout, @@ -834,6 +835,12 @@ impl Application { &unchanged_diag_sources, Some(server_id), ); + + let doc = doc.id(); + helix_event::dispatch(DiagnosticsDidChange { + editor: &mut self.editor, + doc, + }); } } Notification::ShowMessage(params) => { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index d205f234c..234902886 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3550,6 +3550,8 @@ fn goto_first_diag(cx: &mut Context) { None => return, }; doc.set_selection(view.id, selection); + view.diagnostics_handler + .immediately_show_diagnostic(doc, view.id); } fn goto_last_diag(cx: &mut Context) { @@ -3559,6 +3561,8 @@ fn goto_last_diag(cx: &mut Context) { None => return, }; doc.set_selection(view.id, selection); + view.diagnostics_handler + .immediately_show_diagnostic(doc, view.id); } fn goto_next_diag(cx: &mut Context) { @@ -3581,6 +3585,8 @@ fn goto_next_diag(cx: &mut Context) { None => return, }; doc.set_selection(view.id, selection); + view.diagnostics_handler + .immediately_show_diagnostic(doc, view.id); }; cx.editor.apply_motion(motion); @@ -3609,6 +3615,8 @@ fn goto_prev_diag(cx: &mut Context) { None => return, }; doc.set_selection(view.id, selection); + view.diagnostics_handler + .immediately_show_diagnostic(doc, view.id); }; cx.editor.apply_motion(motion) } diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 103d1df24..3b9efb431 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -271,7 +271,10 @@ fn diag_picker( let Some(path) = uri.as_path() else { return; }; - jump_to_position(cx.editor, path, diag.range, *offset_encoding, action) + jump_to_position(cx.editor, path, diag.range, *offset_encoding, action); + let (view, doc) = current!(cx.editor); + view.diagnostics_handler + .immediately_show_diagnostic(doc, view.id); }, ) .with_preview(move |_editor, PickerDiagnostic { uri, diag, .. }| { diff --git a/helix-term/src/events.rs b/helix-term/src/events.rs index 49b44f775..415213c22 100644 --- a/helix-term/src/events.rs +++ b/helix-term/src/events.rs @@ -1,6 +1,6 @@ use helix_event::{events, register_event}; use helix_view::document::Mode; -use helix_view::events::{DocumentDidChange, SelectionDidChange}; +use helix_view::events::{DiagnosticsDidChange, DocumentDidChange, SelectionDidChange}; use crate::commands; use crate::keymap::MappableCommand; @@ -17,4 +17,5 @@ pub fn register() { register_event::(); register_event::(); register_event::(); + register_event::(); } diff --git a/helix-term/src/handlers.rs b/helix-term/src/handlers.rs index fc9273137..b27e34e29 100644 --- a/helix-term/src/handlers.rs +++ b/helix-term/src/handlers.rs @@ -14,6 +14,7 @@ pub use helix_view::handlers::Handlers; mod auto_save; pub mod completion; +mod diagnostics; mod signature_help; pub fn setup(config: Arc>) -> Handlers { @@ -32,5 +33,6 @@ pub fn setup(config: Arc>) -> Handlers { completion::register_hooks(&handlers); signature_help::register_hooks(&handlers); auto_save::register_hooks(&handlers); + diagnostics::register_hooks(&handlers); handlers } diff --git a/helix-term/src/handlers/diagnostics.rs b/helix-term/src/handlers/diagnostics.rs new file mode 100644 index 000000000..3e44d416d --- /dev/null +++ b/helix-term/src/handlers/diagnostics.rs @@ -0,0 +1,24 @@ +use helix_event::{register_hook, send_blocking}; +use helix_view::document::Mode; +use helix_view::events::DiagnosticsDidChange; +use helix_view::handlers::diagnostics::DiagnosticEvent; +use helix_view::handlers::Handlers; + +use crate::events::OnModeSwitch; + +pub(super) fn register_hooks(_handlers: &Handlers) { + register_hook!(move |event: &mut DiagnosticsDidChange<'_>| { + if event.editor.mode != Mode::Insert { + for (view, _) in event.editor.tree.views_mut() { + send_blocking(&view.diagnostics_handler.events, DiagnosticEvent::Refresh) + } + } + Ok(()) + }); + register_hook!(move |event: &mut OnModeSwitch<'_, '_>| { + for (view, _) in event.cx.editor.tree.views_mut() { + view.diagnostics_handler.active = event.new_mode != Mode::Insert; + } + Ok(()) + }); +} diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index e934659cd..c151a7dd5 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -186,11 +186,18 @@ impl EditorView { primary_cursor, }); } + let width = view.inner_width(doc); + let config = doc.config.load(); + let enable_cursor_line = view + .diagnostics_handler + .show_cursorline_diagnostics(doc, view.id); + let inline_diagnostic_config = config.inline_diagnostics.prepare(width, enable_cursor_line); decorations.add_decoration(InlineDiagnostics::new( doc, theme, primary_cursor, - config.lsp.inline_diagnostics.clone(), + inline_diagnostic_config, + config.end_of_line_diagnostics, )); render_document( surface, diff --git a/helix-view/src/annotations/diagnostics.rs b/helix-view/src/annotations/diagnostics.rs index afe0685a5..09085d3fe 100644 --- a/helix-view/src/annotations/diagnostics.rs +++ b/helix-view/src/annotations/diagnostics.rs @@ -60,22 +60,26 @@ pub struct InlineDiagnosticsConfig { } impl InlineDiagnosticsConfig { - // last column where to start diagnostics - // every diagnostics that start afterwards will be displayed with a "backwards - // line" to ensure they are still rendered with `min_diagnostic_widht`. If `width` - // it too small to display diagnostics with atleast `min_diagnostic_width` space - // (or inline diagnostics are displed) `None` is returned. In that case inline - // diagnostics should not be shown - pub fn enable(&self, width: u16) -> bool { - let disabled = matches!( + pub fn disabled(&self) -> bool { + matches!( self, Self { cursor_line: DiagnosticFilter::Disable, other_lines: DiagnosticFilter::Disable, .. } - ); - !disabled && width >= self.min_diagnostic_width + self.prefix_len + ) + } + + pub fn prepare(&self, width: u16, enable_cursor_line: bool) -> Self { + let mut config = self.clone(); + if width < self.min_diagnostic_width + self.prefix_len { + config.cursor_line = DiagnosticFilter::Disable; + config.other_lines = DiagnosticFilter::Disable; + } else if !enable_cursor_line { + config.cursor_line = self.cursor_line.min(self.other_lines); + } + config } pub fn max_diagnostic_start(&self, width: u16) -> u16 { diff --git a/helix-view/src/events.rs b/helix-view/src/events.rs index 8b789cc0d..881412495 100644 --- a/helix-view/src/events.rs +++ b/helix-view/src/events.rs @@ -1,9 +1,10 @@ use helix_core::Rope; use helix_event::events; -use crate::{Document, ViewId}; +use crate::{Document, DocumentId, Editor, ViewId}; events! { DocumentDidChange<'a> { doc: &'a mut Document, view: ViewId, old_text: &'a Rope } SelectionDidChange<'a> { doc: &'a mut Document, view: ViewId } + DiagnosticsDidChange<'a> { editor: &'a mut Editor, doc: DocumentId } } diff --git a/helix-view/src/handlers.rs b/helix-view/src/handlers.rs index e2848f264..93336beb5 100644 --- a/helix-view/src/handlers.rs +++ b/helix-view/src/handlers.rs @@ -5,6 +5,7 @@ use crate::handlers::lsp::SignatureHelpInvoked; use crate::{DocumentId, Editor, ViewId}; pub mod dap; +pub mod diagnostics; pub mod lsp; #[derive(Debug)] diff --git a/helix-view/src/handlers/diagnostics.rs b/helix-view/src/handlers/diagnostics.rs new file mode 100644 index 000000000..2b8ff6325 --- /dev/null +++ b/helix-view/src/handlers/diagnostics.rs @@ -0,0 +1,131 @@ +use std::cell::Cell; +use std::num::NonZeroUsize; +use std::sync::atomic::{self, AtomicUsize}; +use std::sync::Arc; +use std::time::Duration; + +use helix_event::{request_redraw, send_blocking, AsyncHook}; +use tokio::sync::mpsc::Sender; +use tokio::time::Instant; + +use crate::{Document, DocumentId, ViewId}; + +#[derive(Debug)] +pub enum DiagnosticEvent { + CursorLineChanged { generation: usize }, + Refresh, +} + +struct DiagnosticTimeout { + active_generation: Arc, + generation: usize, +} + +const TIMEOUT: Duration = Duration::from_millis(350); + +impl AsyncHook for DiagnosticTimeout { + type Event = DiagnosticEvent; + + fn handle_event( + &mut self, + event: DiagnosticEvent, + timeout: Option, + ) -> Option { + match event { + DiagnosticEvent::CursorLineChanged { generation } => { + if generation > self.generation { + self.generation = generation; + Some(Instant::now() + TIMEOUT) + } else { + timeout + } + } + DiagnosticEvent::Refresh if timeout.is_some() => Some(Instant::now() + TIMEOUT), + DiagnosticEvent::Refresh => None, + } + } + + fn finish_debounce(&mut self) { + if self.active_generation.load(atomic::Ordering::Relaxed) < self.generation { + self.active_generation + .store(self.generation, atomic::Ordering::Relaxed); + request_redraw(); + } + } +} + +pub struct DiagnosticsHandler { + active_generation: Arc, + generation: Cell, + last_doc: Cell, + last_cursor_line: Cell, + pub active: bool, + pub events: Sender, +} + +// make sure we never share handlers across multiple views this is a stop +// gap solution. We just shouldn't be cloneing a view to begin with (we do +// for :hsplit/vsplit) and really this should not be view specific to begin with +// but to fix that larger architecutre changes are needed +impl Clone for DiagnosticsHandler { + fn clone(&self) -> Self { + Self::new() + } +} + +impl DiagnosticsHandler { + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + let active_generation = Arc::new(AtomicUsize::new(0)); + let events = DiagnosticTimeout { + active_generation: active_generation.clone(), + generation: 0, + } + .spawn(); + Self { + active_generation, + generation: Cell::new(0), + events, + last_doc: Cell::new(DocumentId(NonZeroUsize::new(usize::MAX).unwrap())), + last_cursor_line: Cell::new(usize::MAX), + active: true, + } + } +} + +impl DiagnosticsHandler { + pub fn immediately_show_diagnostic(&self, doc: &Document, view: ViewId) { + self.last_doc.set(doc.id()); + let cursor_line = doc + .selection(view) + .primary() + .cursor_line(doc.text().slice(..)); + self.last_cursor_line.set(cursor_line); + self.active_generation + .store(self.generation.get(), atomic::Ordering::Relaxed); + } + pub fn show_cursorline_diagnostics(&self, doc: &Document, view: ViewId) -> bool { + if !self.active { + return false; + } + let cursor_line = doc + .selection(view) + .primary() + .cursor_line(doc.text().slice(..)); + if self.last_cursor_line.get() == cursor_line && self.last_doc.get() == doc.id() { + let active_generation = self.active_generation.load(atomic::Ordering::Relaxed); + self.generation.get() == active_generation + } else { + self.last_doc.set(doc.id()); + self.last_cursor_line.set(cursor_line); + self.generation.set(self.generation.get() + 1); + send_blocking( + &self.events, + DiagnosticEvent::CursorLineChanged { + generation: self.generation.get(), + }, + ); + false + } + } +} diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index a5654759e..af4fdfe4e 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -4,6 +4,7 @@ use crate::{ document::DocumentInlayHints, editor::{GutterConfig, GutterType}, graphics::Rect, + handlers::diagnostics::DiagnosticsHandler, Align, Document, DocumentId, Theme, ViewId, }; @@ -147,6 +148,14 @@ pub struct View { /// mapping keeps track of the last applied history revision so that only new changes /// are applied. doc_revisions: HashMap, + // HACKS: there should really only be a global diagnostics handler (the + // non-focused views should just not have different handling for the cursor + // line). For that we would need accces to editor everywhere (we want to use + // the positioning code) so this can only happen by refactoring View and + // Document into entity component like structure. That is a huge refactor + // left to future work. For now we treat all views as focused and give them + // each their own handler. + pub diagnostics_handler: DiagnosticsHandler, } impl fmt::Debug for View { @@ -176,6 +185,7 @@ impl View { object_selections: Vec::new(), gutters, doc_revisions: HashMap::new(), + diagnostics_handler: DiagnosticsHandler::new(), } } @@ -463,10 +473,13 @@ impl View { .add_inline_annotations(other_inlay_hints, other_style) .add_inline_annotations(padding_after_inlay_hints, None); }; - let width = self.inner_width(doc); let config = doc.config.load(); - if config.lsp.inline_diagnostics.enable(width) { - let config = config.lsp.inline_diagnostics.clone(); + let width = self.inner_width(doc); + let enable_cursor_line = self + .diagnostics_handler + .show_cursorline_diagnostics(doc, self.id); + let config = config.inline_diagnostics.prepare(width, enable_cursor_line); + if !config.disabled() { let cursor = doc .selection(self.id) .primary()