From e680dfb0a00ab144995e696ad3c59477dc483c00 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 6 Oct 2025 17:52:23 -0400 Subject: [PATCH] git_ui: Update project diff more aggressively (#39642) This fixes a regression in #39557--for the project diff, we rely on getting an event when a path inside a git repository changes, even if the git state of the repository didn't change as a result (e.g. a new modification to a file that already had the "modified" status). I've also changed this code to send the `UpdateRepository` proto message even when the git state didn't change, since otherwise we have the same problem in SSH and collab projects. Release Notes: - N/A --- crates/git_ui/src/project_diff.rs | 131 +++++++++++++++++++++- crates/project/src/git_store.rs | 18 +-- crates/project/src/project_tests.rs | 24 +++- crates/project_panel/src/project_panel.rs | 4 +- 4 files changed, 159 insertions(+), 18 deletions(-) diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index e0070ef0f16443a70a9a6407a5ff1cde20ec45fe..0e59d25222b51d9df5afb1f40967b42b7e53cb20 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -27,7 +27,7 @@ use language::{Anchor, Buffer, Capability, OffsetRangeExt}; use multi_buffer::{MultiBuffer, PathKey}; use project::{ Project, ProjectPath, - git_store::{GitStore, GitStoreEvent, RepositoryEvent}, + git_store::{GitStore, GitStoreEvent}, }; use settings::{Settings, SettingsStore}; use std::any::{Any, TypeId}; @@ -177,7 +177,7 @@ impl ProjectDiff { window, move |this, _git_store, event, _window, _cx| match event { GitStoreEvent::ActiveRepositoryChanged(_) - | GitStoreEvent::RepositoryUpdated(_, RepositoryEvent::Updated { .. }, true) + | GitStoreEvent::RepositoryUpdated(_, _, true) | GitStoreEvent::ConflictsUpdated => { *this.update_needed.borrow_mut() = (); } @@ -1346,7 +1346,6 @@ fn merge_anchor_ranges<'a>( }) } -#[cfg(not(target_os = "windows"))] #[cfg(test)] mod tests { use db::indoc; @@ -1624,6 +1623,7 @@ mod tests { project_diff::{self, ProjectDiff}, }; + #[cfg_attr(windows, ignore = "currently fails on windows")] #[gpui::test] async fn test_go_to_prev_hunk_multibuffer(cx: &mut TestAppContext) { init_test(cx); @@ -1711,6 +1711,7 @@ mod tests { )); } + #[cfg_attr(windows, ignore = "currently fails on windows")] #[gpui::test] async fn test_excerpts_splitting_after_restoring_the_middle_excerpt(cx: &mut TestAppContext) { init_test(cx); @@ -1869,4 +1870,128 @@ mod tests { let contents = String::from_utf8(contents).unwrap(); assert_eq!(contents, "ours\n"); } + + #[gpui::test] + async fn test_new_hunk_in_modified_file(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/project"), + json!({ + ".git": {}, + "foo.txt": " + one + two + three + four + five + six + seven + eight + nine + ten + ELEVEN + twelve + ".unindent() + }), + ) + .await; + let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx)); + let diff = cx.new_window_entity(|window, cx| { + ProjectDiff::new(project.clone(), workspace, window, cx) + }); + cx.run_until_parked(); + + fs.set_head_and_index_for_repo( + Path::new(path!("/project/.git")), + &[( + "foo.txt", + " + one + two + three + four + five + six + seven + eight + nine + ten + eleven + twelve + " + .unindent(), + )], + ); + cx.run_until_parked(); + + let editor = diff.read_with(cx, |diff, _| diff.editor.clone()); + assert_state_with_diff( + &editor, + cx, + &" + ˇnine + ten + - eleven + + ELEVEN + twelve + " + .unindent(), + ); + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/project/foo.txt"), cx) + }) + .await + .unwrap(); + buffer.update(cx, |buffer, cx| { + buffer.edit_via_marked_text( + &" + one + «TWO» + three + four + five + six + seven + eight + nine + ten + ELEVEN + twelve + " + .unindent(), + None, + cx, + ); + }); + project + .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx)) + .await + .unwrap(); + cx.run_until_parked(); + + assert_state_with_diff( + &editor, + cx, + &" + one + - two + + TWO + three + four + five + ˇnine + ten + - eleven + + ELEVEN + twelve + " + .unindent(), + ); + } } diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 2e411d557c12ba3437bca03a893aede10f9dd03a..eb0ae8e5d53b943b2e1a81c5a7b720b2b61fa550 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -302,6 +302,7 @@ pub enum RepositoryState { pub enum RepositoryEvent { Updated { full_scan: bool, new_instance: bool }, MergeHeadsChanged, + PathsChanged, } #[derive(Clone, Debug)] @@ -4840,19 +4841,20 @@ impl Repository { } if needs_update { - if let Some(updates_tx) = updates_tx { - updates_tx - .unbounded_send(DownstreamUpdate::UpdateRepository( - this.snapshot.clone(), - )) - .ok(); - } - cx.emit(RepositoryEvent::Updated { full_scan: false, new_instance: false, }); } + + if let Some(updates_tx) = updates_tx { + updates_tx + .unbounded_send(DownstreamUpdate::UpdateRepository( + this.snapshot.clone(), + )) + .ok(); + } + cx.emit(RepositoryEvent::PathsChanged); }) }, ); diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index c65a829a972edeb271f2897f859efbbc17c530f4..d6711cd5b54f5bbdb4dd9777bf8b92f2f592249e 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -8986,9 +8986,14 @@ async fn test_ignored_dirs_events(cx: &mut gpui::TestAppContext) { }); assert_eq!( - repository_updates.lock().as_slice(), + repository_updates + .lock() + .iter() + .filter(|update| !matches!(update, RepositoryEvent::PathsChanged)) + .cloned() + .collect::>(), Vec::new(), - "No further repo events should happen, as only ignored dirs' contents was changed", + "No further RepositoryUpdated events should happen, as only ignored dirs' contents was changed", ); assert_eq!( project_events.lock().as_slice(), @@ -9088,7 +9093,11 @@ async fn test_odd_events_for_ignored_dirs( }); assert_eq!( - repository_updates.lock().drain(..).collect::>(), + repository_updates + .lock() + .drain(..) + .filter(|update| !matches!(update, RepositoryEvent::PathsChanged)) + .collect::>(), vec![ RepositoryEvent::Updated { full_scan: true, @@ -9119,9 +9128,14 @@ async fn test_odd_events_for_ignored_dirs( cx.executor().run_until_parked(); assert_eq!( - repository_updates.lock().as_slice(), + repository_updates + .lock() + .iter() + .filter(|update| !matches!(update, RepositoryEvent::PathsChanged)) + .cloned() + .collect::>(), Vec::new(), - "No further repo events should happen, as only ignored dirs received FS events", + "No further RepositoryUpdated events should happen, as only ignored dirs received FS events", ); assert_eq!( project_events.lock().as_slice(), diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 909de9c0837448e2f1314c45629598237b0e415d..760dd05ed1382307e5cc3471731074df6d95d106 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -30,7 +30,7 @@ use menu::{Confirm, SelectFirst, SelectLast, SelectNext, SelectPrevious}; use project::{ Entry, EntryKind, Fs, GitEntry, GitEntryRef, GitTraversal, Project, ProjectEntryId, ProjectPath, Worktree, WorktreeId, - git_store::{GitStoreEvent, git_traversal::ChildEntriesGitIter}, + git_store::{GitStoreEvent, RepositoryEvent, git_traversal::ChildEntriesGitIter}, project_settings::GoToDiagnosticSeverityFilter, }; use project_panel_settings::ProjectPanelSettings; @@ -500,7 +500,7 @@ impl ProjectPanel { &git_store, window, |this, _, event, window, cx| match event { - GitStoreEvent::RepositoryUpdated(_, _, _) + GitStoreEvent::RepositoryUpdated(_, RepositoryEvent::Updated { .. }, _) | GitStoreEvent::RepositoryAdded(_) | GitStoreEvent::RepositoryRemoved(_) => { this.update_visible_entries(None, false, false, window, cx);