Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support textDocument/diagnostic specification (Pull diagnostics) #11315

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
58447da
Support pull diagnostics
SofusA Jul 20, 2024
fb56f6a
Remove redundant checks
SofusA Jul 20, 2024
8084a2c
Remove unused function
SofusA Jul 20, 2024
c9074d1
Merge branch 'helix-editor:master' into pull-diagnostics
SofusA Jul 23, 2024
120e93e
Merge branch 'helix-editor:master' into pull-diagnostics
SofusA Jul 24, 2024
e0fc9c0
Parse unchanged sources
SofusA Jul 24, 2024
33860d7
Merge branch 'helix-editor:master' into pull-diagnostics
SofusA Jul 25, 2024
bbf6d19
Merge branch 'pull-diagnostics' of https://github.com/SofusA/helix-pu…
SofusA Jul 25, 2024
6541153
Remove comments
SofusA Jul 25, 2024
009f414
Fix lint issue
SofusA Jul 26, 2024
d8fd204
Merge branch 'master' into pull-diagnostics
SofusA Aug 3, 2024
dcd4d45
Merge branch 'pull-diagnostics' of https://github.com/SofusA/helix in…
SofusA Aug 3, 2024
13f048b
Refactor and share code with publish diagnostics
SofusA Aug 3, 2024
29c5318
Iterate over language servers
SofusA Aug 4, 2024
1e446d5
Undo changes to toolchain
SofusA Aug 4, 2024
3a56e48
Use async hook instead
SofusA Aug 6, 2024
31c3794
pass document_id and fix copy paste error
SofusA Aug 12, 2024
2fd8a98
Refactoring
SofusA Aug 12, 2024
4e85776
Merge branch 'helix-editor:master' into pull-diagnostics
SofusA Aug 12, 2024
96a984c
Check all documents
SofusA Aug 13, 2024
5a13b17
Pull diagnostics on open
SofusA Aug 14, 2024
44c5453
Diagnostics for open document as blocking event
SofusA Aug 17, 2024
dcfceb5
Dispatch DocumentDidOpen on ls initialization
SofusA Aug 18, 2024
760c5a8
Pull diagnostics on go to buffer instead
SofusA Aug 19, 2024
afabbc9
Support workspace diagnostic refresh
SofusA Aug 20, 2024
ca0d841
minor improvement to get capability
SofusA Aug 22, 2024
7b9efd8
Merge branch 'helix-editor:master' into pull-diagnostics
SofusA Aug 26, 2024
daede03
workspace diagnostic refresh as request
SofusA Sep 1, 2024
1315749
Refactor add_diagnostics
SofusA Sep 1, 2024
131f0b3
Handle multiple documents in pull diagnostics event
SofusA Sep 1, 2024
97fa306
Merge branch 'helix-editor:master' into pull-diagnostics
SofusA Sep 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions helix-core/src/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ pub enum LanguageServerFeature {
WorkspaceSymbols,
// Symbols, use bitflags, see above?
Diagnostics,
PullDiagnostics,
RenameSymbol,
InlayHints,
}
Expand All @@ -352,6 +353,7 @@ impl Display for LanguageServerFeature {
DocumentSymbols => "document-symbols",
WorkspaceSymbols => "workspace-symbols",
Diagnostics => "diagnostics",
PullDiagnostics => "pull-diagnostics",
RenameSymbol => "rename-symbol",
InlayHints => "inlay-hints",
};
Expand Down
34 changes: 34 additions & 0 deletions helix-lsp/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ impl Client {
Some(OneOf::Left(true) | OneOf::Right(_))
),
LanguageServerFeature::Diagnostics => true, // there's no extra server capability
LanguageServerFeature::PullDiagnostics => capabilities.diagnostic_provider.is_some(),
LanguageServerFeature::RenameSymbol => matches!(
capabilities.rename_provider,
Some(OneOf::Left(true)) | Some(OneOf::Right(_))
Expand Down Expand Up @@ -570,6 +571,9 @@ impl Client {
did_rename: Some(true),
..Default::default()
}),
diagnostic: Some(lsp::DiagnosticWorkspaceClientCapabilities {
refresh_support: Some(true),
}),
..Default::default()
}),
text_document: Some(lsp::TextDocumentClientCapabilities {
Expand Down Expand Up @@ -647,6 +651,10 @@ impl Client {
}),
..Default::default()
}),
diagnostic: Some(lsp::DiagnosticClientCapabilities {
dynamic_registration: Some(false),
related_document_support: Some(true),
}),
publish_diagnostics: Some(lsp::PublishDiagnosticsClientCapabilities {
version_support: Some(true),
tag_support: Some(lsp::TagSupport {
Expand Down Expand Up @@ -1223,6 +1231,32 @@ impl Client {
})
}

pub fn text_document_diagnostic(
&self,
text_document: lsp::TextDocumentIdentifier,
previous_result_id: Option<String>,
) -> Option<impl Future<Output = Result<Value>>> {
let capabilities = self.capabilities();

// Return early if the server does not support pull diagnostic.
let identifier = match capabilities.diagnostic_provider.as_ref()? {
lsp::DiagnosticServerCapabilities::Options(cap) => cap.identifier.clone(),
lsp::DiagnosticServerCapabilities::RegistrationOptions(cap) => {
cap.diagnostic_options.identifier.clone()
}
};

let params = lsp::DocumentDiagnosticParams {
text_document,
identifier,
previous_result_id,
work_done_progress_params: lsp::WorkDoneProgressParams::default(),
partial_result_params: lsp::PartialResultParams::default(),
};

Some(self.call::<lsp::request::DocumentDiagnosticRequest>(params))
}

pub fn text_document_document_highlight(
&self,
text_document: lsp::TextDocumentIdentifier,
Expand Down
4 changes: 3 additions & 1 deletion helix-lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub use client::Client;
pub use futures_executor::block_on;
pub use helix_lsp_types as lsp;
pub use jsonrpc::Call;
use lsp::request::Request;
pub use lsp::{Position, Url};

use futures_util::stream::select_all::SelectAll;
Expand Down Expand Up @@ -569,7 +570,6 @@ pub enum MethodCall {

impl MethodCall {
pub fn parse(method: &str, params: jsonrpc::Params) -> Result<MethodCall> {
use lsp::request::Request;
let request = match method {
lsp::request::WorkDoneProgressCreate::METHOD => {
let params: lsp::WorkDoneProgressCreateParams = params.parse()?;
Expand Down Expand Up @@ -614,6 +614,7 @@ pub enum Notification {
ShowMessage(lsp::ShowMessageParams),
LogMessage(lsp::LogMessageParams),
ProgressMessage(lsp::ProgressParams),
WorkspaceDiagnosticRefresh,
SofusA marked this conversation as resolved.
Show resolved Hide resolved
}

impl Notification {
Expand All @@ -640,6 +641,7 @@ impl Notification {
let params: lsp::ProgressParams = params.parse()?;
Self::ProgressMessage(params)
}
lsp::request::WorkspaceDiagnosticRefresh::METHOD => Self::WorkspaceDiagnosticRefresh,
_ => {
return Err(Error::Unhandled);
}
Expand Down
120 changes: 25 additions & 95 deletions helix-term/src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use helix_view::{
align_view,
document::{DocumentOpenError, DocumentSavedEventResult},
editor::{ConfigEvent, EditorEvent},
events::DiagnosticsDidChange,
graphics::Rect,
theme,
tree::Layout,
Expand All @@ -33,7 +32,7 @@ use crate::{
use log::{debug, error, info, warn};
#[cfg(not(feature = "integration"))]
use std::io::stdout;
use std::{collections::btree_map::Entry, io::stdin, path::Path, sync::Arc};
use std::{io::stdin, path::Path, sync::Arc};

#[cfg(not(windows))]
use anyhow::Context;
Expand Down Expand Up @@ -704,6 +703,15 @@ impl Application {
};

match notification {
Notification::WorkspaceDiagnosticRefresh => {
for document in self.editor.documents() {
let langugage_server = language_server!();
handlers::diagnostics::pull_diagnostics_for_document(
document,
langugage_server,
);
}
}
Notification::Initialized => {
let language_server = language_server!();

Expand Down Expand Up @@ -735,9 +743,11 @@ impl Application {
doc.text(),
language_id,
));

helix_event::dispatch(helix_view::events::DocumentDidOpen { doc });
Copy link
Member

Choose a reason for hiding this comment

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

Ideally we should only be dispatching the internal DocumentDidOpen when we initially open a buffer. Here instead of using DocumentDidOpen we should be able to use pull_diagnostics_for_document

Copy link
Author

Choose a reason for hiding this comment

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

Changed to use pull_diagnostics_for_document directly

}
}
Notification::PublishDiagnostics(mut params) => {
Notification::PublishDiagnostics(params) => {
let uri = match helix_core::Uri::try_from(params.uri) {
Ok(uri) => uri,
Err(err) => {
Expand All @@ -750,100 +760,20 @@ impl Application {
log::error!("Discarding publishDiagnostic notification sent by an uninitialized server: {}", language_server.name());
return;
}
// have to inline the function because of borrow checking...
let doc = self.editor.documents.values_mut()
.find(|doc| doc.uri().is_some_and(|u| u == uri))
.filter(|doc| {
if let Some(version) = params.version {
if version != doc.version() {
log::info!("Version ({version}) is out of date for {uri:?} (expected ({}), dropping PublishDiagnostic notification", doc.version());
return false;
}
}
true
});

let mut unchanged_diag_sources = Vec::new();
if let Some(doc) = &doc {
let lang_conf = doc.language.clone();

if let Some(lang_conf) = &lang_conf {
if let Some(old_diagnostics) = self.editor.diagnostics.get(&uri) {
if !lang_conf.persistent_diagnostic_sources.is_empty() {
// Sort diagnostics first by severity and then by line numbers.
// Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order
params
.diagnostics
.sort_by_key(|d| (d.severity, d.range.start));
}
for source in &lang_conf.persistent_diagnostic_sources {
let new_diagnostics = params
.diagnostics
.iter()
.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)
})
.map(|(d, _)| d);
if new_diagnostics.eq(old_diagnostics) {
unchanged_diag_sources.push(source.clone())
}
}
}
}
}

let diagnostics = params.diagnostics.into_iter().map(|d| (d, server_id));

// Insert the original lsp::Diagnostics here because we may have no open document
// for diagnosic message and so we can't calculate the exact position.
// When using them later in the diagnostics picker, we calculate them on-demand.
let diagnostics = match self.editor.diagnostics.entry(uri) {
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.extend(diagnostics);
current_diagnostics
// Sort diagnostics first by severity and then by line numbers.
}
Entry::Vacant(v) => v.insert(diagnostics.collect()),
};

// 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));

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.source.as_ref().map_or(true, |source| {
!unchanged_diag_sources.contains(source)
})
};
let diagnostics = Editor::doc_diagnostics_with_filter(
&self.editor.language_servers,
&self.editor.diagnostics,
doc,
diagnostic_of_language_server_and_not_in_unchanged_sources,
);
doc.replace_diagnostics(
diagnostics,
&unchanged_diag_sources,
Some(server_id),
);
let diagnostics: Vec<(lsp::Diagnostic, LanguageServerId)> = params
.diagnostics
.into_iter()
.map(|d| (d, server_id))
.collect();

let doc = doc.id();
helix_event::dispatch(DiagnosticsDidChange {
editor: &mut self.editor,
doc,
});
}
self.editor.add_diagnostics(
diagnostics,
server_id,
uri,
params.version,
None,
);
}
Notification::ShowMessage(params) => {
log::warn!("unhandled window/showMessage: {:?}", params);
Expand Down
4 changes: 4 additions & 0 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,10 @@ fn goto_buffer(editor: &mut Editor, direction: Direction, count: usize) {

let id = *id;

if let Some(doc) = editor.document(id) {
helix_event::dispatch(helix_view::events::DocumentDidOpen { doc });
};

editor.switch(id, Action::Replace);
}

Expand Down
5 changes: 4 additions & 1 deletion helix-term/src/events.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use helix_event::{events, register_event};
use helix_view::document::Mode;
use helix_view::events::{DiagnosticsDidChange, DocumentDidChange, SelectionDidChange};
use helix_view::events::{
DiagnosticsDidChange, DocumentDidChange, DocumentDidOpen, SelectionDidChange,
};

use crate::commands;
use crate::keymap::MappableCommand;
Expand All @@ -16,6 +18,7 @@ pub fn register() {
register_event::<PostInsertChar>();
register_event::<PostCommand>();
register_event::<DocumentDidChange>();
register_event::<DocumentDidOpen>();
register_event::<SelectionDidChange>();
register_event::<DiagnosticsDidChange>();
}
6 changes: 5 additions & 1 deletion helix-term/src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,19 @@ use arc_swap::ArcSwap;
use helix_event::AsyncHook;

use crate::config::Config;

use crate::events;
use crate::handlers::auto_save::AutoSaveHandler;
use crate::handlers::completion::CompletionHandler;
use crate::handlers::diagnostics::PullDiagnosticsHandler;
use crate::handlers::signature_help::SignatureHelpHandler;

pub use completion::trigger_auto_completion;
pub use helix_view::handlers::Handlers;

mod auto_save;
pub mod completion;
mod diagnostics;
pub mod diagnostics;
mod signature_help;

pub fn setup(config: Arc<ArcSwap<Config>>) -> Handlers {
Expand All @@ -23,11 +25,13 @@ pub fn setup(config: Arc<ArcSwap<Config>>) -> Handlers {
let completions = CompletionHandler::new(config).spawn();
let signature_hints = SignatureHelpHandler::new().spawn();
let auto_save = AutoSaveHandler::new().spawn();
let pull_diagnostics = PullDiagnosticsHandler::new().spawn();

let handlers = Handlers {
completions,
signature_hints,
auto_save,
pull_diagnostics,
};

completion::register_hooks(&handlers);
Expand Down
Loading