From 3d9f0087ff68218c3bb679e3a5fd6fc3fcebf359 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 8 May 2024 00:37:09 +0300 Subject: [PATCH] Do not show diffs for files with \r\n contents (#11519) --- crates/editor/src/editor_tests.rs | 33 ++++++++++++++++--- crates/editor/src/git.rs | 6 ++-- crates/editor/src/test/editor_test_context.rs | 4 +-- crates/language/src/buffer.rs | 27 ++++++++------- crates/project/src/project.rs | 15 ++++----- crates/worktree/src/worktree.rs | 4 +-- 6 files changed, 56 insertions(+), 33 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 690713f89e..be2f8d68fa 100644 --- a/crates/editor/src/editor_tests.rs +++ b/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::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::(), 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 }); diff --git a/crates/editor/src/git.rs b/crates/editor/src/git.rs index 3712b4cf4c..7b43b4c230 100644 --- a/crates/editor/src/git.rs +++ b/crates/editor/src/git.rs @@ -145,8 +145,7 @@ mod tests { 1.five 1.six " - .unindent() - .into(), + .unindent(), ), cx, ); @@ -182,8 +181,7 @@ mod tests { 2.four 2.six " - .unindent() - .into(), + .unindent(), ), cx, ); diff --git a/crates/editor/src/test/editor_test_context.rs b/crates/editor/src/test/editor_test_context.rs index 8ef5480563..8d6ddc0011 100644 --- a/crates/editor/src/test/editor_test_context.rs +++ b/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, cx)); + self.update_buffer(|buffer, cx| buffer.set_diff_base(diff_base.map(ToOwned::to_owned), cx)); } /// Change the editor's text and selections using a string containing diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 49b2f6a30f..87e042c13c 100644 --- a/crates/language/src/buffer.rs +++ b/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( - buffer, - message.diff_base.map(|text| text.into()), - file, - capability, - ); + let mut this = Self::build(buffer, message.diff_base, 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, + diff_base: Option, file: Option>, 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, cx: &mut ModelContext) { - self.diff_base = diff_base; + pub fn set_diff_base(&mut self, diff_base: Option, cx: &mut ModelContext) { + 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) -> Option> { - 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(); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 40f73948d7..e162f8c325 100644 --- a/crates/project/src/project.rs +++ b/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() - .lock() - .load_index_text(relative_path) - .map(Rope::from) + repo_entry.repo().lock().load_index_text(relative_path) }; 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(()) })? diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 077f811ede..4711aed9b5 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -1069,7 +1069,7 @@ impl LocalWorktree { &self, path: &Path, cx: &mut ModelContext, - ) -> Task)>> { + ) -> Task)>> { 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) } } }));