From 3b745664f9600a487cd5a4e98f235411b97f1ca4 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Wed, 13 May 2026 19:15:35 -0400 Subject: [PATCH] git: Allow choosing branch diff target in branch diff view (#56569) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Summary Added a branch diff base branch picker that shows the selected diff base and prioritizes branches on the active branch’s remote. This also generalizes the branch picker’s checkout vs. select only behavior so its UI/search/rendering can be reused by a future git graph picker, and makes branch diff drop stale in progress base update tasks when the base or repository changes for faster response. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #54050 Release Notes: - git: Allow choosing branch diff base in branch diff view --------- Co-authored-by: Remco Smits --- crates/git_ui/src/branch_picker.rs | 435 ++++++++++++++++++-- crates/git_ui/src/project_diff.rs | 74 +++- crates/project/src/git_store/branch_diff.rs | 85 +++- 3 files changed, 545 insertions(+), 49 deletions(-) diff --git a/crates/git_ui/src/branch_picker.rs b/crates/git_ui/src/branch_picker.rs index a6ed382e9c..63c9bd59b1 100644 --- a/crates/git_ui/src/branch_picker.rs +++ b/crates/git_ui/src/branch_picker.rs @@ -95,6 +95,32 @@ pub fn popover( }) } +pub fn select_popover( + workspace: WeakEntity, + repository: Option>, + selected_branch: Option, + on_select: SelectBranchCallback, + window: &mut Window, + cx: &mut App, +) -> Entity { + cx.new(|cx| { + let list = BranchList::new_select( + workspace, + repository, + BranchListStyle::Popover, + rems(20.), + selected_branch, + on_select, + window, + cx, + ); + list.focus_handle(cx).focus(window, cx); + list + }) +} + +pub type SelectBranchCallback = Arc; + pub fn create_embedded( workspace: WeakEntity, repository: Option>, @@ -145,17 +171,80 @@ impl BranchList { embedded: bool, window: &mut Window, cx: &mut Context, + ) -> Self { + Self::new_inner_with_behavior( + workspace, + repository, + style, + width, + embedded, + BranchSelectionBehavior::Checkout, + window, + cx, + ) + } + + fn new_select( + workspace: WeakEntity, + repository: Option>, + style: BranchListStyle, + width: Rems, + selected_branch: Option, + on_select: SelectBranchCallback, + window: &mut Window, + cx: &mut Context, + ) -> Self { + let mut this = Self::new_inner_with_behavior( + workspace, + repository, + style, + width, + false, + BranchSelectionBehavior::Select { + selected_branch, + on_select, + }, + window, + cx, + ); + this._subscriptions + .push(cx.subscribe(&this.picker, |_, _, _, cx| { + cx.emit(DismissEvent); + })); + this + } + + fn new_inner_with_behavior( + workspace: WeakEntity, + repository: Option>, + style: BranchListStyle, + width: Rems, + embedded: bool, + branch_selection_behavior: BranchSelectionBehavior, + window: &mut Window, + cx: &mut Context, ) -> Self { let all_branches = repository .as_ref() - .map(|repo| process_branches(&repo.read(cx).branch_list)) + .map(|repo| { + process_branches( + &repo.read(cx).branch_list, + branch_selection_behavior.selected_branch(), + ) + }) .unwrap_or_default(); let default_branch_request = repository.clone().map(|repository| { repository.update(cx, |repository, _| repository.default_branch(false)) }); - let mut delegate = BranchListDelegate::new(workspace, repository.clone(), style, cx); + let mut delegate = BranchListDelegate::new( + workspace, + repository.clone(), + style, + branch_selection_behavior, + cx, + ); delegate.all_branches = all_branches; let picker = cx.new(|cx| { @@ -167,7 +256,7 @@ impl BranchList { picker.update(cx, |picker, _| { picker.delegate.focus_handle = picker_focus_handle.clone(); - picker.delegate.show_footer = !embedded; + picker.delegate.show_footer = !embedded && !picker.delegate.is_select_only(); }); let mut subscriptions = Vec::new(); @@ -179,14 +268,16 @@ impl BranchList { move |this, repo, event, window, cx| { if matches!(event, RepositoryEvent::BranchListChanged) { let branch_list = repo.read(cx).branch_list.clone(); - let all_branches = process_branches(&branch_list); this.picker.update(cx, |picker, cx| { picker.delegate.restore_selected_branch = picker .delegate .matches .get(picker.delegate.selected_index) .and_then(|entry| entry.as_branch().map(|b| b.ref_name.clone())); - picker.delegate.all_branches = all_branches; + picker.delegate.all_branches = process_branches( + &branch_list, + picker.delegate.branch_selection_behavior.selected_branch(), + ); picker.refresh(window, cx); }); } @@ -269,6 +360,9 @@ impl BranchList { cx: &mut Context, ) { self.picker.update(cx, |picker, cx| { + if picker.delegate.is_select_only() { + return; + } picker .delegate .delete_at(picker.delegate.selected_index, false, window, cx) @@ -282,6 +376,9 @@ impl BranchList { cx: &mut Context, ) { self.picker.update(cx, |picker, cx| { + if picker.delegate.is_select_only() { + return; + } picker .delegate .delete_at(picker.delegate.selected_index, true, window, cx) @@ -408,12 +505,117 @@ pub struct BranchListDelegate { modifiers: Modifiers, branch_filter: BranchFilter, state: PickerState, + branch_selection_behavior: BranchSelectionBehavior, focus_handle: FocusHandle, restore_selected_branch: Option, show_footer: bool, hovered_delete_index: Option, } +enum BranchSelectionBehavior { + Checkout, + Select { + selected_branch: Option, + on_select: SelectBranchCallback, + }, +} + +impl BranchSelectionBehavior { + fn selected_branch(&self) -> Option<&SharedString> { + match self { + Self::Checkout => None, + Self::Select { + selected_branch, .. + } => selected_branch.as_ref(), + } + } + + fn is_select_only(&self) -> bool { + matches!(self, Self::Select { .. }) + } +} + +#[derive(Clone)] +struct BranchSelectionContext { + selected_branch: Option, + active_branch_ref_name: Option, + active_branch_upstream_ref_name: Option, + active_branch_remote_name: Option, +} + +impl BranchSelectionContext { + fn new( + selected_branch: Option, + repo: Option<&Entity>, + cx: &App, + ) -> Self { + let active_branch = repo.and_then(|repo| repo.read(cx).branch.clone()); + let active_branch_ref_name = active_branch.as_ref().map(|branch| branch.ref_name.clone()); + let active_branch_upstream_ref_name = active_branch.as_ref().and_then(|branch| { + branch + .upstream + .as_ref() + .map(|upstream| upstream.ref_name.clone()) + }); + let active_branch_remote_name = active_branch.as_ref().and_then(|branch| { + branch + .upstream + .as_ref() + .and_then(|upstream| upstream.remote_name()) + .or_else(|| branch.remote_name()) + .map(SharedString::from) + }); + + Self { + selected_branch, + active_branch_ref_name, + active_branch_upstream_ref_name, + active_branch_remote_name, + } + } + + fn priority(&self, branch: &Branch) -> usize { + if self + .selected_branch + .as_ref() + .is_some_and(|selected_branch| branch_matches_ref(branch, selected_branch)) + { + 0 + } else if self.is_on_active_branch_remote(branch) { + 1 + } else if self.is_active_branch(branch) || self.is_active_upstream(branch) { + 3 + } else { + 2 + } + } + + fn is_active_branch(&self, branch: &Branch) -> bool { + self.active_branch_ref_name + .as_ref() + .is_some_and(|ref_name| branch.ref_name.as_ref() == ref_name.as_ref()) + } + + fn is_active_upstream(&self, branch: &Branch) -> bool { + self.active_branch_upstream_ref_name + .as_ref() + .is_some_and(|ref_name| branch.ref_name.as_ref() == ref_name.as_ref()) + } + + fn is_on_active_branch_remote(&self, branch: &Branch) -> bool { + if self.is_active_branch(branch) || self.is_active_upstream(branch) { + return false; + } + + let Some(active_branch_remote_name) = &self.active_branch_remote_name else { + return false; + }; + + branch_remote_name(branch) + .is_some_and(|remote_name| remote_name == active_branch_remote_name.as_ref()) + } +} + #[derive(Debug)] enum PickerState { /// When we display list of branches/remotes @@ -524,7 +726,39 @@ impl Render for DeleteBranchTooltip { } } -fn process_branches(branches: &Arc<[Branch]>) -> Vec { +fn branch_matches_ref(branch: &Branch, branch_ref: &SharedString) -> bool { + branch.ref_name.as_ref() == branch_ref.as_ref() || branch.name() == branch_ref.as_ref() +} + +fn branch_remote_name(branch: &Branch) -> Option<&str> { + branch.remote_name().or_else(|| { + branch + .upstream + .as_ref() + .and_then(|upstream| upstream.remote_name()) + }) +} + +fn sort_branch_entries( + matches: &mut [Entry], + branch_selection_context: Option<&BranchSelectionContext>, +) { + matches.sort_by_key(|entry| { + let Some(branch) = entry.as_branch() else { + return (4, false); + }; + + let priority = branch_selection_context + .map(|context| context.priority(branch)) + .unwrap_or(0); + (priority, branch.is_remote()) + }); +} + +fn process_branches( + branches: &Arc<[Branch]>, + preserved_branch: Option<&SharedString>, +) -> Vec { let remote_upstreams: HashSet<_> = branches .iter() .filter_map(|branch| { @@ -538,7 +772,12 @@ fn process_branches(branches: &Arc<[Branch]>) -> Vec { let mut result: Vec = branches .iter() - .filter(|branch| !remote_upstreams.contains(&branch.ref_name)) + .filter(|branch| { + !remote_upstreams.contains(&branch.ref_name) + || preserved_branch + .as_ref() + .is_some_and(|preserved_branch| branch_matches_ref(branch, preserved_branch)) + }) .cloned() .collect(); @@ -560,8 +799,16 @@ impl BranchListDelegate { workspace: WeakEntity, repo: Option>, style: BranchListStyle, + branch_selection_behavior: BranchSelectionBehavior, cx: &mut Context, ) -> Self { + let restore_selected_branch = match &branch_selection_behavior { + BranchSelectionBehavior::Checkout => None, + BranchSelectionBehavior::Select { + selected_branch, .. + } => selected_branch.clone(), + }; + Self { workspace, matches: vec![], @@ -574,13 +821,18 @@ impl BranchListDelegate { modifiers: Default::default(), branch_filter: BranchFilter::All, state: PickerState::List, + branch_selection_behavior, focus_handle: cx.focus_handle(), - restore_selected_branch: None, + restore_selected_branch, show_footer: false, hovered_delete_index: None, } } + fn is_select_only(&self) -> bool { + self.branch_selection_behavior.is_select_only() + } + fn is_force_delete_hovering_index(&self, index: usize) -> bool { self.modifiers.alt && self.hovered_delete_index == Some(index) } @@ -757,8 +1009,10 @@ impl PickerDelegate for BranchListDelegate { fn placeholder_text(&self, _window: &mut Window, _cx: &mut App) -> Arc { match self.state { PickerState::List | PickerState::NewRemote | PickerState::NewBranch => { - match self.branch_filter { - BranchFilter::All | BranchFilter::Remote => "Switch branch…", + if self.is_select_only() { + "Select branch…" + } else { + "Switch branch…" } } PickerState::CreateRemote(_) => "Enter a name for this remote…", @@ -833,6 +1087,10 @@ impl PickerDelegate for BranchListDelegate { } fn editor_position(&self) -> PickerEditorPosition { + if self.is_select_only() { + return PickerEditorPosition::Start; + } + match self.style { BranchListStyle::Modal => PickerEditorPosition::Start, BranchListStyle::Popover => PickerEditorPosition::End, @@ -863,6 +1121,13 @@ impl PickerDelegate for BranchListDelegate { cx: &mut Context>, ) -> Task<()> { let all_branches = self.all_branches.clone(); + let branch_selection_context = self.is_select_only().then(|| { + BranchSelectionContext::new( + self.branch_selection_behavior.selected_branch().cloned(), + self.repo.as_ref(), + cx, + ) + }); let branch_filter = self.branch_filter; cx.spawn_in(window, async move |picker, cx| { @@ -881,8 +1146,7 @@ impl PickerDelegate for BranchListDelegate { }) .collect(); - // Keep the existing recency sort within each group, but show local branches first. - matches.sort_by_key(|entry| entry.as_branch().is_some_and(|b| b.is_remote())); + sort_branch_entries(&mut matches, branch_selection_context.as_ref()); matches } else { @@ -912,8 +1176,7 @@ impl PickerDelegate for BranchListDelegate { }) .collect(); - // Keep fuzzy-relevance ordering within local/remote groups, but show locals first. - matches.sort_by_key(|entry| entry.as_branch().is_some_and(|b| b.is_remote())); + sort_branch_entries(&mut matches, branch_selection_context.as_ref()); matches }; @@ -935,7 +1198,8 @@ impl PickerDelegate for BranchListDelegate { return; } - if !query.is_empty() + if !picker.delegate.is_select_only() + && !query.is_empty() && !matches.first().is_some_and(|entry| entry.name() == query) { let query = query.replace(' ', "-"); @@ -969,7 +1233,10 @@ impl PickerDelegate for BranchListDelegate { .matches .iter() .position(|entry| { - entry.as_branch().is_some_and(|b| b.ref_name == ref_name) + entry.as_branch().is_some_and(|branch| { + branch.ref_name == ref_name + || branch.name() == ref_name.as_ref() + }) }) .unwrap_or(0); } else { @@ -989,6 +1256,14 @@ impl PickerDelegate for BranchListDelegate { match entry { Entry::Branch { branch, .. } => { + if let BranchSelectionBehavior::Select { on_select, .. } = + &self.branch_selection_behavior + { + on_select(branch.clone(), cx); + cx.emit(DismissEvent); + return; + } + let current_branch = self.repo.as_ref().map(|repo| { repo.read_with(cx, |repo, _| { repo.branch.as_ref().map(|branch| branch.ref_name.clone()) @@ -1099,13 +1374,22 @@ impl PickerDelegate for BranchListDelegate { .unwrap_or_else(|| (None, None, None, None)); let is_head_branch = entry.as_branch().is_some_and(|branch| branch.is_head); + let is_checked_branch = entry.as_branch().is_some_and(|branch| { + if self.is_select_only() { + self.branch_selection_behavior + .selected_branch() + .is_some_and(|selected_branch| branch_matches_ref(branch, selected_branch)) + } else { + branch.is_head + } + }); let entry_icon = match entry { Entry::NewUrl { .. } | Entry::NewBranch { .. } | Entry::NewRemoteName { .. } => { IconName::Plus } Entry::Branch { branch, .. } => { - if is_head_branch { + if is_checked_branch { IconName::Check } else if branch.is_remote() { IconName::Screen @@ -1143,8 +1427,6 @@ impl PickerDelegate for BranchListDelegate { Entry::NewUrl { .. } | Entry::NewBranch { .. } | Entry::NewRemoteName { .. } ); - let is_head_branch = entry.as_branch().is_some_and(|branch| branch.is_head); - let deleted_branch_icon = |entry_ix: usize| { let picker = picker.clone(); let focus_handle = focus_handle.clone(); @@ -1218,7 +1500,7 @@ impl PickerDelegate for BranchListDelegate { .flex_grow() .child( Icon::new(entry_icon) - .color(if is_head_branch { + .color(if is_checked_branch { Color::Accent } else { Color::Muted @@ -1318,9 +1600,18 @@ impl PickerDelegate for BranchListDelegate { let absolute_time = absolute_time.clone(); this.tooltip({ let is_head = is_head_branch; + let is_checked = is_checked_branch; + let is_select_only = self.is_select_only(); Tooltip::element(move |_, _| { v_flex() .child(Label::new(branch_name.clone())) + .when(is_select_only && is_checked, |this| { + this.child( + Label::new("Selected Branch") + .size(LabelSize::Small) + .color(Color::Muted), + ) + }) .when(is_head, |this| { this.child( Label::new("Current Branch") @@ -1345,10 +1636,13 @@ impl PickerDelegate for BranchListDelegate { ), ), ) - .when(!is_new_items && !is_head_branch, |this| { - this.end_slot(deleted_branch_icon(ix)) - .show_end_slot_on_hover() - }) + .when( + !self.is_select_only() && !is_new_items && !is_head_branch, + |this| { + this.end_slot(deleted_branch_icon(ix)) + .show_end_slot_on_hover() + }, + ) .when_some( if is_new_items { create_from_default_button @@ -1364,7 +1658,10 @@ impl PickerDelegate for BranchListDelegate { } fn render_footer(&self, _: &mut Window, cx: &mut Context>) -> Option { - if !self.show_footer || self.editor_position() == PickerEditorPosition::End { + if self.is_select_only() + || !self.show_footer + || self.editor_position() == PickerEditorPosition::End + { return None; } let focus_handle = self.focus_handle.clone(); @@ -1561,7 +1858,9 @@ mod tests { use std::collections::HashSet; use super::*; - use git::repository::{CommitSummary, Remote}; + use git::repository::{ + CommitSummary, Remote, Upstream, UpstreamTracking, UpstreamTrackingStatus, + }; use gpui::{AppContext, TestAppContext, VisualTestContext}; use project::{FakeFs, Project}; use rand::{Rng, rngs::StdRng}; @@ -1584,6 +1883,16 @@ mod tests { is_head: bool, remote_name: Option<&str>, timestamp: Option, + ) -> Branch { + create_test_branch_with_upstream(name, is_head, remote_name, timestamp, None) + } + + fn create_test_branch_with_upstream( + name: &str, + is_head: bool, + remote_name: Option<&str>, + timestamp: Option, + upstream_ref_name: Option<&str>, ) -> Branch { let ref_name = match remote_name { Some(remote_name) => format!("refs/remotes/{remote_name}/{name}"), @@ -1593,7 +1902,13 @@ mod tests { Branch { is_head, ref_name: ref_name.into(), - upstream: None, + upstream: upstream_ref_name.map(|ref_name| Upstream { + ref_name: ref_name.into(), + tracking: UpstreamTracking::Tracked(UpstreamTrackingStatus { + ahead: 0, + behind: 0, + }), + }), most_recent_commit: timestamp.map(|ts| CommitSummary { sha: "abc123".into(), commit_timestamp: ts, @@ -1613,6 +1928,71 @@ mod tests { ] } + #[test] + fn test_select_branch_preserves_selected_remote_upstream_and_prioritizes_active_remote_branches() + { + let selected_branch = SharedString::from("origin/main"); + let branches: Arc<[Branch]> = Arc::from([ + create_test_branch_with_upstream( + "feature", + true, + None, + Some(1200), + Some("refs/remotes/origin/feature"), + ), + create_test_branch_with_upstream( + "main", + false, + None, + Some(1100), + Some("refs/remotes/origin/main"), + ), + create_test_branch("main", false, Some("origin"), Some(1000)), + create_test_branch("feature", false, Some("origin"), Some(900)), + create_test_branch("main", false, Some("fork"), Some(800)), + ]); + + let processed_branches = process_branches(&branches, Some(&selected_branch)); + assert!( + processed_branches + .iter() + .any(|branch| branch.name() == "origin/main"), + "the selected remote branch should be preserved even when a local branch tracks it" + ); + assert!( + processed_branches + .iter() + .all(|branch| branch.name() != "origin/feature"), + "the active branch's unselected remote upstream should still be collapsed" + ); + + let mut entries = processed_branches + .into_iter() + .map(|branch| Entry::Branch { + branch, + positions: Vec::new(), + }) + .collect::>(); + let selection_context = BranchSelectionContext { + selected_branch: Some(selected_branch), + active_branch_ref_name: Some("refs/heads/feature".into()), + active_branch_upstream_ref_name: Some("refs/remotes/origin/feature".into()), + active_branch_remote_name: Some("origin".into()), + }; + + sort_branch_entries(&mut entries, Some(&selection_context)); + + let ordered_branch_names = entries.iter().map(Entry::name).collect::>(); + assert_eq!(ordered_branch_names.first(), Some(&"origin/main")); + assert!( + ordered_branch_names.iter().position(|name| *name == "main") + < ordered_branch_names + .iter() + .position(|name| *name == "fork/main"), + "branches on the active branch's remote should be prioritized" + ); + } + async fn init_branch_list_test( repository: Option>, branches: Vec, @@ -1634,6 +2014,7 @@ mod tests { workspace.downgrade(), repository, BranchListStyle::Modal, + BranchSelectionBehavior::Checkout, cx, ); delegate.all_branches = branches; diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index 3f0e1d4c1f..be7ffda92d 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -1,4 +1,5 @@ use crate::{ + branch_picker, conflict_view::ConflictAddon, git_panel::{GitPanel, GitPanelAddon, GitStatusEntry}, git_panel_settings::GitPanelSettings, @@ -37,7 +38,10 @@ use settings::{Settings, SettingsStore}; use std::any::{Any, TypeId}; use std::sync::Arc; use theme::ActiveTheme; -use ui::{DiffStat, Divider, KeyBinding, Tooltip, prelude::*, vertical_divider}; +use ui::{ + CommonAnimationExt as _, DiffStat, Divider, KeyBinding, PopoverMenu, Tooltip, prelude::*, + vertical_divider, +}; use util::{ResultExt as _, rel_path::RelPath}; use workspace::{ CloseActiveItem, ItemNavHistory, SerializableItem, ToolbarItemEvent, ToolbarItemLocation, @@ -440,6 +444,13 @@ impl ProjectDiff { async |cx| Self::refresh(this, RefreshReason::StatusesChanged, cx).await }) } + BranchDiffEvent::DiffBaseChanged => { + this.pending_scroll.take(); + this._task = window.spawn(cx, { + let this = cx.weak_entity(); + async |cx| Self::refresh(this, RefreshReason::StatusesChanged, cx).await + }) + } }, ); @@ -1152,6 +1163,8 @@ impl Item for ProjectDiff { impl Render for ProjectDiff { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { let is_empty = self.multibuffer.read(cx).is_empty(); + let is_loading = self.branch_diff.read(cx).is_tree_base_loading() || !self._task.is_ready(); + let is_branch_diff_view = matches!(self.diff_base(cx), DiffBase::Merge { .. }); div() @@ -1165,7 +1178,17 @@ impl Render for ProjectDiff { .items_center() .justify_center() .size_full() - .when(is_empty, |el| { + .when(is_empty && is_loading, |el| { + let rems = TextSize::Large.rems(cx); + el.child( + Icon::new(IconName::LoadCircle) + .size(IconSize::Custom(rems)) + .color(Color::Accent) + .with_rotate_animation(3) + .into_any_element(), + ) + }) + .when(is_empty && !is_loading, |el| { let remote_button = if let Some(panel) = self .workspace .upgrade() @@ -1685,6 +1708,15 @@ impl Render for BranchDiffToolbar { let focus_handle = project_diff.focus_handle(cx); let review_count = project_diff.read(cx).total_review_comment_count(); let (additions, deletions) = project_diff.read(cx).calculate_changed_lines(cx); + let diff_base = project_diff.read(cx).diff_base(cx).clone(); + let DiffBase::Merge { base_ref } = diff_base else { + return div(); + }; + let selected_base_ref = base_ref.clone(); + let base_ref_label = format!("Base: {base_ref}"); + let repository = project_diff.read(cx).branch_diff.read(cx).repo().cloned(); + let workspace = project_diff.read(cx).workspace.clone(); + let project_diff_for_picker = project_diff.downgrade(); let is_multibuffer_empty = project_diff.read(cx).multibuffer.read(cx).is_empty(); let is_ai_enabled = AgentSettings::get_global(cx).enabled(cx); @@ -1698,6 +1730,44 @@ impl Render for BranchDiffToolbar { .flex_wrap() .justify_end() .gap_2() + .child( + PopoverMenu::new("branch-diff-base-branch-picker") + .menu(move |window, cx| { + let project_diff = project_diff_for_picker.clone(); + let on_select = + Arc::new(move |branch: git::repository::Branch, cx: &mut App| { + let base_ref: SharedString = branch.name().to_owned().into(); + project_diff + .update(cx, |project_diff, cx| { + let branch_diff = &mut project_diff.branch_diff; + branch_diff.update(cx, |branch_diff, cx| { + branch_diff + .set_diff_base(DiffBase::Merge { base_ref }, cx); + }); + cx.notify(); + }) + .ok(); + }); + Some(branch_picker::select_popover( + workspace.clone(), + repository.clone(), + Some(selected_base_ref.clone()), + on_select, + window, + cx, + )) + }) + .trigger_with_tooltip( + Button::new("branch-diff-base-branch", base_ref_label) + .color(Color::Muted) + .end_icon( + Icon::new(IconName::ChevronDown) + .size(IconSize::XSmall) + .color(Color::Muted), + ), + Tooltip::text("Select base branch"), + ), + ) .when(!is_multibuffer_empty, |this| { this.child(DiffStat::new( "branch-diff-stat", diff --git a/crates/project/src/git_store/branch_diff.rs b/crates/project/src/git_store/branch_diff.rs index e349315dd1..60c93abd0a 100644 --- a/crates/project/src/git_store/branch_diff.rs +++ b/crates/project/src/git_store/branch_diff.rs @@ -7,8 +7,8 @@ use git::{ status::{DiffTreeType, FileStatus, StatusCode, TrackedStatus, TreeDiff, TreeDiffStatus}, }; use gpui::{ - App, AsyncWindowContext, Context, Entity, EventEmitter, SharedString, Subscription, Task, - WeakEntity, Window, + App, AsyncApp, AsyncWindowContext, Context, Entity, EventEmitter, SharedString, Subscription, + Task, WeakEntity, Window, }; use language::Buffer; @@ -40,6 +40,8 @@ pub struct BranchDiff { base_commit: Option, head_commit: Option, tree_diff: Option, + tree_diff_update_needed: bool, + tree_diff_base_task: Option>, _subscription: Subscription, update_needed: postage::watch::Sender<()>, _task: Task<()>, @@ -47,6 +49,7 @@ pub struct BranchDiff { pub enum BranchDiffEvent { FileListChanged, + DiffBaseChanged, } impl EventEmitter for BranchDiff {} @@ -98,6 +101,8 @@ impl BranchDiff { repo, project, tree_diff: None, + tree_diff_update_needed: false, + tree_diff_base_task: None, base_commit: None, head_commit: None, _subscription: git_store_subscription, @@ -110,22 +115,42 @@ impl BranchDiff { &self.diff_base } - pub fn set_diff_base(&mut self, diff_base: DiffBase, cx: &mut Context) { - self.diff_base = diff_base; + pub fn set_repo(&mut self, repo: Option>, cx: &mut Context) { + let same_repo = match (self.repo.as_ref(), repo.as_ref()) { + (Some(current), Some(new)) => current.read(cx).id == new.read(cx).id, + (None, None) => true, + _ => false, + }; + if same_repo { + return; + } + + self.repo = repo; self.tree_diff = None; + self.tree_diff_update_needed = self.diff_base.is_merge_base(); + self.tree_diff_base_task = None; self.base_commit = None; self.head_commit = None; cx.emit(BranchDiffEvent::FileListChanged); *self.update_needed.borrow_mut() = (); } - pub fn set_repo(&mut self, repo: Option>, cx: &mut Context) { - self.repo = repo; + pub fn set_diff_base(&mut self, diff_base: DiffBase, cx: &mut Context) { + if self.diff_base == diff_base { + return; + } + + self.tree_diff_update_needed = diff_base.is_merge_base(); self.tree_diff = None; + self.tree_diff_base_task = None; + self.diff_base = diff_base; self.base_commit = None; self.head_commit = None; - cx.emit(BranchDiffEvent::FileListChanged); - *self.update_needed.borrow_mut() = (); + + cx.emit(BranchDiffEvent::DiffBaseChanged); + if self.tree_diff_update_needed { + *self.update_needed.borrow_mut() = (); + } } pub async fn handle_status_updates( @@ -133,10 +158,12 @@ impl BranchDiff { mut recv: postage::watch::Receiver<()>, cx: &mut AsyncWindowContext, ) { - Self::reload_tree_diff(this.clone(), cx).await.log_err(); + this.update(cx, |this, cx| this.spawn_reload_tree_diff(cx)) + .log_err(); while recv.next().await.is_some() { - let Ok(needs_update) = this.update(cx, |this, cx| { - let mut needs_update = false; + let Ok(()) = this.update(cx, |this, cx| { + let mut needs_update = this.tree_diff_update_needed; + this.tree_diff_update_needed = false; if this.repo.is_none() { let active_repo = this @@ -167,14 +194,13 @@ impl BranchDiff { } }) } - needs_update + + if needs_update { + this.spawn_reload_tree_diff(cx); + } }) else { return; }; - - if needs_update { - Self::reload_tree_diff(this.clone(), cx).await.log_err(); - } } } @@ -253,10 +279,26 @@ impl BranchDiff { } } - pub async fn reload_tree_diff( - this: WeakEntity, - cx: &mut AsyncWindowContext, - ) -> Result<()> { + fn spawn_reload_tree_diff(&mut self, cx: &mut Context) { + if !self.diff_base.is_merge_base() { + return; + } + + let task = cx.spawn(async move |this, cx| { + Self::reload_tree_diff(this, cx).await.log_err(); + }); + + self.tree_diff_base_task = Some(task); + cx.notify(); + } + + pub fn is_tree_base_loading(&self) -> bool { + self.tree_diff_base_task + .as_ref() + .is_some_and(|task| !task.is_ready()) + } + + pub async fn reload_tree_diff(this: WeakEntity, cx: &mut AsyncApp) -> Result<()> { let task = this.update(cx, |this, cx| { let DiffBase::Merge { base_ref } = this.diff_base.clone() else { return None; @@ -295,6 +337,9 @@ impl BranchDiff { let Some(repo) = self.repo.clone() else { return output; }; + if self.diff_base.is_merge_base() && self.tree_diff.is_none() { + return output; + } self.project.update(cx, |_project, cx| { let mut seen = HashSet::default();