From 2606a1073d3cba0086f9cb6a1d86cdea482c79c5 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Sat, 8 Jul 2023 02:14:23 +0900
Subject: [PATCH] git: extract helper that merges pseudo @git targets in
 branches map

---
 lib/src/git.rs         | 27 +++++++++++++++++++++++++--
 src/commands/branch.rs | 34 ++++++++++++----------------------
 2 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/lib/src/git.rs b/lib/src/git.rs
index 2ec71d670..8d4b3d0b1 100644
--- a/lib/src/git.rs
+++ b/lib/src/git.rs
@@ -24,7 +24,7 @@ use thiserror::Error;
 
 use crate::backend::{CommitId, ObjectId};
 use crate::git_backend::NO_GC_REF_NAMESPACE;
-use crate::op_store::RefTarget;
+use crate::op_store::{BranchTarget, RefTarget};
 use crate::repo::{MutableRepo, Repo};
 use crate::revset;
 use crate::settings::GitSettings;
@@ -109,7 +109,30 @@ fn resolve_git_ref_to_commit_id(
     Some(CommitId::from_bytes(git_commit.id().as_bytes()))
 }
 
-pub fn local_branch_git_tracking_refs(view: &View) -> impl Iterator<Item = (&str, &RefTarget)> {
+/// Builds a map of branches which also includes pseudo `@git` remote.
+///
+/// If there's an existing remote named `git`, a list of conflicting branch
+/// names will be returned.
+pub fn build_unified_branches_map(view: &View) -> (BTreeMap<String, BranchTarget>, Vec<String>) {
+    let mut all_branches = view.branches().clone();
+    let mut bad_branch_names = Vec::new();
+    for (branch_name, git_tracking_target) in local_branch_git_tracking_refs(view) {
+        let branch_target = all_branches.entry(branch_name.to_owned()).or_default();
+        if branch_target.remote_targets.contains_key("git") {
+            // TODO(#1690): There should be a mechanism to prevent importing a
+            // remote named "git" in `jj git import`.
+            bad_branch_names.push(branch_name.to_owned());
+        } else {
+            // TODO: `BTreeMap::try_insert` could be used here once that's stabilized.
+            branch_target
+                .remote_targets
+                .insert("git".to_owned(), git_tracking_target.clone());
+        }
+    }
+    (all_branches, bad_branch_names)
+}
+
+fn local_branch_git_tracking_refs(view: &View) -> impl Iterator<Item = (&str, &RefTarget)> {
     view.git_refs().iter().filter_map(|(ref_name, target)| {
         ref_name
             .strip_prefix("refs/heads/")
diff --git a/src/commands/branch.rs b/src/commands/branch.rs
index 9e255a91f..ed2d4572a 100644
--- a/src/commands/branch.rs
+++ b/src/commands/branch.rs
@@ -3,7 +3,7 @@ use std::collections::{BTreeSet, HashSet};
 use clap::builder::NonEmptyStringValueParser;
 use itertools::Itertools;
 use jujutsu_lib::backend::{CommitId, ObjectId};
-use jujutsu_lib::git::local_branch_git_tracking_refs;
+use jujutsu_lib::git;
 use jujutsu_lib::op_store::{BranchTarget, RefTarget};
 use jujutsu_lib::repo::Repo;
 use jujutsu_lib::revset::{self, RevsetExpression};
@@ -322,27 +322,17 @@ fn cmd_branch_list(
 ) -> Result<(), CommandError> {
     let workspace_command = command.workspace_helper(ui)?;
     let repo = workspace_command.repo();
-
-    let mut all_branches = repo.view().branches().clone();
-    for (branch_name, git_tracking_target) in local_branch_git_tracking_refs(repo.view()) {
-        let branch_target = all_branches.entry(branch_name.to_owned()).or_default();
-        if branch_target.remote_targets.contains_key("git") {
-            // TODO(#1690): There should be a mechanism to prevent importing a
-            // remote named "git" in `jj git import`.
-            // TODO: This is not currently tested
-            writeln!(
-                ui.warning(),
-                "WARNING: Branch {branch_name} has a remote-tracking branch for a remote named \
-                 `git`. Local-git tracking branches for it will not be shown.\nIt is recommended \
-                 to rename that remote, as jj normally reserves the `@git` suffix to denote \
-                 local-git tracking branches."
-            )?;
-        } else {
-            // TODO: `BTreeMap::try_insert` could be used here once that's stabilized.
-            branch_target
-                .remote_targets
-                .insert("git".to_string(), git_tracking_target.clone());
-        }
+    let (mut all_branches, bad_branch_names) = git::build_unified_branches_map(repo.view());
+    if !bad_branch_names.is_empty() {
+        // TODO: This is not currently tested
+        writeln!(
+            ui.warning(),
+            "WARNING: Branch {branch_name} has a remote-tracking branch for a remote named `git`. \
+             Local-git tracking branches for it will not be shown.\nIt is recommended to rename \
+             that remote, as jj normally reserves the `@git` suffix to denote local-git tracking \
+             branches.",
+            branch_name = bad_branch_names.join(", "),
+        )?;
     }
 
     if !args.revisions.is_empty() {