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

feat(lsp): configurable diagnostic severity #1325

Merged
merged 5 commits into from
Dec 25, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
25 changes: 13 additions & 12 deletions book/src/guides/adding_languages.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,19 @@ directory](../configuration.md).

These are the available keys and descriptions for the file.

| Key | Description |
| ---- | ----------- |
| name | The name of the language |
| scope | A string like `source.js` that identifies the language. Currently, we strive to match the scope names used by popular TextMate grammars and by the Linguist library. Usually `source.<name>` or `text.<name>` in case of markup languages |
| injection-regex | regex pattern that will be tested against a language name in order to determine whether this language should be used for a potential [language injection][treesitter-language-injection] site. |
| file-types | The filetypes of the language, for example `["yml", "yaml"]` |
| shebangs | The interpreters from the shebang line, for example `["sh", "bash"]` |
| roots | A set of marker files to look for when trying to find the workspace root. For example `Cargo.lock`, `yarn.lock` |
| auto-format | Whether to autoformat this language when saving |
| comment-token | The token to use as a comment-token |
| indent | The indent to use. Has sub keys `tab-width` and `unit` |
| config | Language server configuration |
| Key | Description |
| ---- | ----------- |
| name | The name of the language |
| scope | A string like `source.js` that identifies the language. Currently, we strive to match the scope names used by popular TextMate grammars and by the Linguist library. Usually `source.<name>` or `text.<name>` in case of markup languages |
| injection-regex | regex pattern that will be tested against a language name in order to determine whether this language should be used for a potential [language injection][treesitter-language-injection] site. |
| file-types | The filetypes of the language, for example `["yml", "yaml"]` |
| shebangs | The interpreters from the shebang line, for example `["sh", "bash"]` |
| roots | A set of marker files to look for when trying to find the workspace root. For example `Cargo.lock`, `yarn.lock` |
| auto-format | Whether to autoformat this language when saving |
| diagnostic_severity | Minimal severity of diagnostic for it to be displayed. (Allowed values: `Error`, `Warning`, `Info`, `Hint`) |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the new entry, the rest is just formatting

Copy link
Member

Choose a reason for hiding this comment

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

diagnostic-severity, we remap the name to kebab-case via serde

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, forgot, fixed now

| comment-token | The token to use as a comment-token |
| indent | The indent to use. Has sub keys `tab-width` and `unit` |
| config | Language server configuration |

## Queries

Expand Down
1 change: 0 additions & 1 deletion book/src/languages.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,3 @@ Changes made to the `languages.toml` file in a user's [configuration directory](
name = "rust"
auto-format = false
```

15 changes: 11 additions & 4 deletions helix-core/src/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
//! LSP diagnostic utility types.
use serde::{Deserialize, Serialize};

/// Describes the severity level of a [`Diagnostic`].
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
#[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)]
pub enum Severity {
Error,
Warning,
Info,
Hint,
Info,
Warning,
Error,
}

impl Default for Severity {
fn default() -> Self {
Self::Hint
}
}

/// A range of `char`s within the text.
Expand Down
2 changes: 2 additions & 0 deletions helix-core/src/indent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ where
);

let doc = Rope::from(doc);
use crate::diagnostic::Severity;
use crate::syntax::{
Configuration, IndentationConfiguration, LanguageConfiguration, Loader,
};
Expand All @@ -459,6 +460,7 @@ where
roots: vec![],
comment_token: None,
auto_format: false,
diagnostic_severity: Severity::Warning,
language_server: None,
indent: Some(IndentationConfiguration {
tab_width: 4,
Expand Down
3 changes: 3 additions & 0 deletions helix-core/src/syntax.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{
chars::char_is_line_ending,
diagnostic::Severity,
regex::Regex,
transaction::{ChangeSet, Operation},
Rope, RopeSlice, Tendril,
Expand Down Expand Up @@ -63,6 +64,8 @@ pub struct LanguageConfiguration {

#[serde(default)]
pub auto_format: bool,
#[serde(default)]
pub diagnostic_severity: Severity,
sudormrfbin marked this conversation as resolved.
Show resolved Hide resolved

// content_regex
#[serde(default, skip_serializing, deserialize_with = "deserialize_regex")]
Expand Down
28 changes: 19 additions & 9 deletions helix-term/src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ impl Application {
let doc = self.editor.document_by_path_mut(&path);

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

let diagnostics = params
Expand Down Expand Up @@ -415,19 +416,28 @@ impl Application {
return None;
};

let severity =
diagnostic.severity.map(|severity| match severity {
DiagnosticSeverity::ERROR => Error,
DiagnosticSeverity::WARNING => Warning,
DiagnosticSeverity::INFORMATION => Info,
DiagnosticSeverity::HINT => Hint,
severity => unimplemented!("{:?}", severity),
Copy link
Member

Choose a reason for hiding this comment

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

Should this just return None? Or if this isn't likely to change, maybe unreachable!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is not going to change (at least in foreseeable future). I changed the statement to unreachable!, let me know if I should revert it.

});

if let Some(lang_conf) = lang_conf {
if let Some(severity) = severity {
if severity < lang_conf.diagnostic_severity {
return None;
}
}
};

Some(Diagnostic {
range: Range { start, end },
line: diagnostic.range.start.line as usize,
message: diagnostic.message,
severity: diagnostic.severity.map(
|severity| match severity {
DiagnosticSeverity::ERROR => Error,
DiagnosticSeverity::WARNING => Warning,
DiagnosticSeverity::INFORMATION => Info,
DiagnosticSeverity::HINT => Hint,
severity => unimplemented!("{:?}", severity),
},
),
severity,
// code
// source
})
Expand Down