Improve handling of remote-tracking branches in the picker (#29744)

Cole Miller and Anthony Eid created

Release Notes:

- Changed the git branch picker to make remote-tracking branches less
prominent

---------

Co-authored-by: Anthony Eid <hello@anthonyeid.me>

Change summary

crates/agent/src/thread.rs                                    |   2 
crates/collab/src/tests/integration_tests.rs                  |   8 
crates/collab/src/tests/remote_editing_collaboration_tests.rs |   6 
crates/fs/src/fake_git_repo.rs                                |   2 
crates/git/src/repository.rs                                  | 141 ++--
crates/git_ui/src/branch_picker.rs                            |  50 +
crates/git_ui/src/commit_modal.rs                             |   4 
crates/git_ui/src/git_panel.rs                                |  33 
crates/git_ui/src/project_diff.rs                             |   6 
crates/project/src/git_store.rs                               |  12 
crates/proto/proto/git.proto                                  |   2 
crates/remote_server/src/remote_editing_tests.rs              |   6 
crates/title_bar/src/title_bar.rs                             |   2 
13 files changed, 150 insertions(+), 124 deletions(-)

Detailed changes

crates/agent/src/thread.rs 🔗

@@ -2110,7 +2110,7 @@ impl Thread {
                 .map(|repo| {
                     repo.update(cx, |repo, _| {
                         let current_branch =
-                            repo.branch.as_ref().map(|branch| branch.name.to_string());
+                            repo.branch.as_ref().map(|branch| branch.name().to_owned());
                         repo.send_job(None, |state, _| async move {
                             let RepositoryState::Local { backend, .. } = state else {
                                 return GitState {

crates/collab/src/tests/integration_tests.rs 🔗

@@ -2902,7 +2902,7 @@ async fn test_git_branch_name(
                 .read(cx)
                 .branch
                 .as_ref()
-                .map(|branch| branch.name.to_string()),
+                .map(|branch| branch.name().to_owned()),
             branch_name
         )
     }
@@ -6864,7 +6864,7 @@ async fn test_remote_git_branches(
 
     let branches_b = branches_b
         .into_iter()
-        .map(|branch| branch.name.to_string())
+        .map(|branch| branch.name().to_string())
         .collect::<HashSet<_>>();
 
     assert_eq!(branches_b, branches_set);
@@ -6895,7 +6895,7 @@ async fn test_remote_git_branches(
         })
     });
 
-    assert_eq!(host_branch.name, branches[2]);
+    assert_eq!(host_branch.name(), branches[2]);
 
     // Also try creating a new branch
     cx_b.update(|cx| {
@@ -6933,5 +6933,5 @@ async fn test_remote_git_branches(
         })
     });
 
-    assert_eq!(host_branch.name, "totally-new-branch");
+    assert_eq!(host_branch.name(), "totally-new-branch");
 }

crates/collab/src/tests/remote_editing_collaboration_tests.rs 🔗

@@ -293,7 +293,7 @@ async fn test_ssh_collaboration_git_branches(
 
     let branches_b = branches_b
         .into_iter()
-        .map(|branch| branch.name.to_string())
+        .map(|branch| branch.name().to_string())
         .collect::<HashSet<_>>();
 
     assert_eq!(&branches_b, &branches_set);
@@ -326,7 +326,7 @@ async fn test_ssh_collaboration_git_branches(
         })
     });
 
-    assert_eq!(server_branch.name, branches[2]);
+    assert_eq!(server_branch.name(), branches[2]);
 
     // Also try creating a new branch
     cx_b.update(|cx| {
@@ -366,7 +366,7 @@ async fn test_ssh_collaboration_git_branches(
         })
     });
 
-    assert_eq!(server_branch.name, "totally-new-branch");
+    assert_eq!(server_branch.name(), "totally-new-branch");
 
     // Remove the git repository and check that all participants get the update.
     remote_fs

crates/fs/src/fake_git_repo.rs 🔗

@@ -322,7 +322,7 @@ impl GitRepository for FakeGitRepository {
                 .iter()
                 .map(|branch_name| Branch {
                     is_head: Some(branch_name) == current_branch.as_ref(),
-                    name: branch_name.into(),
+                    ref_name: branch_name.into(),
                     most_recent_commit: None,
                     upstream: None,
                 })

crates/git/src/repository.rs 🔗

@@ -37,12 +37,24 @@ pub const REMOTE_CANCELLED_BY_USER: &str = "Operation cancelled by user";
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
 pub struct Branch {
     pub is_head: bool,
-    pub name: SharedString,
+    pub ref_name: SharedString,
     pub upstream: Option<Upstream>,
     pub most_recent_commit: Option<CommitSummary>,
 }
 
 impl Branch {
+    pub fn name(&self) -> &str {
+        self.ref_name
+            .as_ref()
+            .strip_prefix("refs/heads/")
+            .or_else(|| self.ref_name.as_ref().strip_prefix("refs/remotes/"))
+            .unwrap_or(self.ref_name.as_ref())
+    }
+
+    pub fn is_remote(&self) -> bool {
+        self.ref_name.starts_with("refs/remotes/")
+    }
+
     pub fn tracking_status(&self) -> Option<UpstreamTrackingStatus> {
         self.upstream
             .as_ref()
@@ -71,6 +83,10 @@ impl Upstream {
             .strip_prefix("refs/remotes/")
             .and_then(|stripped| stripped.split("/").next())
     }
+
+    pub fn stripped_ref_name(&self) -> Option<&str> {
+        self.ref_name.strip_prefix("refs/remotes/")
+    }
 }
 
 #[derive(Clone, Copy, Default)]
@@ -803,68 +819,69 @@ impl GitRepository for RealGitRepository {
     fn branches(&self) -> BoxFuture<Result<Vec<Branch>>> {
         let working_directory = self.working_directory();
         let git_binary_path = self.git_binary_path.clone();
-        async move {
-            let fields = [
-                "%(HEAD)",
-                "%(objectname)",
-                "%(parent)",
-                "%(refname)",
-                "%(upstream)",
-                "%(upstream:track)",
-                "%(committerdate:unix)",
-                "%(contents:subject)",
-            ]
-            .join("%00");
-            let args = vec![
-                "for-each-ref",
-                "refs/heads/**/*",
-                "refs/remotes/**/*",
-                "--format",
-                &fields,
-            ];
-            let working_directory = working_directory?;
-            let output = new_smol_command(&git_binary_path)
-                .current_dir(&working_directory)
-                .args(args)
-                .output()
-                .await?;
-
-            if !output.status.success() {
-                return Err(anyhow!(
-                    "Failed to git git branches:\n{}",
-                    String::from_utf8_lossy(&output.stderr)
-                ));
-            }
-
-            let input = String::from_utf8_lossy(&output.stdout);
-
-            let mut branches = parse_branch_input(&input)?;
-            if branches.is_empty() {
-                let args = vec!["symbolic-ref", "--quiet", "--short", "HEAD"];
-
+        self.executor
+            .spawn(async move {
+                let fields = [
+                    "%(HEAD)",
+                    "%(objectname)",
+                    "%(parent)",
+                    "%(refname)",
+                    "%(upstream)",
+                    "%(upstream:track)",
+                    "%(committerdate:unix)",
+                    "%(contents:subject)",
+                ]
+                .join("%00");
+                let args = vec![
+                    "for-each-ref",
+                    "refs/heads/**/*",
+                    "refs/remotes/**/*",
+                    "--format",
+                    &fields,
+                ];
+                let working_directory = working_directory?;
                 let output = new_smol_command(&git_binary_path)
                     .current_dir(&working_directory)
                     .args(args)
                     .output()
                     .await?;
 
-                // git symbolic-ref returns a non-0 exit code if HEAD points
-                // to something other than a branch
-                if output.status.success() {
-                    let name = String::from_utf8_lossy(&output.stdout).trim().to_string();
-
-                    branches.push(Branch {
-                        name: name.into(),
-                        is_head: true,
-                        upstream: None,
-                        most_recent_commit: None,
-                    });
+                if !output.status.success() {
+                    return Err(anyhow!(
+                        "Failed to git git branches:\n{}",
+                        String::from_utf8_lossy(&output.stderr)
+                    ));
                 }
-            }
 
-            Ok(branches)
-        }
-        .boxed()
+                let input = String::from_utf8_lossy(&output.stdout);
+
+                let mut branches = parse_branch_input(&input)?;
+                if branches.is_empty() {
+                    let args = vec!["symbolic-ref", "--quiet", "HEAD"];
+
+                    let output = new_smol_command(&git_binary_path)
+                        .current_dir(&working_directory)
+                        .args(args)
+                        .output()
+                        .await?;
+
+                    // git symbolic-ref returns a non-0 exit code if HEAD points
+                    // to something other than a branch
+                    if output.status.success() {
+                        let name = String::from_utf8_lossy(&output.stdout).trim().to_string();
+
+                        branches.push(Branch {
+                            ref_name: name.into(),
+                            is_head: true,
+                            upstream: None,
+                            most_recent_commit: None,
+                        });
+                    }
+                }
+
+                Ok(branches)
+            })
+            .boxed()
     }
 
     fn change_branch(&self, name: String) -> BoxFuture<Result<()>> {
@@ -1691,15 +1708,7 @@ fn parse_branch_input(input: &str) -> Result<Vec<Branch>> {
         let is_current_branch = fields.next().context("no HEAD")? == "*";
         let head_sha: SharedString = fields.next().context("no objectname")?.to_string().into();
         let parent_sha: SharedString = fields.next().context("no parent")?.to_string().into();
-        let raw_ref_name = fields.next().context("no refname")?;
-        let ref_name: SharedString =
-            if let Some(ref_name) = raw_ref_name.strip_prefix("refs/heads/") {
-                ref_name.to_string().into()
-            } else if let Some(ref_name) = raw_ref_name.strip_prefix("refs/remotes/") {
-                ref_name.to_string().into()
-            } else {
-                return Err(anyhow!("unexpected format for refname"));
-            };
+        let ref_name = fields.next().context("no refname")?.to_string().into();
         let upstream_name = fields.next().context("no upstream")?.to_string();
         let upstream_tracking = parse_upstream_track(fields.next().context("no upstream:track")?)?;
         let commiterdate = fields.next().context("no committerdate")?.parse::<i64>()?;
@@ -1711,7 +1720,7 @@ fn parse_branch_input(input: &str) -> Result<Vec<Branch>> {
 
         branches.push(Branch {
             is_head: is_current_branch,
-            name: ref_name,
+            ref_name: ref_name,
             most_recent_commit: Some(CommitSummary {
                 sha: head_sha,
                 subject,
@@ -1974,7 +1983,7 @@ mod tests {
             parse_branch_input(&input).unwrap(),
             vec![Branch {
                 is_head: true,
-                name: "zed-patches".into(),
+                ref_name: "refs/heads/zed-patches".into(),
                 upstream: Some(Upstream {
                     ref_name: "refs/remotes/origin/zed-patches".into(),
                     tracking: UpstreamTracking::Tracked(UpstreamTrackingStatus {

crates/git_ui/src/branch_picker.rs 🔗

@@ -1,6 +1,7 @@
 use anyhow::{Context as _, anyhow};
 use fuzzy::StringMatchCandidate;
 
+use collections::HashSet;
 use git::repository::Branch;
 use gpui::{
     App, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, InteractiveElement,
@@ -95,12 +96,28 @@ impl BranchList {
                 .context("No active repository")?
                 .await??;
 
-            all_branches.sort_by_key(|branch| {
-                branch
-                    .most_recent_commit
-                    .as_ref()
-                    .map(|commit| 0 - commit.commit_timestamp)
-            });
+            let all_branches = cx
+                .background_spawn(async move {
+                    let upstreams: HashSet<_> = all_branches
+                        .iter()
+                        .filter_map(|branch| {
+                            let upstream = branch.upstream.as_ref()?;
+                            Some(upstream.ref_name.clone())
+                        })
+                        .collect();
+
+                    all_branches.retain(|branch| !upstreams.contains(&branch.ref_name));
+
+                    all_branches.sort_by_key(|branch| {
+                        branch
+                            .most_recent_commit
+                            .as_ref()
+                            .map(|commit| 0 - commit.commit_timestamp)
+                    });
+
+                    all_branches
+                })
+                .await;
 
             this.update_in(cx, |this, window, cx| {
                 this.picker.update(cx, |picker, cx| {
@@ -266,6 +283,7 @@ impl PickerDelegate for BranchListDelegate {
             let mut matches: Vec<BranchEntry> = if query.is_empty() {
                 all_branches
                     .into_iter()
+                    .filter(|branch| !branch.is_remote())
                     .take(RECENT_BRANCHES_COUNT)
                     .map(|branch| BranchEntry {
                         branch,
@@ -277,7 +295,7 @@ impl PickerDelegate for BranchListDelegate {
                 let candidates = all_branches
                     .iter()
                     .enumerate()
-                    .map(|(ix, command)| StringMatchCandidate::new(ix, &command.name.clone()))
+                    .map(|(ix, branch)| StringMatchCandidate::new(ix, branch.name()))
                     .collect::<Vec<StringMatchCandidate>>();
                 fuzzy::match_strings(
                     &candidates,
@@ -303,11 +321,11 @@ impl PickerDelegate for BranchListDelegate {
                     if !query.is_empty()
                         && !matches
                             .first()
-                            .is_some_and(|entry| entry.branch.name == query)
+                            .is_some_and(|entry| entry.branch.name() == query)
                     {
                         matches.push(BranchEntry {
                             branch: Branch {
-                                name: query.clone().into(),
+                                ref_name: format!("refs/heads/{query}").into(),
                                 is_head: false,
                                 upstream: None,
                                 most_recent_commit: None,
@@ -335,19 +353,19 @@ impl PickerDelegate for BranchListDelegate {
             return;
         };
         if entry.is_new {
-            self.create_branch(entry.branch.name.clone(), window, cx);
+            self.create_branch(entry.branch.name().to_owned().into(), window, cx);
             return;
         }
 
         let current_branch = self.repo.as_ref().map(|repo| {
             repo.update(cx, |repo, _| {
-                repo.branch.as_ref().map(|branch| branch.name.clone())
+                repo.branch.as_ref().map(|branch| branch.ref_name.clone())
             })
         });
 
         if current_branch
             .flatten()
-            .is_some_and(|current_branch| current_branch == entry.branch.name)
+            .is_some_and(|current_branch| current_branch == entry.branch.ref_name)
         {
             cx.emit(DismissEvent);
             return;
@@ -368,7 +386,7 @@ impl PickerDelegate for BranchListDelegate {
 
                     anyhow::Ok(async move {
                         repo.update(&mut cx, |repo, _| {
-                            repo.change_branch(branch.name.to_string())
+                            repo.change_branch(branch.name().to_string())
                         })?
                         .await?
                     })
@@ -443,13 +461,13 @@ impl PickerDelegate for BranchListDelegate {
                                     if entry.is_new {
                                         Label::new(format!(
                                             "Create branch \"{}\"…",
-                                            entry.branch.name
+                                            entry.branch.name()
                                         ))
                                         .single_line()
                                         .into_any_element()
                                     } else {
                                         HighlightedLabel::new(
-                                            entry.branch.name.clone(),
+                                            entry.branch.name().to_owned(),
                                             entry.positions.clone(),
                                         )
                                         .truncate()
@@ -470,7 +488,7 @@ impl PickerDelegate for BranchListDelegate {
                                 let message = if entry.is_new {
                                     if let Some(current_branch) =
                                         self.repo.as_ref().and_then(|repo| {
-                                            repo.read(cx).branch.as_ref().map(|b| b.name.clone())
+                                            repo.read(cx).branch.as_ref().map(|b| b.name())
                                         })
                                     {
                                         format!("based off {}", current_branch)

crates/git_ui/src/commit_modal.rs 🔗

@@ -321,8 +321,8 @@ impl CommitModal {
         let branch = active_repo
             .as_ref()
             .and_then(|repo| repo.read(cx).branch.as_ref())
-            .map(|b| b.name.clone())
-            .unwrap_or_else(|| "<no branch>".into());
+            .map(|b| b.name().to_owned())
+            .unwrap_or_else(|| "<no branch>".to_owned());
 
         let branch_picker_button = panel_button(branch)
             .icon(IconName::GitBranch)

crates/git_ui/src/git_panel.rs 🔗

@@ -1953,7 +1953,12 @@ impl GitPanel {
             })?;
 
             let pull = repo.update(cx, |repo, cx| {
-                repo.pull(branch.name.clone(), remote.name.clone(), askpass, cx)
+                repo.pull(
+                    branch.name().to_owned().into(),
+                    remote.name.clone(),
+                    askpass,
+                    cx,
+                )
             })?;
 
             let remote_message = pull.await?;
@@ -2020,7 +2025,7 @@ impl GitPanel {
 
             let push = repo.update(cx, |repo, cx| {
                 repo.push(
-                    branch.name.clone(),
+                    branch.name().to_owned().into(),
                     remote.name.clone(),
                     options,
                     askpass_delegate,
@@ -2030,7 +2035,7 @@ impl GitPanel {
 
             let remote_output = push.await?;
 
-            let action = RemoteAction::Push(branch.name, remote);
+            let action = RemoteAction::Push(branch.name().to_owned().into(), remote);
             this.update(cx, |this, cx| match remote_output {
                 Ok(remote_message) => this.show_remote_output(action, remote_message, cx),
                 Err(e) => {
@@ -2092,7 +2097,7 @@ impl GitPanel {
                         return Err(anyhow::anyhow!("No active branch"));
                     };
 
-                    Ok(repo.get_remotes(Some(current_branch.name.to_string())))
+                    Ok(repo.get_remotes(Some(current_branch.name().to_string())))
                 })??
                 .await??;
 
@@ -4363,19 +4368,17 @@ impl RenderOnce for PanelRepoFooter {
         let branch_name = self
             .branch
             .as_ref()
-            .map(|branch| branch.name.clone())
+            .map(|branch| branch.name().to_owned())
             .or_else(|| {
                 self.head_commit.as_ref().map(|commit| {
-                    SharedString::from(
-                        commit
-                            .sha
-                            .chars()
-                            .take(MAX_SHORT_SHA_LEN)
-                            .collect::<String>(),
-                    )
+                    commit
+                        .sha
+                        .chars()
+                        .take(MAX_SHORT_SHA_LEN)
+                        .collect::<String>()
                 })
             })
-            .unwrap_or_else(|| SharedString::from(" (no branch)"));
+            .unwrap_or_else(|| " (no branch)".to_owned());
         let show_separator = self.branch.is_some() || self.head_commit.is_some();
 
         let active_repo_name = self.active_repository.clone();
@@ -4542,7 +4545,7 @@ impl Component for PanelRepoFooter {
         fn branch(upstream: Option<UpstreamTracking>) -> Branch {
             Branch {
                 is_head: true,
-                name: "some-branch".into(),
+                ref_name: "some-branch".into(),
                 upstream: upstream.map(|tracking| Upstream {
                     ref_name: "origin/some-branch".into(),
                     tracking,
@@ -4559,7 +4562,7 @@ impl Component for PanelRepoFooter {
         fn custom(branch_name: &str, upstream: Option<UpstreamTracking>) -> Branch {
             Branch {
                 is_head: true,
-                name: branch_name.to_string().into(),
+                ref_name: branch_name.to_string().into(),
                 upstream: upstream.map(|tracking| Upstream {
                     ref_name: format!("zed/{}", branch_name).into(),
                     tracking,

crates/git_ui/src/project_diff.rs 🔗

@@ -1099,7 +1099,7 @@ impl RenderOnce for ProjectDiffEmptyState {
                             v_flex()
                                 .child(Headline::new(ahead_string).size(HeadlineSize::Small))
                                 .child(
-                                    Label::new(format!("Push your changes to {}", branch.name))
+                                    Label::new(format!("Push your changes to {}", branch.name()))
                                         .color(Color::Muted),
                                 ),
                         )
@@ -1113,7 +1113,7 @@ impl RenderOnce for ProjectDiffEmptyState {
                             v_flex()
                                 .child(Headline::new("Publish Branch").size(HeadlineSize::Small))
                                 .child(
-                                    Label::new(format!("Create {} on remote", branch.name))
+                                    Label::new(format!("Create {} on remote", branch.name()))
                                         .color(Color::Muted),
                                 ),
                         )
@@ -1183,7 +1183,7 @@ mod preview {
             fn branch(upstream: Option<UpstreamTracking>) -> Branch {
                 Branch {
                     is_head: true,
-                    name: "some-branch".into(),
+                    ref_name: "some-branch".into(),
                     upstream: upstream.map(|tracking| Upstream {
                         ref_name: "origin/some-branch".into(),
                         tracking,

crates/project/src/git_store.rs 🔗

@@ -3790,13 +3790,9 @@ impl Repository {
 
     pub fn branches(&mut self) -> oneshot::Receiver<Result<Vec<Branch>>> {
         let id = self.id;
-        self.send_job(None, move |repo, cx| async move {
+        self.send_job(None, move |repo, _| async move {
             match repo {
-                RepositoryState::Local { backend, .. } => {
-                    let backend = backend.clone();
-                    cx.background_spawn(async move { backend.branches().await })
-                        .await
-                }
+                RepositoryState::Local { backend, .. } => backend.branches().await,
                 RepositoryState::Remote { project_id, client } => {
                     let response = client
                         .request(proto::GitGetBranches {
@@ -4460,7 +4456,7 @@ fn deserialize_blame_buffer_response(
 fn branch_to_proto(branch: &git::repository::Branch) -> proto::Branch {
     proto::Branch {
         is_head: branch.is_head,
-        name: branch.name.to_string(),
+        ref_name: branch.ref_name.to_string(),
         unix_timestamp: branch
             .most_recent_commit
             .as_ref()
@@ -4489,7 +4485,7 @@ fn branch_to_proto(branch: &git::repository::Branch) -> proto::Branch {
 fn proto_to_branch(proto: &proto::Branch) -> git::repository::Branch {
     git::repository::Branch {
         is_head: proto.is_head,
-        name: proto.name.clone().into(),
+        ref_name: proto.ref_name.clone().into(),
         upstream: proto
             .upstream
             .as_ref()

crates/proto/proto/git.proto 🔗

@@ -75,7 +75,7 @@ message GetPermalinkToLineResponse {
 
 message Branch {
     bool is_head = 1;
-    string name = 2;
+    string ref_name = 2;
     optional uint64 unix_timestamp = 3;
     optional GitUpstream upstream = 4;
     optional CommitSummary most_recent_commit = 5;

crates/remote_server/src/remote_editing_tests.rs 🔗

@@ -1472,7 +1472,7 @@ async fn test_remote_git_branches(cx: &mut TestAppContext, server_cx: &mut TestA
 
     let remote_branches = remote_branches
         .into_iter()
-        .map(|branch| branch.name.to_string())
+        .map(|branch| branch.name().to_string())
         .collect::<HashSet<_>>();
 
     assert_eq!(&remote_branches, &branches_set);
@@ -1505,7 +1505,7 @@ async fn test_remote_git_branches(cx: &mut TestAppContext, server_cx: &mut TestA
         })
     });
 
-    assert_eq!(server_branch.name, branches[2]);
+    assert_eq!(server_branch.name(), branches[2]);
 
     // Also try creating a new branch
     cx.update(|cx| {
@@ -1545,7 +1545,7 @@ async fn test_remote_git_branches(cx: &mut TestAppContext, server_cx: &mut TestA
         })
     });
 
-    assert_eq!(server_branch.name, "totally-new-branch");
+    assert_eq!(server_branch.name(), "totally-new-branch");
 }
 
 pub async fn init_test(

crates/title_bar/src/title_bar.rs 🔗

@@ -518,7 +518,7 @@ impl TitleBar {
             let repo = repository.read(cx);
             repo.branch
                 .as_ref()
-                .map(|branch| branch.name.clone())
+                .map(|branch| branch.name())
                 .map(|name| util::truncate_and_trailoff(&name, MAX_BRANCH_NAME_LENGTH))
                 .or_else(|| {
                     repo.head_commit.as_ref().map(|commit| {