From 5f4f171b73232e5ec5e4b7153084a93033ab5cf0 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Sun, 16 Oct 2022 14:52:04 -0500 Subject: [PATCH] Fix debug assertion for diagnostic sort order (#4319) The debug assertion that document diagnostics are sorted incorrectly panics for cases like `[161..164, 162..162]`. The merging behavior in the following lines that relies on the assertion only needs the input ranges to be sorted by `range.start`, so this change simplifies the assertion to only catch violations of that assumption. --- helix-term/src/ui/editor.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index fb70fa0f..8e9d8631 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -302,16 +302,7 @@ impl EditorView { let mut warning_vec = Vec::new(); let mut error_vec = Vec::new(); - let diagnostics = doc.diagnostics(); - - // Diagnostics must be sorted by range. Otherwise, the merge strategy - // below would not be accurate. - debug_assert!(diagnostics - .windows(2) - .all(|window| window[0].range.start <= window[1].range.start - && window[0].range.end <= window[1].range.end)); - - for diagnostic in diagnostics { + for diagnostic in doc.diagnostics() { // Separate diagnostics into different Vecs by severity. let (vec, scope) = match diagnostic.severity { Some(Severity::Info) => (&mut info_vec, info), @@ -325,6 +316,11 @@ impl EditorView { // merge the two together. Otherwise push a new span. match vec.last_mut() { Some((_, range)) if diagnostic.range.start <= range.end => { + // This branch merges overlapping diagnostics, assuming that the current + // diagnostic starts on range.start or later. If this assertion fails, + // we will discard some part of `diagnostic`. This implies that + // `doc.diagnostics()` is not sorted by `diagnostic.range`. + debug_assert!(range.start <= diagnostic.range.start); range.end = diagnostic.range.end.max(range.end) } _ => vec.push((scope, diagnostic.range.start..diagnostic.range.end)),