F8 navigation bugs (#3163)

Release Notes:

- Fixed "go to previous hunk" getting suck on deletion.
- Fixed a rare case where "go to (prev) diagnostic" could get stuck with
specifically overlapping diagnostics.

crates/editor/src/editor.rs

@@ -7213,6 +7213,7 @@ impl Editor {
                     && entry.diagnostic.severity <= DiagnosticSeverity::WARNING
                     && !entry.range.is_empty()
                     && Some(entry.range.end) != active_primary_range.as_ref().map(|r| *r.end())
+                    && !entry.range.contains(&search_start)
                 {
                     Some((entry.range, entry.diagnostic.group_id))
                 } else {
@@ -7319,11 +7320,11 @@ impl Editor {
         let display_point = initial_point.to_display_point(snapshot);
         let mut hunks = hunks
             .map(|hunk| diff_hunk_to_display(hunk, &snapshot))
-            .skip_while(|hunk| {
+            .filter(|hunk| {
                 if is_wrapped {
-                    false
+                    true
                 } else {
-                    hunk.contains_display_row(display_point.row())
+                    !hunk.contains_display_row(display_point.row())
                 }
             })
             .dedup();
@@ -8957,6 +8958,16 @@ impl Editor {
         telemetry.report_clickhouse_event(event, telemetry_settings);
     }
 
+    #[cfg(any(test, feature = "test-support"))]
+    fn report_editor_event(
+        &self,
+        _operation: &'static str,
+        _file_extension: Option<String>,
+        _cx: &AppContext,
+    ) {
+    }
+
+    #[cfg(not(any(test, feature = "test-support")))]
     fn report_editor_event(
         &self,
         operation: &'static str,

crates/editor/src/editor_tests.rs

@@ -6717,6 +6717,102 @@ fn test_combine_syntax_and_fuzzy_match_highlights() {
     );
 }
 
+#[gpui::test]
+async fn go_to_prev_overlapping_diagnostic(
+    deterministic: Arc<Deterministic>,
+    cx: &mut gpui::TestAppContext,
+) {
+    init_test(cx, |_| {});
+
+    let mut cx = EditorTestContext::new(cx).await;
+    let project = cx.update_editor(|editor, _| editor.project.clone().unwrap());
+
+    cx.set_state(indoc! {"
+        ˇfn func(abc def: i32) -> u32 {
+        }
+    "});
+
+    cx.update(|cx| {
+        project.update(cx, |project, cx| {
+            project
+                .update_diagnostics(
+                    LanguageServerId(0),
+                    lsp::PublishDiagnosticsParams {
+                        uri: lsp::Url::from_file_path("/root/file").unwrap(),
+                        version: None,
+                        diagnostics: vec![
+                            lsp::Diagnostic {
+                                range: lsp::Range::new(
+                                    lsp::Position::new(0, 11),
+                                    lsp::Position::new(0, 12),
+                                ),
+                                severity: Some(lsp::DiagnosticSeverity::ERROR),
+                                ..Default::default()
+                            },
+                            lsp::Diagnostic {
+                                range: lsp::Range::new(
+                                    lsp::Position::new(0, 12),
+                                    lsp::Position::new(0, 15),
+                                ),
+                                severity: Some(lsp::DiagnosticSeverity::ERROR),
+                                ..Default::default()
+                            },
+                            lsp::Diagnostic {
+                                range: lsp::Range::new(
+                                    lsp::Position::new(0, 25),
+                                    lsp::Position::new(0, 28),
+                                ),
+                                severity: Some(lsp::DiagnosticSeverity::ERROR),
+                                ..Default::default()
+                            },
+                        ],
+                    },
+                    &[],
+                    cx,
+                )
+                .unwrap()
+        });
+    });
+
+    deterministic.run_until_parked();
+
+    cx.update_editor(|editor, cx| {
+        editor.go_to_prev_diagnostic(&GoToPrevDiagnostic, cx);
+    });
+
+    cx.assert_editor_state(indoc! {"
+        fn func(abc def: i32) -> ˇu32 {
+        }
+    "});
+
+    cx.update_editor(|editor, cx| {
+        editor.go_to_prev_diagnostic(&GoToPrevDiagnostic, cx);
+    });
+
+    cx.assert_editor_state(indoc! {"
+        fn func(abc ˇdef: i32) -> u32 {
+        }
+    "});
+
+    cx.update_editor(|editor, cx| {
+        editor.go_to_prev_diagnostic(&GoToPrevDiagnostic, cx);
+    });
+
+    cx.assert_editor_state(indoc! {"
+        fn func(abcˇ def: i32) -> u32 {
+        }
+    "});
+
+    cx.update_editor(|editor, cx| {
+        editor.go_to_prev_diagnostic(&GoToPrevDiagnostic, cx);
+    });
+
+    cx.assert_editor_state(indoc! {"
+        fn func(abc def: i32) -> ˇu32 {
+        }
+    "});
+}
+
 #[gpui::test]
 async fn go_to_hunk(deterministic: Arc<Deterministic>, cx: &mut gpui::TestAppContext) {
     init_test(cx, |_| {});
@@ -6799,6 +6895,46 @@ async fn go_to_hunk(deterministic: Arc<Deterministic>, cx: &mut gpui::TestAppCon
         .unindent(),
     );
 
+    cx.update_editor(|editor, cx| {
+        editor.go_to_prev_hunk(&GoToPrevHunk, cx);
+    });
+
+    cx.assert_editor_state(
+        &r#"
+        use some::modified;
+
+        ˇ
+        fn main() {
+            println!("hello there");
+
+            println!("around the");
+            println!("world");
+        }
+        "#
+        .unindent(),
+    );
+
+    cx.update_editor(|editor, cx| {
+        for _ in 0..3 {
+            editor.go_to_prev_hunk(&GoToPrevHunk, cx);
+        }
+    });
+
+    cx.assert_editor_state(
+        &r#"
+        use some::modified;
+
+
+        fn main() {
+        ˇ    println!("hello there");
+
+            println!("around the");
+            println!("world");
+        }
+        "#
+        .unindent(),
+    );
+
     cx.update_editor(|editor, cx| {
         editor.fold(&Fold, cx);
 

crates/editor/src/git.rs

@@ -36,7 +36,7 @@ impl DisplayDiffHunk {
 
             DisplayDiffHunk::Unfolded {
                 display_row_range, ..
-            } => display_row_range.start..=display_row_range.end - 1,
+            } => display_row_range.start..=display_row_range.end,
         };
 
         range.contains(&display_row)
@@ -77,8 +77,8 @@ pub fn diff_hunk_to_display(hunk: DiffHunk<u32>, snapshot: &DisplaySnapshot) ->
     } else {
         let start = hunk_start_point.to_display_point(snapshot).row();
 
-        let hunk_end_row_inclusive = hunk.buffer_range.end.max(hunk.buffer_range.start);
+        let hunk_end_row = hunk.buffer_range.end.max(hunk.buffer_range.start);
-        let hunk_end_point = Point::new(hunk_end_row_inclusive, 0);
+        let hunk_end_point = Point::new(hunk_end_row, 0);
         let end = hunk_end_point.to_display_point(snapshot).row();
 
         DisplayDiffHunk::Unfolded {

crates/editor/src/test.rs

@@ -8,6 +8,7 @@ use crate::{
 
 use gpui::{ModelHandle, ViewContext};
 
+use project::Project;
 use util::test::{marked_text_offsets, marked_text_ranges};
 
 #[cfg(test)]
@@ -63,9 +64,20 @@ pub fn assert_text_with_selections(
     assert_eq!(editor.selections.ranges(cx), text_ranges);
 }
 
+// RA thinks this is dead code even though it is used in a whole lot of tests
+#[allow(dead_code)]
+#[cfg(any(test, feature = "test-support"))]
 pub(crate) fn build_editor(
     buffer: ModelHandle<MultiBuffer>,
     cx: &mut ViewContext<Editor>,
 ) -> Editor {
     Editor::new(EditorMode::Full, buffer, None, None, cx)
 }
+
+pub(crate) fn build_editor_with_project(
+    project: ModelHandle<Project>,
+    buffer: ModelHandle<MultiBuffer>,
+    cx: &mut ViewContext<Editor>,
+) -> Editor {
+    Editor::new(EditorMode::Full, buffer, Some(project), None, cx)
+}

crates/editor/src/test/editor_test_context.rs

@@ -18,7 +18,7 @@ use util::{
     test::{generate_marked_text, marked_text_ranges},
 };
 
-use super::build_editor;
+use super::build_editor_with_project;
 
 pub struct EditorTestContext<'a> {
     pub cx: &'a mut gpui::TestAppContext,
@@ -29,13 +29,24 @@ pub struct EditorTestContext<'a> {
 impl<'a> EditorTestContext<'a> {
     pub async fn new(cx: &'a mut gpui::TestAppContext) -> EditorTestContext<'a> {
         let fs = FakeFs::new(cx.background());
-        let project = Project::test(fs, [], cx).await;
+        // fs.insert_file("/file", "".to_owned()).await;
+        fs.insert_tree(
+            "/root",
+            gpui::serde_json::json!({
+                "file": "",
+            }),
+        )
+        .await;
+        let project = Project::test(fs, ["/root".as_ref()], cx).await;
         let buffer = project
-            .update(cx, |project, cx| project.create_buffer("", None, cx))
+            .update(cx, |project, cx| {
+                project.open_local_buffer("/root/file", cx)
+            })
+            .await
             .unwrap();
         let window = cx.add_window(|cx| {
             cx.focus_self();
-            build_editor(MultiBuffer::build_from_buffer(buffer, cx), cx)
+            build_editor_with_project(project, MultiBuffer::build_from_buffer(buffer, cx), cx)
         });
         let editor = window.root(cx);
         Self {