From 87b3fefdd13915d9697f4d17c9e403af72158d54 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Wed, 5 Mar 2025 13:07:36 -0500 Subject: [PATCH] Fix panic when expanding a deletion hunk with blame open (#26130) Closes #26118 Release Notes: - Fixed a panic when expanding diff hunks while git blame is open --- crates/collab/src/tests/editor_tests.rs | 12 +++++++ crates/editor/src/git/blame.rs | 42 ++++++++++++++++++++----- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/crates/collab/src/tests/editor_tests.rs b/crates/collab/src/tests/editor_tests.rs index b0345241298642c2f03055385710a38ec0e7d586..1bc0813e39307e9e30e55dc8340e9c84cfa9a79f 100644 --- a/crates/collab/src/tests/editor_tests.rs +++ b/crates/collab/src/tests/editor_tests.rs @@ -2027,6 +2027,15 @@ async fn test_git_blame_is_forwarded(cx_a: &mut TestAppContext, cx_b: &mut TestA .unwrap() .downcast::() .unwrap(); + let buffer_id_b = editor_b.update(cx_b, |editor_b, cx| { + editor_b + .buffer() + .read(cx) + .as_singleton() + .unwrap() + .read(cx) + .remote_id() + }); // client_b now requests git blame for the open buffer editor_b.update_in(cx_b, |editor_b, window, cx| { @@ -2045,6 +2054,7 @@ async fn test_git_blame_is_forwarded(cx_a: &mut TestAppContext, cx_b: &mut TestA &(0..4) .map(|row| RowInfo { buffer_row: Some(row), + buffer_id: Some(buffer_id_b), ..Default::default() }) .collect::>(), @@ -2092,6 +2102,7 @@ async fn test_git_blame_is_forwarded(cx_a: &mut TestAppContext, cx_b: &mut TestA &(0..4) .map(|row| RowInfo { buffer_row: Some(row), + buffer_id: Some(buffer_id_b), ..Default::default() }) .collect::>(), @@ -2127,6 +2138,7 @@ async fn test_git_blame_is_forwarded(cx_a: &mut TestAppContext, cx_b: &mut TestA &(0..4) .map(|row| RowInfo { buffer_row: Some(row), + buffer_id: Some(buffer_id_b), ..Default::default() }) .collect::>(), diff --git a/crates/editor/src/git/blame.rs b/crates/editor/src/git/blame.rs index 0ef829786fc42871253eb9a752d95a1d049b1dff..58681336425a52ab176979683b6598fb7781c9ec 100644 --- a/crates/editor/src/git/blame.rs +++ b/crates/editor/src/git/blame.rs @@ -195,9 +195,12 @@ impl GitBlame { ) -> impl 'a + Iterator> { self.sync(cx); + let buffer_id = self.buffer_snapshot.remote_id(); let mut cursor = self.entries.cursor::(&()); rows.into_iter().map(move |info| { - let row = info.buffer_row?; + let row = info + .buffer_row + .filter(|_| info.buffer_id == Some(buffer_id))?; cursor.seek_forward(&row, Bias::Right, &()); cursor.item()?.blame.clone() }) @@ -535,6 +538,7 @@ mod tests { use serde_json::json; use settings::SettingsStore; use std::{cmp, env, ops::Range, path::Path}; + use text::BufferId; use unindent::Unindent as _; use util::{path, RandomCharIter}; @@ -552,16 +556,18 @@ mod tests { #[track_caller] fn assert_blame_rows( blame: &mut GitBlame, + buffer_id: BufferId, rows: Range, expected: Vec>, cx: &mut Context, ) { - assert_eq!( + pretty_assertions::assert_eq!( blame .blame_for_rows( &rows .map(|row| RowInfo { buffer_row: Some(row), + buffer_id: Some(buffer_id), ..Default::default() }) .collect::>(), @@ -694,6 +700,7 @@ mod tests { }) .await .unwrap(); + let buffer_id = buffer.update(cx, |buffer, _| buffer.remote_id()); let git_blame = cx.new(|cx| GitBlame::new(buffer.clone(), project, false, true, cx)); @@ -701,12 +708,13 @@ mod tests { git_blame.update(cx, |blame, cx| { // All lines - assert_eq!( + pretty_assertions::assert_eq!( blame .blame_for_rows( &(0..8) .map(|buffer_row| RowInfo { buffer_row: Some(buffer_row), + buffer_id: Some(buffer_id), ..Default::default() }) .collect::>(), @@ -725,12 +733,13 @@ mod tests { ] ); // Subset of lines - assert_eq!( + pretty_assertions::assert_eq!( blame .blame_for_rows( &(1..4) .map(|buffer_row| RowInfo { buffer_row: Some(buffer_row), + buffer_id: Some(buffer_id), ..Default::default() }) .collect::>(), @@ -744,12 +753,13 @@ mod tests { ] ); // Subset of lines, with some not displayed - assert_eq!( + pretty_assertions::assert_eq!( blame .blame_for_rows( &[ RowInfo { buffer_row: Some(1), + buffer_id: Some(buffer_id), ..Default::default() }, Default::default(), @@ -800,6 +810,7 @@ mod tests { }) .await .unwrap(); + let buffer_id = buffer.update(cx, |buffer, _| buffer.remote_id()); let git_blame = cx.new(|cx| GitBlame::new(buffer.clone(), project, false, true, cx)); @@ -810,6 +821,7 @@ mod tests { // lines. assert_blame_rows( blame, + buffer_id, 0..4, vec![ Some(blame_entry("1b1b1b", 0..4)), @@ -828,6 +840,7 @@ mod tests { git_blame.update(cx, |blame, cx| { assert_blame_rows( blame, + buffer_id, 0..2, vec![None, Some(blame_entry("1b1b1b", 0..4))], cx, @@ -840,6 +853,7 @@ mod tests { git_blame.update(cx, |blame, cx| { assert_blame_rows( blame, + buffer_id, 1..4, vec![ None, @@ -852,7 +866,13 @@ mod tests { // Before we insert a newline at the end, sanity check: git_blame.update(cx, |blame, cx| { - assert_blame_rows(blame, 3..4, vec![Some(blame_entry("1b1b1b", 0..4))], cx); + assert_blame_rows( + blame, + buffer_id, + 3..4, + vec![Some(blame_entry("1b1b1b", 0..4))], + cx, + ); }); // Insert a newline at the end buffer.update(cx, |buffer, cx| { @@ -862,6 +882,7 @@ mod tests { git_blame.update(cx, |blame, cx| { assert_blame_rows( blame, + buffer_id, 3..5, vec![Some(blame_entry("1b1b1b", 0..4)), None], cx, @@ -870,7 +891,13 @@ mod tests { // Before we insert a newline at the start, sanity check: git_blame.update(cx, |blame, cx| { - assert_blame_rows(blame, 2..3, vec![Some(blame_entry("1b1b1b", 0..4))], cx); + assert_blame_rows( + blame, + buffer_id, + 2..3, + vec![Some(blame_entry("1b1b1b", 0..4))], + cx, + ); }); // Usage example @@ -882,6 +909,7 @@ mod tests { git_blame.update(cx, |blame, cx| { assert_blame_rows( blame, + buffer_id, 2..4, vec![None, Some(blame_entry("1b1b1b", 0..4))], cx,