Implement `shift-f8` to go to previous diagnostic

Antonio Scandurra and Nathan Sobo created

Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Change summary

crates/editor/src/editor.rs           | 66 +++++++++++++++++++---------
crates/editor/src/items.rs            |  2 
crates/editor/src/multi_buffer.rs     |  6 ++
crates/language/src/buffer.rs         |  6 +
crates/language/src/diagnostic_set.rs | 13 ++++
crates/language/src/tests.rs          |  4 
crates/project/src/project.rs         | 10 ++--
crates/server/src/rpc.rs              |  2 
8 files changed, 74 insertions(+), 35 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -115,7 +115,7 @@ action!(ToggleComments);
 action!(SelectLargerSyntaxNode);
 action!(SelectSmallerSyntaxNode);
 action!(MoveToEnclosingBracket);
-action!(ShowNextDiagnostic);
+action!(GoToDiagnostic, Direction);
 action!(GoToDefinition);
 action!(FindAllReferences);
 action!(Rename);
@@ -136,6 +136,12 @@ action!(OpenExcerpts);
 enum DocumentHighlightRead {}
 enum DocumentHighlightWrite {}
 
+#[derive(Copy, Clone, PartialEq, Eq)]
+pub enum Direction {
+    Prev,
+    Next,
+}
+
 pub fn init(cx: &mut MutableAppContext, path_openers: &mut Vec<Box<dyn PathOpener>>) {
     path_openers.push(Box::new(items::BufferOpener));
     cx.add_bindings(vec![
@@ -250,7 +256,8 @@ pub fn init(cx: &mut MutableAppContext, path_openers: &mut Vec<Box<dyn PathOpene
         Binding::new("ctrl-w", SelectLargerSyntaxNode, Some("Editor")),
         Binding::new("alt-down", SelectSmallerSyntaxNode, Some("Editor")),
         Binding::new("ctrl-shift-W", SelectSmallerSyntaxNode, Some("Editor")),
-        Binding::new("f8", ShowNextDiagnostic, Some("Editor")),
+        Binding::new("f8", GoToDiagnostic(Direction::Next), Some("Editor")),
+        Binding::new("shift-f8", GoToDiagnostic(Direction::Prev), Some("Editor")),
         Binding::new("f2", Rename, Some("Editor")),
         Binding::new("f12", GoToDefinition, Some("Editor")),
         Binding::new("alt-shift-f12", FindAllReferences, Some("Editor")),
@@ -319,7 +326,7 @@ pub fn init(cx: &mut MutableAppContext, path_openers: &mut Vec<Box<dyn PathOpene
     cx.add_action(Editor::select_larger_syntax_node);
     cx.add_action(Editor::select_smaller_syntax_node);
     cx.add_action(Editor::move_to_enclosing_bracket);
-    cx.add_action(Editor::show_next_diagnostic);
+    cx.add_action(Editor::go_to_diagnostic);
     cx.add_action(Editor::go_to_definition);
     cx.add_action(Editor::page_up);
     cx.add_action(Editor::page_down);
@@ -4173,7 +4180,11 @@ impl Editor {
         self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
-    pub fn show_next_diagnostic(&mut self, _: &ShowNextDiagnostic, cx: &mut ViewContext<Self>) {
+    pub fn go_to_diagnostic(
+        &mut self,
+        &GoToDiagnostic(direction): &GoToDiagnostic,
+        cx: &mut ViewContext<Self>,
+    ) {
         let buffer = self.buffer.read(cx).snapshot(cx);
         let selection = self.newest_selection_with_snapshot::<usize>(&buffer);
         let mut active_primary_range = self.active_diagnostics.as_ref().map(|active_diagnostics| {
@@ -4193,20 +4204,23 @@ impl Editor {
         };
 
         loop {
-            let next_group = buffer
-                .diagnostics_in_range::<_, usize>(search_start..buffer.len())
-                .find_map(|entry| {
-                    if entry.diagnostic.is_primary
-                        && !entry.range.is_empty()
-                        && Some(entry.range.end) != active_primary_range.as_ref().map(|r| *r.end())
-                    {
-                        Some((entry.range, entry.diagnostic.group_id))
-                    } else {
-                        None
-                    }
-                });
+            let mut diagnostics = if direction == Direction::Prev {
+                buffer.diagnostics_in_range::<_, usize>(0..search_start, true)
+            } else {
+                buffer.diagnostics_in_range::<_, usize>(search_start..buffer.len(), false)
+            };
+            let group = diagnostics.find_map(|entry| {
+                if entry.diagnostic.is_primary
+                    && !entry.range.is_empty()
+                    && Some(entry.range.end) != active_primary_range.as_ref().map(|r| *r.end())
+                {
+                    Some((entry.range, entry.diagnostic.group_id))
+                } else {
+                    None
+                }
+            });
 
-            if let Some((primary_range, group_id)) = next_group {
+            if let Some((primary_range, group_id)) = group {
                 self.activate_diagnostics(group_id, cx);
                 self.update_selections(
                     vec![Selection {
@@ -4220,13 +4234,23 @@ impl Editor {
                     cx,
                 );
                 break;
-            } else if search_start == 0 {
-                break;
             } else {
                 // Cycle around to the start of the buffer, potentially moving back to the start of
                 // the currently active diagnostic.
-                search_start = 0;
                 active_primary_range.take();
+                if direction == Direction::Prev {
+                    if search_start == buffer.len() {
+                        break;
+                    } else {
+                        search_start = buffer.len();
+                    }
+                } else {
+                    if search_start == 0 {
+                        break;
+                    } else {
+                        search_start = 0;
+                    }
+                }
             }
         }
     }
@@ -4590,7 +4614,7 @@ impl Editor {
             let buffer = self.buffer.read(cx).snapshot(cx);
             let primary_range_start = active_diagnostics.primary_range.start.to_offset(&buffer);
             let is_valid = buffer
-                .diagnostics_in_range::<_, usize>(active_diagnostics.primary_range.clone())
+                .diagnostics_in_range::<_, usize>(active_diagnostics.primary_range.clone(), false)
                 .any(|entry| {
                     entry.diagnostic.is_primary
                         && !entry.range.is_empty()

crates/editor/src/items.rs 🔗

@@ -373,7 +373,7 @@ impl DiagnosticMessage {
             .head();
         let new_diagnostic = buffer
             .read(cx)
-            .diagnostics_in_range::<_, usize>(cursor_position..cursor_position)
+            .diagnostics_in_range::<_, usize>(cursor_position..cursor_position, false)
             .filter(|entry| !entry.range.is_empty())
             .min_by_key(|entry| (entry.diagnostic.severity, entry.range.len()))
             .map(|entry| entry.diagnostic);

crates/editor/src/multi_buffer.rs 🔗

@@ -2179,6 +2179,7 @@ impl MultiBufferSnapshot {
     pub fn diagnostics_in_range<'a, T, O>(
         &'a self,
         range: Range<T>,
+        reversed: bool,
     ) -> impl Iterator<Item = DiagnosticEntry<O>> + 'a
     where
         T: 'a + ToOffset,
@@ -2187,7 +2188,10 @@ impl MultiBufferSnapshot {
         self.as_singleton()
             .into_iter()
             .flat_map(move |(_, _, buffer)| {
-                buffer.diagnostics_in_range(range.start.to_offset(self)..range.end.to_offset(self))
+                buffer.diagnostics_in_range(
+                    range.start.to_offset(self)..range.end.to_offset(self),
+                    reversed,
+                )
             })
     }
 

crates/language/src/buffer.rs 🔗

@@ -1575,7 +1575,7 @@ impl BufferSnapshot {
         let mut diagnostic_endpoints = Vec::new();
         if language_aware {
             tree = self.tree.as_ref();
-            for entry in self.diagnostics_in_range::<_, usize>(range.clone()) {
+            for entry in self.diagnostics_in_range::<_, usize>(range.clone(), false) {
                 diagnostic_endpoints.push(DiagnosticEndpoint {
                     offset: entry.range.start,
                     is_start: true,
@@ -1838,12 +1838,14 @@ impl BufferSnapshot {
     pub fn diagnostics_in_range<'a, T, O>(
         &'a self,
         search_range: Range<T>,
+        reversed: bool,
     ) -> impl 'a + Iterator<Item = DiagnosticEntry<O>>
     where
         T: 'a + Clone + ToOffset,
         O: 'a + FromAnchor,
     {
-        self.diagnostics.range(search_range.clone(), self, true)
+        self.diagnostics
+            .range(search_range.clone(), self, true, reversed)
     }
 
     pub fn diagnostic_groups(&self) -> Vec<DiagnosticGroup<Anchor>> {

crates/language/src/diagnostic_set.rs 🔗

@@ -71,6 +71,7 @@ impl DiagnosticSet {
         range: Range<T>,
         buffer: &'a text::BufferSnapshot,
         inclusive: bool,
+        reversed: bool,
     ) -> impl 'a + Iterator<Item = DiagnosticEntry<O>>
     where
         T: 'a + ToOffset,
@@ -90,11 +91,19 @@ impl DiagnosticSet {
             }
         });
 
-        cursor.next(buffer);
+        if reversed {
+            cursor.prev(buffer);
+        } else {
+            cursor.next(buffer);
+        }
         iter::from_fn({
             move || {
                 if let Some(diagnostic) = cursor.item() {
-                    cursor.next(buffer);
+                    if reversed {
+                        cursor.prev(buffer);
+                    } else {
+                        cursor.next(buffer);
+                    }
                     Some(diagnostic.resolve(buffer))
                 } else {
                     None

crates/language/src/tests.rs 🔗

@@ -768,10 +768,10 @@ fn test_random_collaboration(cx: &mut MutableAppContext, mut rng: StdRng) {
         );
         assert_eq!(
             buffer
-                .diagnostics_in_range::<_, usize>(0..buffer.len())
+                .diagnostics_in_range::<_, usize>(0..buffer.len(), false)
                 .collect::<Vec<_>>(),
             first_buffer
-                .diagnostics_in_range::<_, usize>(0..first_buffer.len())
+                .diagnostics_in_range::<_, usize>(0..first_buffer.len(), false)
                 .collect::<Vec<_>>(),
             "Replica {} diagnostics != Replica 0 diagnostics",
             buffer.replica_id()

crates/project/src/project.rs 🔗

@@ -4869,7 +4869,7 @@ mod tests {
         buffer.read_with(cx, |buffer, _| {
             let snapshot = buffer.snapshot();
             let diagnostics = snapshot
-                .diagnostics_in_range::<_, Point>(0..buffer.len())
+                .diagnostics_in_range::<_, Point>(0..buffer.len(), false)
                 .collect::<Vec<_>>();
             assert_eq!(
                 diagnostics,
@@ -4985,7 +4985,7 @@ mod tests {
             assert_eq!(
                 buffer
                     .snapshot()
-                    .diagnostics_in_range::<_, Point>(Point::new(3, 0)..Point::new(5, 0))
+                    .diagnostics_in_range::<_, Point>(Point::new(3, 0)..Point::new(5, 0), false)
                     .collect::<Vec<_>>(),
                 &[
                     DiagnosticEntry {
@@ -5063,7 +5063,7 @@ mod tests {
             assert_eq!(
                 buffer
                     .snapshot()
-                    .diagnostics_in_range::<_, Point>(Point::new(2, 0)..Point::new(3, 0))
+                    .diagnostics_in_range::<_, Point>(Point::new(2, 0)..Point::new(3, 0), false)
                     .collect::<Vec<_>>(),
                 &[
                     DiagnosticEntry {
@@ -5150,7 +5150,7 @@ mod tests {
             assert_eq!(
                 buffer
                     .snapshot()
-                    .diagnostics_in_range::<_, Point>(0..buffer.len())
+                    .diagnostics_in_range::<_, Point>(0..buffer.len(), false)
                     .collect::<Vec<_>>(),
                 &[
                     DiagnosticEntry {
@@ -6428,7 +6428,7 @@ mod tests {
 
         assert_eq!(
             buffer
-                .diagnostics_in_range::<_, Point>(0..buffer.len())
+                .diagnostics_in_range::<_, Point>(0..buffer.len(), false)
                 .collect::<Vec<_>>(),
             &[
                 DiagnosticEntry {

crates/server/src/rpc.rs 🔗

@@ -2061,7 +2061,7 @@ mod tests {
             assert_eq!(
                 buffer
                     .snapshot()
-                    .diagnostics_in_range::<_, Point>(0..buffer.len())
+                    .diagnostics_in_range::<_, Point>(0..buffer.len(), false)
                     .map(|entry| entry)
                     .collect::<Vec<_>>(),
                 &[