Skip to content

Commit

Permalink
only show inline diagnostics after a delay
Browse files Browse the repository at this point in the history
  • Loading branch information
pascalkuthe committed Jul 15, 2024
1 parent d8a1156 commit 7283ef8
Show file tree
Hide file tree
Showing 12 changed files with 219 additions and 17 deletions.
7 changes: 7 additions & 0 deletions helix-term/src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use helix_view::{
align_view,
document::{DocumentOpenError, DocumentSavedEventResult},
editor::{ConfigEvent, EditorEvent},
events::DiagnosticsDidChange,
graphics::Rect,
theme,
tree::Layout,
Expand Down Expand Up @@ -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) => {
Expand Down
8 changes: 8 additions & 0 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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)
}
Expand Down
5 changes: 4 additions & 1 deletion helix-term/src/commands/lsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, .. }| {
Expand Down
3 changes: 2 additions & 1 deletion helix-term/src/events.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -17,4 +17,5 @@ pub fn register() {
register_event::<PostCommand>();
register_event::<DocumentDidChange>();
register_event::<SelectionDidChange>();
register_event::<DiagnosticsDidChange>();
}
2 changes: 2 additions & 0 deletions helix-term/src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ArcSwap<Config>>) -> Handlers {
Expand All @@ -32,5 +33,6 @@ pub fn setup(config: Arc<ArcSwap<Config>>) -> Handlers {
completion::register_hooks(&handlers);
signature_help::register_hooks(&handlers);
auto_save::register_hooks(&handlers);
diagnostics::register_hooks(&handlers);
handlers
}
24 changes: 24 additions & 0 deletions helix-term/src/handlers/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -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(())
});
}
9 changes: 8 additions & 1 deletion helix-term/src/ui/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
24 changes: 14 additions & 10 deletions helix-view/src/annotations/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion helix-view/src/events.rs
Original file line number Diff line number Diff line change
@@ -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 }
}
1 change: 1 addition & 0 deletions helix-view/src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
131 changes: 131 additions & 0 deletions helix-view/src/handlers/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -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<AtomicUsize>,
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<Instant>,
) -> Option<Instant> {
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<AtomicUsize>,
generation: Cell<usize>,
last_doc: Cell<DocumentId>,
last_cursor_line: Cell<usize>,
pub active: bool,
pub events: Sender<DiagnosticEvent>,
}

// 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
}
}
}
19 changes: 16 additions & 3 deletions helix-view/src/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::{
document::DocumentInlayHints,
editor::{GutterConfig, GutterType},
graphics::Rect,
handlers::diagnostics::DiagnosticsHandler,
Align, Document, DocumentId, Theme, ViewId,
};

Expand Down Expand Up @@ -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<DocumentId, usize>,
// 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 {
Expand Down Expand Up @@ -176,6 +185,7 @@ impl View {
object_selections: Vec::new(),
gutters,
doc_revisions: HashMap::new(),
diagnostics_handler: DiagnosticsHandler::new(),
}
}

Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 7283ef8

Please sign in to comment.