-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add workspace and document diagnostics picker #2013
Changes from all commits
7245c7f
e8a0198
f3c6c40
b4c5fdd
e164409
2920808
f0e0890
6f87bab
469c4cf
ce36e55
7086952
07854d9
6ce102c
4819226
dbd2848
60daf1c
2bfc8f9
4c5c9df
e94bb27
19aa615
4db6955
9d9aeaf
7740547
38ccf7e
7301d05
6014ec8
154e985
3fdc7d2
5a3378f
48f1d4f
6779697
627bb09
45ff7fd
9477627
3bb9498
8c5e289
973f4dd
50e67c9
00be70b
1b94cb7
21607e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,25 @@ | ||
use helix_lsp::{ | ||
block_on, lsp, | ||
block_on, | ||
lsp::{self, DiagnosticSeverity, NumberOrString}, | ||
util::{diagnostic_to_lsp_diagnostic, lsp_pos_to_pos, lsp_range_to_range, range_to_lsp_range}, | ||
OffsetEncoding, | ||
}; | ||
use tui::text::{Span, Spans}; | ||
|
||
use super::{align_view, push_jump, Align, Context, Editor}; | ||
|
||
use helix_core::Selection; | ||
use helix_view::editor::Action; | ||
use helix_core::{path, Selection}; | ||
use helix_view::{ | ||
editor::Action, | ||
theme::{Modifier, Style}, | ||
}; | ||
|
||
use crate::{ | ||
compositor::{self, Compositor}, | ||
ui::{self, overlay::overlayed, FileLocation, FilePicker, Popup, PromptEvent}, | ||
}; | ||
|
||
use std::borrow::Cow; | ||
use std::{borrow::Cow, collections::BTreeMap}; | ||
|
||
/// Gets the language server that is attached to a document, and | ||
/// if it's not active displays a status message. Using this macro | ||
|
@@ -145,6 +150,97 @@ fn sym_picker( | |
.truncate_start(false) | ||
} | ||
|
||
fn diag_picker( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It follows the same naming schema as the There's a function |
||
cx: &Context, | ||
diagnostics: BTreeMap<lsp::Url, Vec<lsp::Diagnostic>>, | ||
current_path: Option<lsp::Url>, | ||
offset_encoding: OffsetEncoding, | ||
) -> FilePicker<(lsp::Url, lsp::Diagnostic)> { | ||
// TODO: drop current_path comparison and instead use workspace: bool flag? | ||
|
||
// flatten the map to a vec of (url, diag) pairs | ||
let mut flat_diag = Vec::new(); | ||
for (url, diags) in diagnostics { | ||
flat_diag.reserve(diags.len()); | ||
for diag in diags { | ||
flat_diag.push((url.clone(), diag)); | ||
} | ||
} | ||
|
||
let hint = cx.editor.theme.get("hint"); | ||
let info = cx.editor.theme.get("info"); | ||
let warning = cx.editor.theme.get("warning"); | ||
let error = cx.editor.theme.get("error"); | ||
|
||
FilePicker::new( | ||
flat_diag, | ||
move |(url, diag)| { | ||
let mut style = diag | ||
.severity | ||
.map(|s| match s { | ||
DiagnosticSeverity::HINT => hint, | ||
DiagnosticSeverity::INFORMATION => info, | ||
DiagnosticSeverity::WARNING => warning, | ||
DiagnosticSeverity::ERROR => error, | ||
_ => Style::default(), | ||
}) | ||
.unwrap_or_default(); | ||
|
||
// remove background as it is distracting in the picker list | ||
style.bg = None; | ||
hirschenberger marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
let code = diag | ||
.code | ||
.as_ref() | ||
.map(|c| match c { | ||
NumberOrString::Number(n) => n.to_string(), | ||
NumberOrString::String(s) => s.to_string(), | ||
}) | ||
.unwrap_or_default(); | ||
|
||
let truncated_path = path::get_truncated_path(url.path()) | ||
.to_string_lossy() | ||
.into_owned(); | ||
|
||
Spans::from(vec![ | ||
Span::styled( | ||
diag.source.clone().unwrap_or_default(), | ||
style.add_modifier(Modifier::BOLD), | ||
), | ||
Span::raw(": "), | ||
Span::styled(truncated_path, style), | ||
Span::raw(" - "), | ||
Span::styled(code, style.add_modifier(Modifier::BOLD)), | ||
Span::raw(": "), | ||
Span::styled(&diag.message, style), | ||
]) | ||
}, | ||
move |cx, (url, diag), action| { | ||
if current_path.as_ref() == Some(url) { | ||
let (view, doc) = current!(cx.editor); | ||
push_jump(view, doc); | ||
} else { | ||
let path = url.to_file_path().unwrap(); | ||
cx.editor.open(&path, action).expect("editor.open failed"); | ||
} | ||
|
||
let (view, doc) = current!(cx.editor); | ||
|
||
if let Some(range) = lsp_range_to_range(doc.text(), diag.range, offset_encoding) { | ||
// we flip the range so that the cursor sits on the start of the symbol | ||
// (for example start of the function). | ||
doc.set_selection(view.id, Selection::single(range.head, range.anchor)); | ||
align_view(doc, view, Align::Center); | ||
} | ||
}, | ||
move |_editor, (url, diag)| { | ||
let location = lsp::Location::new(url.clone(), diag.range); | ||
Some(location_to_file_location(&location)) | ||
}, | ||
) | ||
.truncate_start(false) | ||
} | ||
|
||
pub fn symbol_picker(cx: &mut Context) { | ||
fn nested_to_flat( | ||
list: &mut Vec<lsp::SymbolInformation>, | ||
|
@@ -215,6 +311,37 @@ pub fn workspace_symbol_picker(cx: &mut Context) { | |
) | ||
} | ||
|
||
pub fn diagnostics_picker(cx: &mut Context) { | ||
let doc = doc!(cx.editor); | ||
let language_server = language_server!(cx.editor, doc); | ||
if let Some(current_url) = doc.url() { | ||
let offset_encoding = language_server.offset_encoding(); | ||
let diagnostics = cx | ||
.editor | ||
.diagnostics | ||
.get(¤t_url) | ||
.cloned() | ||
.unwrap_or_default(); | ||
let picker = diag_picker( | ||
cx, | ||
[(current_url.clone(), diagnostics)].into(), | ||
Some(current_url), | ||
offset_encoding, | ||
); | ||
cx.push_layer(Box::new(overlayed(picker))); | ||
} | ||
} | ||
|
||
pub fn workspace_diagnostics_picker(cx: &mut Context) { | ||
let doc = doc!(cx.editor); | ||
let language_server = language_server!(cx.editor, doc); | ||
let current_url = doc.url(); | ||
let offset_encoding = language_server.offset_encoding(); | ||
let diagnostics = cx.editor.diagnostics.clone(); | ||
let picker = diag_picker(cx, diagnostics, current_url, offset_encoding); | ||
cx.push_layer(Box::new(overlayed(picker))); | ||
} | ||
|
||
impl ui::menu::Item for lsp::CodeActionOrCommand { | ||
fn label(&self) -> &str { | ||
match self { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this loop necessary?
d
is anOption
. I'd useif let Some()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to me you actually want to iterate over
ancestors()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
d
is anOsStr
andbase
is a&Path
Path
implementsIterator
for component traversal.