view: unify set/remove_remote_branch() functions to take Option<RefTarget>

lib/src/git.rs

@@ -540,7 +540,7 @@ pub fn remove_remote(
         .filter_map(|r| r.starts_with(&prefix).then(|| r.clone()))
         .collect_vec();
     for branch in branches_to_delete {
-        mut_repo.remove_remote_branch(&branch, remote_name);
+        mut_repo.set_remote_branch_target(&branch, remote_name, None);
     }
     for git_ref in git_refs_to_delete {
         mut_repo.set_git_ref_target(&git_ref, None);

lib/src/repo.rs

@@ -983,12 +983,14 @@ impl MutableRepo {
             .with_ref(|v| v.get_remote_branch(name, remote_name))
     }
 
-    pub fn set_remote_branch(&mut self, name: String, remote_name: String, target: RefTarget) {
-        self.view_mut().set_remote_branch(name, remote_name, target);
-    }
-
-    pub fn remove_remote_branch(&mut self, name: &str, remote_name: &str) {
-        self.view_mut().remove_remote_branch(name, remote_name);
+    pub fn set_remote_branch_target(
+        &mut self,
+        name: &str,
+        remote_name: &str,
+        target: Option<RefTarget>,
+    ) {
+        self.view_mut()
+            .set_remote_branch_target(name, remote_name, target);
     }
 
     pub fn rename_remote(&mut self, old: &str, new: &str) {

lib/src/view.rs

@@ -142,7 +142,7 @@ impl View {
                     self.set_local_branch(name, target);
                 }
                 RefName::RemoteBranch { branch, remote } => {
-                    self.set_remote_branch(branch, remote, target);
+                    self.set_remote_branch_target(&branch, &remote, Some(target));
                 }
                 RefName::Tag(name) => {
                     self.set_tag_target(&name, Some(target));
@@ -157,7 +157,7 @@ impl View {
                     self.remove_local_branch(&name);
                 }
                 RefName::RemoteBranch { branch, remote } => {
-                    self.remove_remote_branch(&branch, &remote);
+                    self.set_remote_branch_target(&branch, &remote, None);
                 }
                 RefName::Tag(name) => {
                     self.set_tag_target(&name, None);
@@ -208,7 +208,22 @@ impl View {
             .and_then(|branch_target| branch_target.remote_targets.get(remote_name).cloned())
     }
 
-    pub fn set_remote_branch(&mut self, name: String, remote_name: String, target: RefTarget) {
+    /// Sets remote-tracking branch to point to the given target. If the target
+    /// is absent, the branch will be removed.
+    pub fn set_remote_branch_target(
+        &mut self,
+        name: &str,
+        remote_name: &str,
+        target: Option<RefTarget>,
+    ) {
+        if let Some(target) = target {
+            self.insert_remote_branch(name.to_owned(), remote_name.to_owned(), target);
+        } else {
+            self.remove_remote_branch(name, remote_name);
+        }
+    }
+
+    fn insert_remote_branch(&mut self, name: String, remote_name: String, target: RefTarget) {
         self.data
             .branches
             .entry(name)
@@ -217,7 +232,7 @@ impl View {
             .insert(remote_name, target);
     }
 
-    pub fn remove_remote_branch(&mut self, name: &str, remote_name: &str) {
+    fn remove_remote_branch(&mut self, name: &str, remote_name: &str) {
         if let Some(branch) = self.data.branches.get_mut(name) {
             branch.remote_targets.remove(remote_name);
             if branch.remote_targets.is_empty() && branch.local_target.is_none() {

lib/tests/test_mut_repo.rs

@@ -473,10 +473,10 @@ fn test_has_changed(use_git: bool) {
         .set_wc_commit(ws_id.clone(), commit1.id().clone())
         .unwrap();
     mut_repo.set_local_branch("main".to_string(), RefTarget::Normal(commit1.id().clone()));
-    mut_repo.set_remote_branch(
-        "main".to_string(),
-        "origin".to_string(),
-        RefTarget::Normal(commit1.id().clone()),
+    mut_repo.set_remote_branch_target(
+        "main",
+        "origin",
+        Some(RefTarget::Normal(commit1.id().clone())),
     );
     let repo = tx.commit();
     // Test the setup
@@ -492,17 +492,17 @@ fn test_has_changed(use_git: bool) {
         .set_wc_commit(ws_id.clone(), commit1.id().clone())
         .unwrap();
     mut_repo.set_local_branch("main".to_string(), RefTarget::Normal(commit1.id().clone()));
-    mut_repo.set_remote_branch(
-        "main".to_string(),
-        "origin".to_string(),
-        RefTarget::Normal(commit1.id().clone()),
+    mut_repo.set_remote_branch_target(
+        "main",
+        "origin",
+        Some(RefTarget::Normal(commit1.id().clone())),
     );
     assert!(!mut_repo.has_changes());
 
     mut_repo.remove_public_head(commit2.id());
     mut_repo.remove_head(commit2.id());
     mut_repo.remove_local_branch("stable");
-    mut_repo.remove_remote_branch("stable", "origin");
+    mut_repo.set_remote_branch_target("stable", "origin", None);
     assert!(!mut_repo.has_changes());
 
     mut_repo.add_head(&commit2);
@@ -532,16 +532,16 @@ fn test_has_changed(use_git: bool) {
     mut_repo.set_local_branch("main".to_string(), RefTarget::Normal(commit1.id().clone()));
     assert!(!mut_repo.has_changes());
 
-    mut_repo.set_remote_branch(
-        "main".to_string(),
-        "origin".to_string(),
-        RefTarget::Normal(commit2.id().clone()),
+    mut_repo.set_remote_branch_target(
+        "main",
+        "origin",
+        Some(RefTarget::Normal(commit2.id().clone())),
     );
     assert!(mut_repo.has_changes());
-    mut_repo.set_remote_branch(
-        "main".to_string(),
-        "origin".to_string(),
-        RefTarget::Normal(commit1.id().clone()),
+    mut_repo.set_remote_branch_target(
+        "main",
+        "origin",
+        Some(RefTarget::Normal(commit1.id().clone())),
     );
     assert!(!mut_repo.has_changes());
 }
@@ -629,9 +629,9 @@ fn test_rename_remote(use_git: bool) {
     let mut tx = repo.start_transaction(&settings, "test");
     let mut_repo = tx.mut_repo();
     let commit = write_random_commit(mut_repo, &settings);
-    let target = RefTarget::Normal(commit.id().clone());
-    mut_repo.set_remote_branch("main".to_string(), "origin".to_string(), target.clone());
+    let target = Some(RefTarget::Normal(commit.id().clone()));
+    mut_repo.set_remote_branch_target("main", "origin", target.clone());
     mut_repo.rename_remote("origin", "upstream");
-    assert_eq!(mut_repo.get_remote_branch("main", "upstream"), Some(target));
+    assert_eq!(mut_repo.get_remote_branch("main", "upstream"), target);
     assert_eq!(mut_repo.get_remote_branch("main", "origin"), None);
 }

lib/tests/test_revset.rs

@@ -426,24 +426,24 @@ fn test_resolve_symbol_branches() {
     let commit5 = write_random_commit(mut_repo, &settings);
 
     mut_repo.set_local_branch("local".to_owned(), RefTarget::Normal(commit1.id().clone()));
-    mut_repo.set_remote_branch(
-        "remote".to_owned(),
-        "origin".to_owned(),
-        RefTarget::Normal(commit2.id().clone()),
+    mut_repo.set_remote_branch_target(
+        "remote",
+        "origin",
+        Some(RefTarget::Normal(commit2.id().clone())),
     );
     mut_repo.set_local_branch(
         "local-remote".to_owned(),
         RefTarget::Normal(commit3.id().clone()),
     );
-    mut_repo.set_remote_branch(
-        "local-remote".to_owned(),
-        "origin".to_owned(),
-        RefTarget::Normal(commit4.id().clone()),
+    mut_repo.set_remote_branch_target(
+        "local-remote",
+        "origin",
+        Some(RefTarget::Normal(commit4.id().clone())),
     );
-    mut_repo.set_remote_branch(
-        "local-remote".to_owned(),
-        "mirror".to_owned(),
-        mut_repo.get_local_branch("local-remote").unwrap(),
+    mut_repo.set_remote_branch_target(
+        "local-remote",
+        "mirror",
+        mut_repo.get_local_branch("local-remote"),
     );
     mut_repo.set_git_ref_target(
         "refs/heads/local-remote",
@@ -457,13 +457,13 @@ fn test_resolve_symbol_branches() {
             adds: vec![commit3.id().clone(), commit2.id().clone()],
         },
     );
-    mut_repo.set_remote_branch(
-        "remote-conflicted".to_owned(),
-        "origin".to_owned(),
-        RefTarget::Conflict {
+    mut_repo.set_remote_branch_target(
+        "remote-conflicted",
+        "origin",
+        Some(RefTarget::Conflict {
             removes: vec![commit3.id().clone()],
             adds: vec![commit5.id().clone(), commit4.id().clone()],
-        },
+        }),
     );
 
     // Local only
@@ -1779,15 +1779,15 @@ fn test_evaluate_expression_remote_branches(use_git: bool) {
     // Can get branches when there are none
     assert_eq!(resolve_commit_ids(mut_repo, "remote_branches()"), vec![]);
     // Can get a few branches
-    mut_repo.set_remote_branch(
-        "branch1".to_string(),
-        "origin".to_string(),
-        RefTarget::Normal(commit1.id().clone()),
+    mut_repo.set_remote_branch_target(
+        "branch1",
+        "origin",
+        Some(RefTarget::Normal(commit1.id().clone())),
     );
-    mut_repo.set_remote_branch(
-        "branch2".to_string(),
-        "private".to_string(),
-        RefTarget::Normal(commit2.id().clone()),
+    mut_repo.set_remote_branch_target(
+        "branch2",
+        "private",
+        Some(RefTarget::Normal(commit2.id().clone())),
     );
     assert_eq!(
         resolve_commit_ids(mut_repo, "remote_branches()"),
@@ -1835,10 +1835,10 @@ fn test_evaluate_expression_remote_branches(use_git: bool) {
     );
     // Two branches pointing to the same commit does not result in a duplicate in
     // the revset
-    mut_repo.set_remote_branch(
-        "branch3".to_string(),
-        "origin".to_string(),
-        RefTarget::Normal(commit2.id().clone()),
+    mut_repo.set_remote_branch_target(
+        "branch3",
+        "origin",
+        Some(RefTarget::Normal(commit2.id().clone())),
     );
     assert_eq!(
         resolve_commit_ids(mut_repo, "remote_branches()"),
@@ -1851,23 +1851,23 @@ fn test_evaluate_expression_remote_branches(use_git: bool) {
         vec![commit2.id().clone(), commit1.id().clone()]
     );
     // Can get branches when there are conflicted refs
-    mut_repo.set_remote_branch(
-        "branch1".to_string(),
-        "origin".to_string(),
-        RefTarget::Conflict {
+    mut_repo.set_remote_branch_target(
+        "branch1",
+        "origin",
+        Some(RefTarget::Conflict {
             removes: vec![commit1.id().clone()],
             adds: vec![commit2.id().clone(), commit3.id().clone()],
-        },
+        }),
     );
-    mut_repo.set_remote_branch(
-        "branch2".to_string(),
-        "private".to_string(),
-        RefTarget::Conflict {
+    mut_repo.set_remote_branch_target(
+        "branch2",
+        "private",
+        Some(RefTarget::Conflict {
             removes: vec![commit2.id().clone()],
             adds: vec![commit3.id().clone(), commit4.id().clone()],
-        },
+        }),
     );
-    mut_repo.remove_remote_branch("branch3", "origin");
+    mut_repo.set_remote_branch_target("branch3", "origin", None);
     assert_eq!(
         resolve_commit_ids(mut_repo, "remote_branches()"),
         vec![

lib/tests/test_rewrite.rs

@@ -1025,10 +1025,10 @@ fn test_rebase_descendants_basic_branch_update_with_non_local_branch() {
     let commit_b = graph_builder.commit_with_parents(&[&commit_a]);
     tx.mut_repo()
         .set_local_branch("main".to_string(), RefTarget::Normal(commit_b.id().clone()));
-    tx.mut_repo().set_remote_branch(
-        "main".to_string(),
-        "origin".to_string(),
-        RefTarget::Normal(commit_b.id().clone()),
+    tx.mut_repo().set_remote_branch_target(
+        "main",
+        "origin",
+        Some(RefTarget::Normal(commit_b.id().clone())),
     );
     tx.mut_repo()
         .set_tag_target("v1", Some(RefTarget::Normal(commit_b.id().clone())));

lib/tests/test_view.rs

@@ -252,15 +252,15 @@ fn test_merge_views_branches() {
         "main".to_string(),
         RefTarget::Normal(main_branch_local_tx0.id().clone()),
     );
-    mut_repo.set_remote_branch(
-        "main".to_string(),
-        "origin".to_string(),
-        RefTarget::Normal(main_branch_origin_tx0.id().clone()),
+    mut_repo.set_remote_branch_target(
+        "main",
+        "origin",
+        Some(RefTarget::Normal(main_branch_origin_tx0.id().clone())),
     );
-    mut_repo.set_remote_branch(
-        "main".to_string(),
-        "alternate".to_string(),
-        RefTarget::Normal(main_branch_alternate_tx0.id().clone()),
+    mut_repo.set_remote_branch_target(
+        "main",
+        "alternate",
+        Some(RefTarget::Normal(main_branch_alternate_tx0.id().clone())),
     );
     let feature_branch_local_tx0 = write_random_commit(mut_repo, &settings);
     mut_repo.set_git_ref_target(
@@ -275,10 +275,10 @@ fn test_merge_views_branches() {
         "main".to_string(),
         RefTarget::Normal(main_branch_local_tx1.id().clone()),
     );
-    tx1.mut_repo().set_remote_branch(
-        "main".to_string(),
-        "origin".to_string(),
-        RefTarget::Normal(main_branch_origin_tx1.id().clone()),
+    tx1.mut_repo().set_remote_branch_target(
+        "main",
+        "origin",
+        Some(RefTarget::Normal(main_branch_origin_tx1.id().clone())),
     );
     let feature_branch_tx1 = write_random_commit(tx1.mut_repo(), &settings);
     tx1.mut_repo().set_local_branch(
@@ -293,10 +293,10 @@ fn test_merge_views_branches() {
         "main".to_string(),
         RefTarget::Normal(main_branch_local_tx2.id().clone()),
     );
-    tx2.mut_repo().set_remote_branch(
-        "main".to_string(),
-        "origin".to_string(),
-        RefTarget::Normal(main_branch_origin_tx1.id().clone()),
+    tx2.mut_repo().set_remote_branch_target(
+        "main",
+        "origin",
+        Some(RefTarget::Normal(main_branch_origin_tx1.id().clone())),
     );
     tx2.commit();