Do not show diffs for files with \r\n contents (#11519)

crates/editor/src/editor_tests.rs

@@ -2826,6 +2826,31 @@ async fn test_join_lines_with_git_diff_base(
     );
 }
 
+#[gpui::test]
+async fn test_custom_newlines_cause_no_false_positive_diffs(
+    executor: BackgroundExecutor,
+    cx: &mut gpui::TestAppContext,
+) {
+    init_test(cx, |_| {});
+    let mut cx = EditorTestContext::new(cx).await;
+    cx.set_state("Line 0\r\nLine 1\rˇ\nLine 2\r\nLine 3");
+    cx.set_diff_base(Some("Line 0\r\nLine 1\r\nLine 2\r\nLine 3"));
+    executor.run_until_parked();
+
+    cx.update_editor(|editor, cx| {
+        assert_eq!(
+            editor
+                .buffer()
+                .read(cx)
+                .snapshot(cx)
+                .git_diff_hunks_in_range(0..u32::MAX)
+                .collect::<Vec<_>>(),
+            Vec::new(),
+            "Should not have any diffs for files with custom newlines"
+        );
+    });
+}
+
 #[gpui::test]
 async fn test_manipulate_lines_with_single_selection(cx: &mut TestAppContext) {
     init_test(cx, |_| {});
@@ -9026,7 +9051,7 @@ async fn test_multibuffer_reverts(cx: &mut gpui::TestAppContext) {
                     .collect::<String>(),
                 cx,
             );
-            buffer.set_diff_base(Some(sample_text.into()), cx);
+            buffer.set_diff_base(Some(sample_text), cx);
         });
         cx.executor().run_until_parked();
     }
@@ -10041,17 +10066,17 @@ async fn test_toggle_diff_expand_in_multi_buffer(cx: &mut gpui::TestAppContext)
         "vvvv\nwwww\nxxxx\nyyyy\nzzzz\n@@@@\n{{{{\n||||\n}}}}\n~~~~\n\u{7f}\u{7f}\u{7f}\u{7f}";
     let buffer_1 = cx.new_model(|cx| {
         let mut buffer = Buffer::local(modified_sample_text_1.to_string(), cx);
-        buffer.set_diff_base(Some(sample_text_1.clone().into()), cx);
+        buffer.set_diff_base(Some(sample_text_1.clone()), cx);
         buffer
     });
     let buffer_2 = cx.new_model(|cx| {
         let mut buffer = Buffer::local(modified_sample_text_2.to_string(), cx);
-        buffer.set_diff_base(Some(sample_text_2.clone().into()), cx);
+        buffer.set_diff_base(Some(sample_text_2.clone()), cx);
         buffer
     });
     let buffer_3 = cx.new_model(|cx| {
         let mut buffer = Buffer::local(modified_sample_text_3.to_string(), cx);
-        buffer.set_diff_base(Some(sample_text_3.clone().into()), cx);
+        buffer.set_diff_base(Some(sample_text_3.clone()), cx);
         buffer
     });
 

crates/editor/src/git.rs

@@ -145,8 +145,7 @@ mod tests {
                         1.five
                         1.six
                     "
-                    .unindent()
+                    .unindent(),
-                    .into(),
                 ),
                 cx,
             );
@@ -182,8 +181,7 @@ mod tests {
                         2.four
                         2.six
                     "
-                    .unindent()
+                    .unindent(),
-                    .into(),
                 ),
                 cx,
             );

crates/editor/src/test/editor_test_context.rs

@@ -21,7 +21,6 @@ use std::{
         Arc,
     },
 };
