bendns' repos / helix
Unnamed repository; edit this file 'description' to name the repository.
Refactor DiagnosticProvider as an enum
This resolves a TODO in the core diagnostic module to refactor this type. It was originally an alias of `LanguageServerId` for simplicity. Refactoring as an enum is a necessary step towards introducing "internal" diagnostics - diagnostics emitted by core features such as a spell checker. Fully supporting this use-case will require further larger changes to the diagnostic type, but the change to the provider can be made first. Note that `Copy` is not derived for `DiagnosticProvider` (as it was previously because `LanguageServerId` is `Copy`). In the child commits we will add the `identifier` used in LSP pull diagnostics which is a string - not `Copy`.
Browse Source
Michael Davis 11 months ago
parent 2d4c2a1 · commit 683fac6
Diffstat
-rw-r--r--helix-core/src/diagnostic.rs16
-rw-r--r--helix-term/src/application.rs10
-rw-r--r--helix-term/src/commands/lsp.rs26
-rw-r--r--helix-term/src/commands/typed.rs2
-rw-r--r--helix-view/src/document.rs43
-rw-r--r--helix-view/src/editor.rs22
-rw-r--r--helix-view/src/gutter.rs7
-rw-r--r--helix-view/src/handlers/lsp.rs19
8 files changed, 87 insertions, 58 deletions
diff --git a/helix-core/src/diagnostic.rs b/helix-core/src/diagnostic.rs
index 333c9409..ba7df1b0 100644
--- a/helix-core/src/diagnostic.rs
+++ b/helix-core/src/diagnostic.rs
@@ -50,8 +50,20 @@ pub struct Diagnostic {
pub data: Option<serde_json::Value>,
}
-// TODO turn this into an enum + feature flag when lsp becomes optional
-pub type DiagnosticProvider = LanguageServerId;
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub enum DiagnosticProvider {
+ Lsp { server_id: LanguageServerId },
+ // Future internal features can go here...
+}
+
+impl DiagnosticProvider {
+ pub fn language_server_id(&self) -> Option<LanguageServerId> {
+ match self {
+ Self::Lsp { server_id, .. } => Some(*server_id),
+ // _ => None,
+ }
+ }
+}
// while I would prefer having this in helix-lsp that necessitates a bunch of
// conversions I would rather not add. I think its fine since this just a very
diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs
index cb270b86..afcb1a10 100644
--- a/helix-term/src/application.rs
+++ b/helix-term/src/application.rs
@@ -740,8 +740,10 @@ impl Application {
log::error!("Discarding publishDiagnostic notification sent by an uninitialized server: {}", language_server.name());
return;
}
+ let provider =
+ helix_core::diagnostic::DiagnosticProvider::Lsp { server_id };
self.editor.handle_lsp_diagnostics(
- language_server.id(),
+ &provider,
uri,
params.version,
params.diagnostics,
@@ -854,14 +856,16 @@ impl Application {
// we need to clear those and remove the entries from the list if this leads to
// an empty diagnostic list for said files
for diags in self.editor.diagnostics.values_mut() {
- diags.retain(|(_, lsp_id)| *lsp_id != server_id);
+ diags.retain(|(_, provider)| {
+ provider.language_server_id() != Some(server_id)
+ });
}
self.editor.diagnostics.retain(|_, diags| !diags.is_empty());
// Clear any diagnostics for documents with this server open.
for doc in self.editor.documents_mut() {
- doc.clear_diagnostics(Some(server_id));
+ doc.clear_diagnostics_for_language_server(server_id);
}
// Remove the language server from the registry.
diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs
index cd041acd..6d6b9744 100644
--- a/helix-term/src/commands/lsp.rs
+++ b/helix-term/src/commands/lsp.rs
@@ -14,7 +14,8 @@ use tui::{text::Span, widgets::Row};
use super::{align_view, push_jump, Align, Context, Editor};
use helix_core::{
- syntax::LanguageServerFeature, text_annotations::InlineAnnotation, Selection, Uri,
+ diagnostic::DiagnosticProvider, syntax::LanguageServerFeature,
+ text_annotations::InlineAnnotation, Selection, Uri,
};
use helix_stdx::path;
use helix_view::{
@@ -31,13 +32,7 @@ use crate::{
ui::{self, overlay::overlaid, FileLocation, Picker, Popup, PromptEvent},
};
-use std::{
- cmp::Ordering,
- collections::{BTreeMap, HashSet},
- fmt::Display,
- future::Future,
- path::Path,
-};
+use std::{cmp::Ordering, collections::HashSet, fmt::Display, future::Future, path::Path};
/// Gets the first language server that is attached to a document which supports a specific feature.
/// If there is no configured language server that supports the feature, this displays a status message.
@@ -209,7 +204,7 @@ type DiagnosticsPicker = Picker<PickerDiagnostic, DiagnosticStyles>;
fn diag_picker(
cx: &Context,
- diagnostics: BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+ diagnostics: impl IntoIterator<Item = (Uri, Vec<(lsp::Diagnostic, DiagnosticProvider)>)>,
format: DiagnosticsFormat,
) -> DiagnosticsPicker {
// TODO: drop current_path comparison and instead use workspace: bool flag?
@@ -219,8 +214,11 @@ fn diag_picker(
for (uri, diags) in diagnostics {
flat_diag.reserve(diags.len());
- for (diag, ls) in diags {
- if let Some(ls) = cx.editor.language_server_by_id(ls) {
+ for (diag, provider) in diags {
+ if let Some(ls) = provider
+ .language_server_id()
+ .and_then(|id| cx.editor.language_server_by_id(id))
+ {
flat_diag.push(PickerDiagnostic {
location: Location {
uri: uri.clone(),
@@ -560,11 +558,7 @@ pub fn diagnostics_picker(cx: &mut Context) {
let doc = doc!(cx.editor);
if let Some(uri) = doc.uri() {
let diagnostics = cx.editor.diagnostics.get(&uri).cloned().unwrap_or_default();
- let picker = diag_picker(
- cx,
- [(uri, diagnostics)].into(),
- DiagnosticsFormat::HideSourcePath,
- );
+ let picker = diag_picker(cx, [(uri, diagnostics)], DiagnosticsFormat::HideSourcePath);
cx.push_layer(Box::new(overlaid(picker)));
}
}
diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs
index 4317993c..e1c09a04 100644
--- a/helix-term/src/commands/typed.rs
+++ b/helix-term/src/commands/typed.rs
@@ -1622,7 +1622,7 @@ fn lsp_stop(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> any
for doc in cx.editor.documents_mut() {
if let Some(client) = doc.remove_language_server_by_name(ls_name) {
- doc.clear_diagnostics(Some(client.id()));
+ doc.clear_diagnostics_for_language_server(client.id());
doc.reset_all_inlay_hints();
doc.inlay_hints_oudated = true;
}
diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs
index ee5cb468..f74f15c6 100644
--- a/helix-view/src/document.rs
+++ b/helix-view/src/document.rs
@@ -5,6 +5,7 @@ use futures_util::future::BoxFuture;
use futures_util::FutureExt;
use helix_core::auto_pairs::AutoPairs;
use helix_core::chars::char_is_word;
+use helix_core::diagnostic::DiagnosticProvider;
use helix_core::doc_formatter::TextFormat;
use helix_core::encoding::Encoding;
use helix_core::snippets::{ActiveSnippet, SnippetRenderCtx};
@@ -1433,8 +1434,13 @@ impl Document {
true
});
- self.diagnostics
- .sort_by_key(|diagnostic| (diagnostic.range, diagnostic.severity, diagnostic.provider));
+ self.diagnostics.sort_by_key(|diagnostic| {
+ (
+ diagnostic.range,
+ diagnostic.severity,
+ diagnostic.provider.clone(),
+ )
+ });
// Update the inlay hint annotations' positions, helping ensure they are displayed in the proper place
let apply_inlay_hint_changes = |annotations: &mut Vec<InlineAnnotation>| {
@@ -1980,7 +1986,7 @@ impl Document {
text: &Rope,
language_config: Option<&LanguageConfiguration>,
diagnostic: &helix_lsp::lsp::Diagnostic,
- language_server_id: LanguageServerId,
+ provider: DiagnosticProvider,
offset_encoding: helix_lsp::OffsetEncoding,
) -> Option<Diagnostic> {
use helix_core::diagnostic::{Range, Severity::*};
@@ -2060,7 +2066,7 @@ impl Document {
tags,
source: diagnostic.source.clone(),
data: diagnostic.data.clone(),
- provider: language_server_id,
+ provider,
})
}
@@ -2073,13 +2079,18 @@ impl Document {
&mut self,
diagnostics: impl IntoIterator<Item = Diagnostic>,
unchanged_sources: &[String],
- language_server_id: Option<LanguageServerId>,
+ provider: Option<&DiagnosticProvider>,
) {
if unchanged_sources.is_empty() {
- self.clear_diagnostics(language_server_id);
+ if let Some(provider) = provider {
+ self.diagnostics
+ .retain(|diagnostic| &diagnostic.provider != provider);
+ } else {
+ self.diagnostics.clear();
+ }
} else {
self.diagnostics.retain(|d| {
- if language_server_id.is_some_and(|id| id != d.provider) {
+ if provider.is_some_and(|provider| provider != &d.provider) {
return true;
}
@@ -2091,17 +2102,19 @@ impl Document {
});
}
self.diagnostics.extend(diagnostics);
- self.diagnostics
- .sort_by_key(|diagnostic| (diagnostic.range, diagnostic.severity, diagnostic.provider));
+ self.diagnostics.sort_by_key(|diagnostic| {
+ (
+ diagnostic.range,
+ diagnostic.severity,
+ diagnostic.provider.clone(),
+ )
+ });
}
/// clears diagnostics for a given language server id if set, otherwise all diagnostics are cleared
- pub fn clear_diagnostics(&mut self, language_server_id: Option<LanguageServerId>) {
- if let Some(id) = language_server_id {
- self.diagnostics.retain(|d| d.provider != id);
- } else {
- self.diagnostics.clear();
- }
+ pub fn clear_diagnostics_for_language_server(&mut self, id: LanguageServerId) {
+ self.diagnostics
+ .retain(|d| d.provider.language_server_id() != Some(id));
}
/// Get the document's auto pairs. If the document has a recognized
diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs
index e6a585d6..31373630 100644
--- a/helix-view/src/editor.rs
+++ b/helix-view/src/editor.rs
@@ -45,6 +45,7 @@ use anyhow::{anyhow, bail, Error};
pub use helix_core::diagnostic::Severity;
use helix_core::{
auto_pairs::AutoPairs,
+ diagnostic::DiagnosticProvider,
syntax::{self, AutoPairConfig, IndentationHeuristic, LanguageServerFeature, SoftWrap},
Change, LineEnding, Position, Range, Selection, Uri, NATIVE_LINE_ENDING,
};
@@ -1041,6 +1042,8 @@ pub struct Breakpoint {
use futures_util::stream::{Flatten, Once};
+type Diagnostics = BTreeMap<Uri, Vec<(lsp::Diagnostic, DiagnosticProvider)>>;
+
pub struct Editor {
/// Current editing mode.
pub mode: Mode,
@@ -1060,7 +1063,7 @@ pub struct Editor {
pub macro_recording: Option<(char, Vec<KeyEvent>)>,
pub macro_replaying: Vec<char>,
pub language_servers: helix_lsp::Registry,
- pub diagnostics: BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+ pub diagnostics: Diagnostics,
pub diff_providers: DiffProviderRegistry,
pub debugger: Option<dap::Client>,
@@ -1207,7 +1210,7 @@ impl Editor {
macro_replaying: Vec::new(),
theme: theme_loader.default(),
language_servers,
- diagnostics: BTreeMap::new(),
+ diagnostics: Diagnostics::new(),
diff_providers: DiffProviderRegistry::default(),
debugger: None,
debugger_events: SelectAll::new(),
@@ -2007,7 +2010,7 @@ impl Editor {
/// Returns all supported diagnostics for the document
pub fn doc_diagnostics<'a>(
language_servers: &'a helix_lsp::Registry,
- diagnostics: &'a BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+ diagnostics: &'a Diagnostics,
document: &Document,
) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
Editor::doc_diagnostics_with_filter(language_servers, diagnostics, document, |_, _| true)
@@ -2017,9 +2020,9 @@ impl Editor {
/// filtered by `filter` which is invocated with the raw `lsp::Diagnostic` and the language server id it came from
pub fn doc_diagnostics_with_filter<'a>(
language_servers: &'a helix_lsp::Registry,
- diagnostics: &'a BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+ diagnostics: &'a Diagnostics,
document: &Document,
- filter: impl Fn(&lsp::Diagnostic, LanguageServerId) -> bool + 'a,
+ filter: impl Fn(&lsp::Diagnostic, &DiagnosticProvider) -> bool + 'a,
) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
let text = document.text().clone();
let language_config = document.language.clone();
@@ -2027,8 +2030,9 @@ impl Editor {
.uri()
.and_then(|uri| diagnostics.get(&uri))
.map(|diags| {
- diags.iter().filter_map(move |(diagnostic, lsp_id)| {
- let ls = language_servers.get_by_id(*lsp_id)?;
+ diags.iter().filter_map(move |(diagnostic, provider)| {
+ let server_id = provider.language_server_id()?;
+ let ls = language_servers.get_by_id(server_id)?;
language_config
.as_ref()
.and_then(|c| {
@@ -2038,12 +2042,12 @@ impl Editor {
})
})
.and_then(|_| {
- if filter(diagnostic, *lsp_id) {
+ if filter(diagnostic, provider) {
Document::lsp_diagnostic_to_diagnostic(
&text,
language_config.as_deref(),
diagnostic,
- *lsp_id,
+ provider.clone(),
ls.offset_encoding(),
)
} else {
diff --git a/helix-view/src/gutter.rs b/helix-view/src/gutter.rs
index 7cd91271..665a78bc 100644
--- a/helix-view/src/gutter.rs
+++ b/helix-view/src/gutter.rs
@@ -69,9 +69,10 @@ pub fn diagnostic<'doc>(
.iter()
.take_while(|d| {
d.line == line
- && doc
- .language_servers_with_feature(LanguageServerFeature::Diagnostics)
- .any(|ls| ls.id() == d.provider)
+ && d.provider.language_server_id().map_or(true, |id| {
+ doc.language_servers_with_feature(LanguageServerFeature::Diagnostics)
+ .any(|ls| ls.id() == id)
+ })
});
diagnostics_on_line.max_by_key(|d| d.severity).map(|d| {
write!(out, "●").ok();
diff --git a/helix-view/src/handlers/lsp.rs b/helix-view/src/handlers/lsp.rs
index bf541929..14e37c15 100644
--- a/helix-view/src/handlers/lsp.rs
+++ b/helix-view/src/handlers/lsp.rs
@@ -4,6 +4,7 @@ use std::fmt::Display;
use crate::editor::Action;
use crate::events::DiagnosticsDidChange;
use crate::Editor;
+use helix_core::diagnostic::DiagnosticProvider;
use helix_core::Uri;
use helix_lsp::util::generate_transaction_from_edits;
use helix_lsp::{lsp, LanguageServerId, OffsetEncoding};
@@ -276,7 +277,7 @@ impl Editor {
pub fn handle_lsp_diagnostics(
&mut self,
- server_id: LanguageServerId,
+ provider: &DiagnosticProvider,
uri: Uri,
version: Option<i32>,
mut diagnostics: Vec<lsp::Diagnostic>,
@@ -309,8 +310,8 @@ impl Editor {
.filter(|d| d.source.as_ref() == Some(source));
let old_diagnostics = old_diagnostics
.iter()
- .filter(|(d, d_server)| {
- *d_server == server_id && d.source.as_ref() == Some(source)
+ .filter(|(d, d_provider)| {
+ d_provider == provider && d.source.as_ref() == Some(source)
})
.map(|(d, _)| d);
if new_diagnostics.eq(old_diagnostics) {
@@ -319,7 +320,7 @@ impl Editor {
}
}
- let diagnostics = diagnostics.into_iter().map(|d| (d, server_id));
+ let diagnostics = diagnostics.into_iter().map(|d| (d, provider.clone()));
// Insert the original lsp::Diagnostics here because we may have no open document
// for diagnostic message and so we can't calculate the exact position.
@@ -328,7 +329,7 @@ impl Editor {
Entry::Occupied(o) => {
let current_diagnostics = o.into_mut();
// there may entries of other language servers, which is why we can't overwrite the whole entry
- current_diagnostics.retain(|(_, lsp_id)| *lsp_id != server_id);
+ current_diagnostics.retain(|(_, d_provider)| d_provider != provider);
current_diagnostics.extend(diagnostics);
current_diagnostics
// Sort diagnostics first by severity and then by line numbers.
@@ -338,12 +339,12 @@ impl Editor {
// Sort diagnostics first by severity and then by line numbers.
// Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order
- diagnostics.sort_by_key(|(d, server_id)| (d.severity, d.range.start, *server_id));
+ diagnostics.sort_by_key(|(d, provider)| (d.severity, d.range.start, provider.clone()));
if let Some(doc) = doc {
let diagnostic_of_language_server_and_not_in_unchanged_sources =
- |diagnostic: &lsp::Diagnostic, ls_id| {
- ls_id == server_id
+ |diagnostic: &lsp::Diagnostic, d_provider: &DiagnosticProvider| {
+ d_provider == provider
&& diagnostic
.source
.as_ref()
@@ -355,7 +356,7 @@ impl Editor {
doc,
diagnostic_of_language_server_and_not_in_unchanged_sources,
);
- doc.replace_diagnostics(diagnostics, &unchanged_diag_sources, Some(server_id));
+ doc.replace_diagnostics(diagnostics, &unchanged_diag_sources, Some(provider));
let doc = doc.id();
helix_event::dispatch(DiagnosticsDidChange { editor: self, doc });