From fdfb2870c92642c025cf7e2ee0cf6eaa71e96ba4 Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Tue, 12 May 2026 14:25:33 -0400 Subject: [PATCH 1/5] Some fixes in --- crates/git_ui/src/branch_picker.rs | 162 ++++++++++++++++++-- crates/git_ui/src/project_diff.rs | 78 +++++++++- crates/project/src/git_store/branch_diff.rs | 28 +++- 3 files changed, 253 insertions(+), 15 deletions(-) diff --git a/crates/git_ui/src/branch_picker.rs b/crates/git_ui/src/branch_picker.rs index 839997cc588ec3..a447a8fcb50b2b 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::Modal, + rems(34.), + 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,6 +171,58 @@ 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() @@ -155,7 +233,13 @@ impl BranchList { 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 +251,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(); @@ -269,6 +353,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 +369,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 +498,21 @@ 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, + }, +} + #[derive(Debug)] enum PickerState { /// When we display list of branches/remotes @@ -533,8 +632,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![], @@ -547,13 +654,21 @@ 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 { + matches!( + self.branch_selection_behavior, + BranchSelectionBehavior::Select { .. } + ) + } + fn is_force_delete_hovering_index(&self, index: usize) -> bool { self.modifiers.alt && self.hovered_delete_index == Some(index) } @@ -729,8 +844,12 @@ 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 { + match self.branch_filter { + BranchFilter::All | BranchFilter::Remote => "Switch branch…", + } } } PickerState::CreateRemote(_) => "Enter a name for this remote…", @@ -907,7 +1026,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(' ', "-"); @@ -941,7 +1061,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 { @@ -961,6 +1084,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()) @@ -1317,10 +1448,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 @@ -1336,7 +1470,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(); @@ -1606,6 +1743,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 3301fbc66f76fd..28f6b79a47cfcd 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, @@ -400,6 +404,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 + }) + } }, ); @@ -770,6 +781,8 @@ impl ProjectDiff { reason: RefreshReason, cx: &mut AsyncWindowContext, ) -> Result<()> { + let start = std::time::Instant::now(); + dbg!(reason, start); let mut path_keys = Vec::new(); let buffers_to_load = this.update(cx, |this, cx| { let (repo, buffers_to_load) = this.branch_diff.update(cx, |branch_diff, cx| { @@ -1125,7 +1138,17 @@ impl Render for ProjectDiff { .items_center() .justify_center() .size_full() - .when(is_empty, |el| { + .when(is_empty && !self._task.is_ready(), |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 && self._task.is_ready(), |el| { let remote_button = if let Some(panel) = self .workspace .upgrade() @@ -1645,6 +1668,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); @@ -1658,6 +1690,48 @@ 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 this = &mut *project_diff; + this.branch_diff.update(cx, |branch_diff, cx| { + branch_diff.set_diff_base( + DiffBase::Merge { base_ref: 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 dc7c8bf647585d..f3f84798587386 100644 --- a/crates/project/src/git_store/branch_diff.rs +++ b/crates/project/src/git_store/branch_diff.rs @@ -40,6 +40,7 @@ pub struct BranchDiff { base_commit: Option, head_commit: Option, tree_diff: Option, + tree_diff_update_needed: bool, _subscription: Subscription, update_needed: postage::watch::Sender<()>, _task: Task<()>, @@ -47,6 +48,7 @@ pub struct BranchDiff { pub enum BranchDiffEvent { FileListChanged, + DiffBaseChanged, } impl EventEmitter for BranchDiff {} @@ -98,6 +100,7 @@ impl BranchDiff { repo, project, tree_diff: None, + tree_diff_update_needed: false, base_commit: None, head_commit: None, _subscription: git_store_subscription, @@ -113,12 +116,31 @@ impl BranchDiff { pub fn set_repo(&mut self, repo: Option>, cx: &mut Context) { self.repo = repo; self.tree_diff = None; + self.tree_diff_update_needed = self.diff_base.is_merge_base(); self.base_commit = None; self.head_commit = None; cx.emit(BranchDiffEvent::FileListChanged); *self.update_needed.borrow_mut() = (); } + pub fn set_diff_base(&mut self, diff_base: DiffBase, cx: &mut Context) { + if self.diff_base == diff_base { + return; + } + + let should_reload_tree_diff = diff_base.is_merge_base(); + self.diff_base = diff_base; + self.tree_diff_update_needed = should_reload_tree_diff; + self.base_commit = None; + self.head_commit = None; + + self.tree_diff = None; + cx.emit(BranchDiffEvent::DiffBaseChanged); + if should_reload_tree_diff { + *self.update_needed.borrow_mut() = (); + } + } + pub async fn handle_status_updates( this: WeakEntity, mut recv: postage::watch::Receiver<()>, @@ -127,7 +149,8 @@ impl BranchDiff { Self::reload_tree_diff(this.clone(), cx).await.log_err(); while recv.next().await.is_some() { let Ok(needs_update) = this.update(cx, |this, cx| { - let mut needs_update = false; + let mut needs_update = this.tree_diff_update_needed; + this.tree_diff_update_needed = false; if this.repo.is_none() { let active_repo = this @@ -286,6 +309,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(); From adb222ba3ba104c75ee1cc1480d340095bbb306b Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Tue, 12 May 2026 14:37:42 -0400 Subject: [PATCH 2/5] Fix merge conflict Co-authored-by: Remco Smits --- crates/project/src/git_store/branch_diff.rs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/crates/project/src/git_store/branch_diff.rs b/crates/project/src/git_store/branch_diff.rs index 93932ec7166eb8..85763fccac696f 100644 --- a/crates/project/src/git_store/branch_diff.rs +++ b/crates/project/src/git_store/branch_diff.rs @@ -113,15 +113,6 @@ impl BranchDiff { &self.diff_base } - pub fn set_diff_base(&mut self, diff_base: DiffBase, cx: &mut Context) { - self.diff_base = diff_base; - self.tree_diff = 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; self.tree_diff = None; @@ -137,15 +128,14 @@ impl BranchDiff { return; } - let should_reload_tree_diff = diff_base.is_merge_base(); + self.tree_diff_update_needed = diff_base.is_merge_base(); + self.tree_diff = None; self.diff_base = diff_base; - self.tree_diff_update_needed = should_reload_tree_diff; self.base_commit = None; self.head_commit = None; - self.tree_diff = None; cx.emit(BranchDiffEvent::DiffBaseChanged); - if should_reload_tree_diff { + if self.tree_diff_update_needed { *self.update_needed.borrow_mut() = (); } } From 35bcd7a5ce293f36cc9f93f5412c9e26c3336012 Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Tue, 12 May 2026 18:12:54 -0400 Subject: [PATCH 3/5] Fix refresh --- crates/git_ui/src/project_diff.rs | 8 ++-- crates/project/src/git_store/branch_diff.rs | 46 +++++++++++++++------ 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index 1508bbb1dc134d..7a2a8c5740e5db 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -821,8 +821,6 @@ impl ProjectDiff { reason: RefreshReason, cx: &mut AsyncWindowContext, ) -> Result<()> { - let start = std::time::Instant::now(); - dbg!(reason, start); let mut path_keys = Vec::new(); let buffers_to_load = this.update(cx, |this, cx| { let (repo, buffers_to_load) = this.branch_diff.update(cx, |branch_diff, cx| { @@ -1165,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() @@ -1178,7 +1178,7 @@ impl Render for ProjectDiff { .items_center() .justify_center() .size_full() - .when(is_empty && !self._task.is_ready(), |el| { + .when(is_empty && is_loading, |el| { let rems = TextSize::Large.rems(cx); el.child( Icon::new(IconName::LoadCircle) @@ -1188,7 +1188,7 @@ impl Render for ProjectDiff { .into_any_element(), ) }) - .when(is_empty && self._task.is_ready(), |el| { + .when(is_empty && !is_loading, |el| { let remote_button = if let Some(panel) = self .workspace .upgrade() diff --git a/crates/project/src/git_store/branch_diff.rs b/crates/project/src/git_store/branch_diff.rs index 85763fccac696f..a6b42a5df01a81 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; @@ -41,6 +41,7 @@ pub struct BranchDiff { 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<()>, @@ -101,6 +102,7 @@ impl BranchDiff { project, tree_diff: None, tree_diff_update_needed: false, + tree_diff_base_task: None, base_commit: None, head_commit: None, _subscription: git_store_subscription, @@ -117,6 +119,7 @@ impl BranchDiff { 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); @@ -130,6 +133,7 @@ impl BranchDiff { 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; @@ -145,9 +149,10 @@ 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 Ok(()) = this.update(cx, |this, cx| { let mut needs_update = this.tree_diff_update_needed; this.tree_diff_update_needed = false; @@ -180,14 +185,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(); - } } } @@ -266,10 +270,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; From 07c95c143096a3cb02cb4fc8faa1994709b59707 Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Wed, 13 May 2026 18:35:33 -0400 Subject: [PATCH 4/5] clean up UI --- crates/git_ui/src/branch_picker.rs | 287 ++++++++++++++++++++++++++--- crates/git_ui/src/project_diff.rs | 16 +- 2 files changed, 272 insertions(+), 31 deletions(-) diff --git a/crates/git_ui/src/branch_picker.rs b/crates/git_ui/src/branch_picker.rs index a447a8fcb50b2b..26e33db8ce5142 100644 --- a/crates/git_ui/src/branch_picker.rs +++ b/crates/git_ui/src/branch_picker.rs @@ -107,8 +107,8 @@ pub fn select_popover( let list = BranchList::new_select( workspace, repository, - BranchListStyle::Modal, - rems(34.), + BranchListStyle::Popover, + rems(20.), selected_branch, on_select, window, @@ -226,7 +226,12 @@ impl BranchList { ) -> 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| { @@ -263,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); }); } @@ -513,6 +520,102 @@ enum BranchSelectionBehavior { }, } +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 @@ -596,7 +699,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| { @@ -610,7 +745,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(); @@ -663,10 +803,7 @@ impl BranchListDelegate { } fn is_select_only(&self) -> bool { - matches!( - self.branch_selection_behavior, - BranchSelectionBehavior::Select { .. } - ) + self.branch_selection_behavior.is_select_only() } fn is_force_delete_hovering_index(&self, index: usize) -> bool { @@ -924,6 +1061,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, @@ -954,6 +1095,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| { @@ -972,8 +1120,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 { @@ -1003,8 +1150,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 }; @@ -1202,13 +1348,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 @@ -1246,8 +1401,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(); @@ -1321,7 +1474,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 @@ -1421,9 +1574,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") @@ -1670,7 +1832,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}; @@ -1693,6 +1857,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}"), @@ -1702,7 +1876,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, @@ -1722,6 +1902,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, diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index 7a2a8c5740e5db..be7ffda92d0f02 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -1739,16 +1739,12 @@ impl Render for BranchDiffToolbar { let base_ref: SharedString = branch.name().to_owned().into(); project_diff .update(cx, |project_diff, cx| { - { - let this = &mut *project_diff; - this.branch_diff.update(cx, |branch_diff, cx| { - branch_diff.set_diff_base( - DiffBase::Merge { base_ref: base_ref }, - cx, - ); - }); - cx.notify(); - }; + 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(); }); From f216313467f37a321e52dd5635106254dabc379a Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Wed, 13 May 2026 18:48:09 -0400 Subject: [PATCH 5/5] Dedupe set_repo and simplify branch picker placeholder --- crates/git_ui/src/branch_picker.rs | 4 +--- crates/project/src/git_store/branch_diff.rs | 9 +++++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/git_ui/src/branch_picker.rs b/crates/git_ui/src/branch_picker.rs index 26e33db8ce5142..fd45c5fdefb13f 100644 --- a/crates/git_ui/src/branch_picker.rs +++ b/crates/git_ui/src/branch_picker.rs @@ -984,9 +984,7 @@ impl PickerDelegate for BranchListDelegate { if self.is_select_only() { "Select branch…" } else { - match self.branch_filter { - BranchFilter::All | BranchFilter::Remote => "Switch branch…", - } + "Switch branch…" } } PickerState::CreateRemote(_) => "Enter a name for this remote…", diff --git a/crates/project/src/git_store/branch_diff.rs b/crates/project/src/git_store/branch_diff.rs index a6b42a5df01a81..60c93abd0a17e4 100644 --- a/crates/project/src/git_store/branch_diff.rs +++ b/crates/project/src/git_store/branch_diff.rs @@ -116,6 +116,15 @@ impl BranchDiff { } 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();