-use text::Rope;
 use ui::Context;
 use util::{
     assert_set_eq,
@@ -272,8 +271,7 @@ impl EditorTestContext {
     }
 
     pub fn set_diff_base(&mut self, diff_base: Option<&str>) {
-        let diff_base = diff_base.map(Rope::from);
+        self.update_buffer(|buffer, cx| buffer.set_diff_base(diff_base.map(ToOwned::to_owned), cx));
-        self.update_buffer(|buffer, cx| buffer.set_diff_base(diff_base, cx));
     }
 
     /// Change the editor's text and selections using a string containing

crates/language/src/buffer.rs

@@ -564,12 +564,7 @@ impl Buffer {
         let buffer_id = BufferId::new(message.id)
             .with_context(|| anyhow!("Could not deserialize buffer_id"))?;
         let buffer = TextBuffer::new(replica_id, buffer_id, message.base_text);
-        let mut this = Self::build(
+        let mut this = Self::build(buffer, message.diff_base, file, capability);
-            buffer,
-            message.diff_base.map(|text| text.into()),
-            file,
-            capability,
-        );
         this.text.set_line_ending(proto::deserialize_line_ending(
             rpc::proto::LineEnding::from_i32(message.line_ending)
                 .ok_or_else(|| anyhow!("missing line_ending"))?,
@@ -658,7 +653,7 @@ impl Buffer {
     /// Builds a [Buffer] with the given underlying [TextBuffer], diff base, [File] and [Capability].
     pub fn build(
         buffer: TextBuffer,
-        diff_base: Option<Rope>,
+        diff_base: Option<String>,
         file: Option<Arc<dyn File>>,
         capability: Capability,
     ) -> Self {
@@ -671,7 +666,12 @@ impl Buffer {
             transaction_depth: 0,
             was_dirty_before_starting_transaction: None,
             text: buffer,
-            diff_base,
+            diff_base: diff_base
+                .map(|mut raw_diff_base| {
+                    LineEnding::normalize(&mut raw_diff_base);
+                    raw_diff_base
+                })
+                .map(Rope::from),
             diff_base_version: 0,
             git_diff: git::diff::BufferDiff::new(),
             file,
@@ -900,8 +900,13 @@ impl Buffer {
 
     /// Sets the text that will be used to compute a Git diff
     /// against the buffer text.
-    pub fn set_diff_base(&mut self, diff_base: Option<Rope>, cx: &mut ModelContext<Self>) {
+    pub fn set_diff_base(&mut self, diff_base: Option<String>, cx: &mut ModelContext<Self>) {
-        self.diff_base = diff_base;
+        self.diff_base = diff_base
+            .map(|mut raw_diff_base| {
+                LineEnding::normalize(&mut raw_diff_base);
+                raw_diff_base
+            })
+            .map(Rope::from);
         self.diff_base_version += 1;
         if let Some(recalc_task) = self.git_diff_recalc(cx) {
             cx.spawn(|buffer, mut cx| async move {
@@ -923,7 +928,7 @@ impl Buffer {
 
     /// Recomputes the Git diff status.
     pub fn git_diff_recalc(&mut self, cx: &mut ModelContext<Self>) -> Option<Task<()>> {
-        let diff_base = self.diff_base.clone()?; // TODO: Make this an Arc
+        let diff_base = self.diff_base.clone()?;
         let snapshot = self.snapshot();
 
         let mut diff = self.git_diff.clone();

crates/project/src/project.rs

@@ -97,7 +97,7 @@ use std::{
 };
 use task::static_source::{StaticSource, TrackedFile};
 use terminals::Terminals;
-use text::{Anchor, BufferId, LineEnding, Rope};
+use text::{Anchor, BufferId, LineEnding};
 use util::{
     debug_panic, defer,
     http::{HttpClient, Url},
@@ -7559,11 +7559,7 @@ impl Project {
                                     None
                                 } else {
                                     let relative_path = path.strip_prefix(&work_directory).ok()?;
-                                    repo_entry
+                                    repo_entry.repo().lock().load_index_text(relative_path)
-                                        .repo()
-                                        .lock()
-                                        .load_index_text(relative_path)
-                                        .map(Rope::from)
                                 };
                                 Some((buffer, base_text))
                             }
@@ -7591,7 +7587,7 @@ impl Project {
                         .send(proto::UpdateDiffBase {
                             project_id,
                             buffer_id,
-                            diff_base: diff_base.map(|rope| rope.to_string()),
+                            diff_base,
                         })
                         .log_err();
                 }
@@ -8666,14 +8662,15 @@ impl Project {
         this.update(&mut cx, |this, cx| {
             let buffer_id = envelope.payload.buffer_id;
             let buffer_id = BufferId::new(buffer_id)?;
-            let diff_base = envelope.payload.diff_base.map(Rope::from);
             if let Some(buffer) = this
                 .opened_buffers
                 .get_mut(&buffer_id)
                 .and_then(|b| b.upgrade())
                 .or_else(|| this.incomplete_remote_buffers.get(&buffer_id).cloned())
             {
-                buffer.update(cx, |buffer, cx| buffer.set_diff_base(diff_base, cx));
+                buffer.update(cx, |buffer, cx| {
+                    buffer.set_diff_base(envelope.payload.diff_base, cx)
+                });
             }
             Ok(())
         })?

crates/worktree/src/worktree.rs

@@ -1069,7 +1069,7 @@ impl LocalWorktree {
         &self,
         path: &Path,
         cx: &mut ModelContext<Worktree>,
-    ) -> Task<Result<(File, String, Option<Rope>)>> {
+    ) -> Task<Result<(File, String, Option<String>)>> {
         let path = Arc::from(path);
         let abs_path = self.absolutize(&path);
         let fs = self.fs.clone();
@@ -1100,7 +1100,7 @@ impl LocalWorktree {
                                 if abs_path_metadata.is_dir || abs_path_metadata.is_symlink {
                                     None
                                 } else {
-                                    git_repo.lock().load_index_text(&repo_path).map(Rope::from)
+                                    git_repo.lock().load_index_text(&repo_path)
                                 }
                             }
                         }));