mirror of
https://github.com/ilyakooo0/helix.git
synced 2024-08-17 12:00:22 +03:00
make diagnostics stick to word boundaries
Diagnostics are currently extended if text is inserted at their end. This is desirable when inserting text after an identifier. For example consider: let foo = 2; --- unused variable Renaming the identifier should extend the diagnostic: let foobar = 2; ------ unused variable This is currently implemented in helix but as a consequence adding whitespaces or a type hint also extends the diagnostic: let foo = 2; -------- unused variable let foo: Bar = 2; -------- unused variable In these cases the diagnostic should remain unchanged: let foo = 2; --- unused variable let foo: Bar = 2; --- unused variable As a heuristic helix will now only extend diagnostics that end on a word char if new chars are appended to the word (so not for punctuation/ whitespace). The idea for this mapping was inspired for the word level tracking vscode uses for many positions. While VSCode doesn't currently update diagnostics after receiving publishDiagnostic it does use this system for inlay hints for example. Similarly, the new association mechanism implemented here can be used for word level tracking of inlay hints. A similar mapping function is implemented for word starts. Together these can be used to make a diagnostic stick to a word. If that word is removed that diagnostic is automatically removed too. This is the exact same behavior VSCode inlay hints eixibit.
This commit is contained in:
parent
8653e1b02f
commit
515ef17207
@ -39,6 +39,10 @@ pub enum DiagnosticTag {
|
|||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
pub struct Diagnostic {
|
pub struct Diagnostic {
|
||||||
pub range: Range,
|
pub range: Range,
|
||||||
|
// whether this diagnostic ends at the end of(or inside) a word
|
||||||
|
pub ends_at_word: bool,
|
||||||
|
pub starts_at_word: bool,
|
||||||
|
pub zero_width: bool,
|
||||||
pub line: usize,
|
pub line: usize,
|
||||||
pub message: String,
|
pub message: String,
|
||||||
pub severity: Option<Severity>,
|
pub severity: Option<Severity>,
|
||||||
|
@ -1,7 +1,7 @@
|
|||||||
use ropey::RopeSlice;
|
use ropey::RopeSlice;
|
||||||
use smallvec::SmallVec;
|
use smallvec::SmallVec;
|
||||||
|
|
||||||
use crate::{Range, Rope, Selection, Tendril};
|
use crate::{chars::char_is_word, Range, Rope, Selection, Tendril};
|
||||||
use std::{borrow::Cow, iter::once};
|
use std::{borrow::Cow, iter::once};
|
||||||
|
|
||||||
/// (from, to, replacement)
|
/// (from, to, replacement)
|
||||||
@ -23,6 +23,30 @@ pub enum Operation {
|
|||||||
pub enum Assoc {
|
pub enum Assoc {
|
||||||
Before,
|
Before,
|
||||||
After,
|
After,
|
||||||
|
/// Acts like `After` if a word character is inserted
|
||||||
|
/// after the position, otherwise acts like `Before`
|
||||||
|
AfterWord,
|
||||||
|
/// Acts like `Before` if a word character is inserted
|
||||||
|
/// before the position, otherwise acts like `After`
|
||||||
|
BeforeWord,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Assoc {
|
||||||
|
/// Whether to stick to gaps
|
||||||
|
fn stay_at_gaps(self) -> bool {
|
||||||
|
!matches!(self, Self::BeforeWord | Self::AfterWord)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn insert_offset(self, s: &str) -> usize {
|
||||||
|
let chars = s.chars().count();
|
||||||
|
match self {
|
||||||
|
Assoc::After => chars,
|
||||||
|
Assoc::AfterWord => s.chars().take_while(|&c| char_is_word(c)).count(),
|
||||||
|
// return position before inserted text
|
||||||
|
Assoc::Before => 0,
|
||||||
|
Assoc::BeforeWord => chars - s.chars().rev().take_while(|&c| char_is_word(c)).count(),
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Default, Clone, PartialEq, Eq)]
|
#[derive(Debug, Default, Clone, PartialEq, Eq)]
|
||||||
@ -415,8 +439,6 @@ macro_rules! map {
|
|||||||
map!(|pos, _| (old_end > pos).then_some(new_pos), i);
|
map!(|pos, _| (old_end > pos).then_some(new_pos), i);
|
||||||
}
|
}
|
||||||
Insert(s) => {
|
Insert(s) => {
|
||||||
let ins = s.chars().count();
|
|
||||||
|
|
||||||
// a subsequent delete means a replace, consume it
|
// a subsequent delete means a replace, consume it
|
||||||
if let Some((_, Delete(len))) = iter.peek() {
|
if let Some((_, Delete(len))) = iter.peek() {
|
||||||
iter.next();
|
iter.next();
|
||||||
@ -424,13 +446,13 @@ macro_rules! map {
|
|||||||
old_end = old_pos + len;
|
old_end = old_pos + len;
|
||||||
// in range of replaced text
|
// in range of replaced text
|
||||||
map!(
|
map!(
|
||||||
|pos, assoc| (old_end > pos).then(|| {
|
|pos, assoc: Assoc| (old_end > pos).then(|| {
|
||||||
// at point or tracking before
|
// at point or tracking before
|
||||||
if pos == old_pos || assoc == Assoc::Before {
|
if pos == old_pos && assoc.stay_at_gaps() {
|
||||||
new_pos
|
new_pos
|
||||||
} else {
|
} else {
|
||||||
// place to end of insert
|
// place to end of insert
|
||||||
new_pos + ins
|
new_pos + assoc.insert_offset(s)
|
||||||
}
|
}
|
||||||
}),
|
}),
|
||||||
i
|
i
|
||||||
@ -438,20 +460,15 @@ macro_rules! map {
|
|||||||
} else {
|
} else {
|
||||||
// at insert point
|
// at insert point
|
||||||
map!(
|
map!(
|
||||||
|pos, assoc| (old_pos == pos).then(|| {
|
|pos, assoc: Assoc| (old_pos == pos).then(|| {
|
||||||
// return position before inserted text
|
// return position before inserted text
|
||||||
if assoc == Assoc::Before {
|
new_pos + assoc.insert_offset(s)
|
||||||
new_pos
|
|
||||||
} else {
|
|
||||||
// after text
|
|
||||||
new_pos + ins
|
|
||||||
}
|
|
||||||
}),
|
}),
|
||||||
i
|
i
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
new_pos += ins;
|
new_pos += s.chars().count();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
old_pos = old_end;
|
old_pos = old_end;
|
||||||
@ -884,6 +901,48 @@ fn map_pos() {
|
|||||||
let mut positions = [4, 2];
|
let mut positions = [4, 2];
|
||||||
cs.update_positions(positions.iter_mut().map(|pos| (pos, Assoc::After)));
|
cs.update_positions(positions.iter_mut().map(|pos| (pos, Assoc::After)));
|
||||||
assert_eq!(positions, [4, 2]);
|
assert_eq!(positions, [4, 2]);
|
||||||
|
// stays at word boundary
|
||||||
|
let cs = ChangeSet {
|
||||||
|
changes: vec![
|
||||||
|
Retain(2), // <space><space>
|
||||||
|
Insert(" ab".into()),
|
||||||
|
Retain(2), // cd
|
||||||
|
Insert("de ".into()),
|
||||||
|
],
|
||||||
|
len: 4,
|
||||||
|
len_after: 10,
|
||||||
|
};
|
||||||
|
assert_eq!(cs.map_pos(2, Assoc::BeforeWord), 3);
|
||||||
|
assert_eq!(cs.map_pos(4, Assoc::AfterWord), 9);
|
||||||
|
let cs = ChangeSet {
|
||||||
|
changes: vec![
|
||||||
|
Retain(1), // <space>
|
||||||
|
Insert(" b".into()),
|
||||||
|
Delete(1), // c
|
||||||
|
Retain(1), // d
|
||||||
|
Insert("e ".into()),
|
||||||
|
Delete(1), // <space>
|
||||||
|
],
|
||||||
|
len: 5,
|
||||||
|
len_after: 7,
|
||||||
|
};
|
||||||
|
assert_eq!(cs.map_pos(1, Assoc::BeforeWord), 2);
|
||||||
|
assert_eq!(cs.map_pos(3, Assoc::AfterWord), 5);
|
||||||
|
let cs = ChangeSet {
|
||||||
|
changes: vec![
|
||||||
|
Retain(1), // <space>
|
||||||
|
Insert("a".into()),
|
||||||
|
Delete(2), // <space>b
|
||||||
|
Retain(1), // d
|
||||||
|
Insert("e".into()),
|
||||||
|
Delete(1), // f
|
||||||
|
Retain(1), // <space>
|
||||||
|
],
|
||||||
|
len: 5,
|
||||||
|
len_after: 7,
|
||||||
|
};
|
||||||
|
assert_eq!(cs.map_pos(2, Assoc::BeforeWord), 1);
|
||||||
|
assert_eq!(cs.map_pos(4, Assoc::AfterWord), 4);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
@ -1,6 +1,7 @@
|
|||||||
use arc_swap::{access::Map, ArcSwap};
|
use arc_swap::{access::Map, ArcSwap};
|
||||||
use futures_util::Stream;
|
use futures_util::Stream;
|
||||||
use helix_core::{
|
use helix_core::{
|
||||||
|
chars::char_is_word,
|
||||||
diagnostic::{DiagnosticTag, NumberOrString},
|
diagnostic::{DiagnosticTag, NumberOrString},
|
||||||
path::get_relative_path,
|
path::get_relative_path,
|
||||||
pos_at_coords, syntax, Selection,
|
pos_at_coords, syntax, Selection,
|
||||||
@ -811,7 +812,6 @@ macro_rules! language_server {
|
|||||||
log::warn!("lsp position out of bounds - {:?}", diagnostic);
|
log::warn!("lsp position out of bounds - {:?}", diagnostic);
|
||||||
return None;
|
return None;
|
||||||
};
|
};
|
||||||
|
|
||||||
let severity =
|
let severity =
|
||||||
diagnostic.severity.map(|severity| match severity {
|
diagnostic.severity.map(|severity| match severity {
|
||||||
DiagnosticSeverity::ERROR => Error,
|
DiagnosticSeverity::ERROR => Error,
|
||||||
@ -863,8 +863,17 @@ macro_rules! language_server {
|
|||||||
Vec::new()
|
Vec::new()
|
||||||
};
|
};
|
||||||
|
|
||||||
|
let ends_at_word = start != end
|
||||||
|
&& end != 0
|
||||||
|
&& text.get_char(end - 1).map_or(false, char_is_word);
|
||||||
|
let starts_at_word = start != end
|
||||||
|
&& text.get_char(start).map_or(false, char_is_word);
|
||||||
|
|
||||||
Some(Diagnostic {
|
Some(Diagnostic {
|
||||||
range: Range { start, end },
|
range: Range { start, end },
|
||||||
|
ends_at_word,
|
||||||
|
starts_at_word,
|
||||||
|
zero_width: start == end,
|
||||||
line: diagnostic.range.start.line as usize,
|
line: diagnostic.range.start.line as usize,
|
||||||
message: diagnostic.message.clone(),
|
message: diagnostic.message.clone(),
|
||||||
severity,
|
severity,
|
||||||
|
@ -1213,20 +1213,32 @@ fn apply_impl(
|
|||||||
|
|
||||||
let changes = transaction.changes();
|
let changes = transaction.changes();
|
||||||
|
|
||||||
changes.update_positions(
|
// map diagnostics over changes too
|
||||||
self.diagnostics
|
changes.update_positions(self.diagnostics.iter_mut().map(|diagnostic| {
|
||||||
.iter_mut()
|
let assoc = if diagnostic.starts_at_word {
|
||||||
.map(|diagnostic| (&mut diagnostic.range.start, Assoc::After)),
|
Assoc::BeforeWord
|
||||||
);
|
} else {
|
||||||
changes.update_positions(
|
Assoc::After
|
||||||
self.diagnostics
|
};
|
||||||
.iter_mut()
|
(&mut diagnostic.range.start, assoc)
|
||||||
.map(|diagnostic| (&mut diagnostic.range.end, Assoc::After)),
|
}));
|
||||||
);
|
changes.update_positions(self.diagnostics.iter_mut().map(|diagnostic| {
|
||||||
// map state.diagnostics over changes::map_pos too
|
let assoc = if diagnostic.ends_at_word {
|
||||||
for diagnostic in &mut self.diagnostics {
|
Assoc::AfterWord
|
||||||
diagnostic.line = self.text.char_to_line(diagnostic.range.start);
|
} else {
|
||||||
|
Assoc::Before
|
||||||
|
};
|
||||||
|
(&mut diagnostic.range.end, assoc)
|
||||||
|
}));
|
||||||
|
self.diagnostics.retain_mut(|diagnostic| {
|
||||||
|
if diagnostic.range.start > diagnostic.range.end
|
||||||
|
|| (!diagnostic.zero_width && diagnostic.range.start == diagnostic.range.end)
|
||||||
|
{
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
|
diagnostic.line = self.text.char_to_line(diagnostic.range.start);
|
||||||
|
true
|
||||||
|
});
|
||||||
|
|
||||||
self.diagnostics
|
self.diagnostics
|
||||||
.sort_unstable_by_key(|diagnostic| diagnostic.range);
|
.sort_unstable_by_key(|diagnostic| diagnostic.range);
|
||||||
|
Loading…
Reference in New Issue
Block a user