Skip to content

Commit

Permalink
Add config to mark diagnostic sources as persistent
Browse files Browse the repository at this point in the history
  • Loading branch information
pascalkuthe committed Aug 1, 2023
1 parent a9849eb commit c65b0d9
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 40 deletions.
1 change: 1 addition & 0 deletions book/src/languages.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ These configuration keys are available:
| `formatter` | The formatter for the language, it will take precedence over the lsp when defined. The formatter must be able to take the original file as input from stdin and write the formatted file to stdout |
| `text-width` | Maximum line length. Used for the `:reflow` command and soft-wrapping if `soft-wrap.wrap-at-text-width` is set, defaults to `editor.text-width` |
| `workspace-lsp-roots` | Directories relative to the workspace root that are treated as LSP roots. Should only be set in `.helix/config.toml`. Overwrites the setting of the same name in `config.toml` if set. |
| `persistent-diagnostic-sources` | An array of LSP diagnostic sources assumed unchanged when the language server resends the same set of diagnostics. Helix can track the position for these diagnostics internally instead. Useful for diagnostics that are recomputed on save.

### File-type detection and the `file-types` key

Expand Down
2 changes: 2 additions & 0 deletions helix-core/src/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ pub struct LanguageConfiguration {
/// Hardcoded LSP root directories relative to the workspace root, like `examples` or `tools/fuzz`.
/// Falling back to the current working directory if none are configured.
pub workspace_lsp_roots: Option<Vec<PathBuf>>,
#[serde(default)]
pub persistent_diagnostic_sources: Vec<String>,
}

#[derive(Debug, PartialEq, Eq, Hash)]
Expand Down
105 changes: 68 additions & 37 deletions helix-term/src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ impl Application {
));
}
}
Notification::PublishDiagnostics(params) => {
Notification::PublishDiagnostics(mut params) => {
let path = match params.uri.to_file_path() {
Ok(path) => path,
Err(_) => {
Expand All @@ -752,31 +752,69 @@ impl Application {
return;
}
let offset_encoding = language_server.offset_encoding();
let doc = self.editor.document_by_path_mut(&path).filter(|doc| {
if let Some(version) = params.version {
if version != doc.version() {
log::info!("Version ({version}) is out of date for {path:?} (expected ({}), dropping PublishDiagnostic notification", doc.version());
return false;
// have to inline the function because of borrow checking...
let doc = self.editor.documents.values_mut()
.find(|doc| doc.path().map(|p| p == &path).unwrap_or(false))
.filter(|doc| {
if let Some(version) = params.version {
if version != doc.version() {
log::info!("Version ({version}) is out of date for {path:?} (expected ({}), dropping PublishDiagnostic notification", doc.version());
return false;
}
}
}

true
});
true
});

if let Some(doc) = doc {
let lang_conf = doc.language_config();
let text = doc.text();
let lang_conf = doc.language.clone();
let text = doc.text().clone();

let mut unchaged_diag_sources_ = Vec::new();
if let Some(lang_conf) = &lang_conf {
if let Some(old_diagnostics) =
self.editor.diagnostics.get(&params.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_unstable_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) {
unchaged_diag_sources_.push(source.clone())
}
}
}
}

let diagnostics = params
.diagnostics
.iter()
.filter_map(|diagnostic| {
let unchaged_diag_sources = &unchaged_diag_sources_;
let diagnostics =
params.diagnostics.iter().filter_map(move |diagnostic| {
use helix_core::diagnostic::{Diagnostic, Range, Severity::*};
use lsp::DiagnosticSeverity;

if diagnostic.source.as_ref().map_or(false, |source| {
unchaged_diag_sources.contains(source)
}) {
return None;
}

// TODO: convert inside server
let start = if let Some(start) = lsp_pos_to_pos(
text,
&text,
diagnostic.range.start,
offset_encoding,
) {
Expand All @@ -787,7 +825,7 @@ impl Application {
};

let end = if let Some(end) =
lsp_pos_to_pos(text, diagnostic.range.end, offset_encoding)
lsp_pos_to_pos(&text, diagnostic.range.end, offset_encoding)
{
end
} else {
Expand All @@ -807,7 +845,7 @@ impl Application {
),
});

if let Some(lang_conf) = lang_conf {
if let Some(lang_conf) = &lang_conf {
if let Some(severity) = severity {
if severity < lang_conf.diagnostic_severity {
return None;
Expand Down Expand Up @@ -857,38 +895,31 @@ impl Application {
data: diagnostic.data.clone(),
language_server_id: server_id,
})
})
.collect();
});

doc.replace_diagnostics(diagnostics, server_id);
doc.replace_diagnostics(diagnostics, unchaged_diag_sources, server_id);
}

let mut diagnostics = params
.diagnostics
.into_iter()
.map(|d| (d, server_id))
.collect();
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.
match self.editor.diagnostics.entry(params.uri) {
let diagnostics = match self.editor.diagnostics.entry(params.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.append(&mut diagnostics);
// Sort diagnostics first by severity and then by line numbers.
// Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order
current_diagnostics.extend(diagnostics);
current_diagnostics
.sort_unstable_by_key(|(d, _)| (d.severity, d.range.start));
}
Entry::Vacant(v) => {
diagnostics
.sort_unstable_by_key(|(d, _)| (d.severity, d.range.start));
v.insert(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_unstable_by_key(|(d, _)| (d.severity, d.range.start));
}
Notification::ShowMessage(params) => {
log::warn!("unhandled window/showMessage: {:?}", params);
Expand Down
21 changes: 18 additions & 3 deletions helix-view/src/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1667,11 +1667,26 @@ impl Document {

pub fn replace_diagnostics(
&mut self,
mut diagnostics: Vec<Diagnostic>,
diagnostics: impl IntoIterator<Item = Diagnostic>,
unchanged_sources: &[String],
language_server_id: usize,
) {
self.clear_diagnostics(language_server_id);
self.diagnostics.append(&mut diagnostics);
if unchanged_sources.is_empty() {
self.clear_diagnostics(language_server_id);
} else {
self.diagnostics.retain(|d| {
if d.language_server_id != language_server_id {
return true;
}

if let Some(source) = &d.source {
unchanged_sources.contains(source)
} else {
false
}
});
}
self.diagnostics.extend(diagnostics);
self.diagnostics
.sort_unstable_by_key(|diagnostic| diagnostic.range);
}
Expand Down
1 change: 1 addition & 0 deletions languages.toml
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ auto-format = true
comment-token = "//"
language-servers = [ "rust-analyzer" ]
indent = { tab-width = 4, unit = " " }
persistent-diagnostic-sources = ["rustc", "clippy"]

[language.auto-pairs]
'(' = ')'
Expand Down

0 comments on commit c65b0d9

Please sign in to comment.