From 78eea06c36604a5a5c5b9ddb2319656c0a9dac7c Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 11 Jul 2023 23:01:54 +0900 Subject: [PATCH] view: unify set/remove_remote_branch() functions to take Option --- lib/src/git.rs | 2 +- lib/src/repo.rs | 14 ++++--- lib/src/view.rs | 23 +++++++++-- lib/tests/test_mut_repo.rs | 40 +++++++++---------- lib/tests/test_revset.rs | 80 +++++++++++++++++++------------------- lib/tests/test_rewrite.rs | 8 ++-- lib/tests/test_view.rs | 32 +++++++-------- 7 files changed, 108 insertions(+), 91 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index 4064d443a..cba5a0991 100644 --- a/lib/src/git.rs +++ b/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); diff --git a/lib/src/repo.rs b/lib/src/repo.rs index c597422d0..30a026003 100644 --- a/lib/src/repo.rs +++ b/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, + ) { + self.view_mut() + .set_remote_branch_target(name, remote_name, target); } pub fn rename_remote(&mut self, old: &str, new: &str) { diff --git a/lib/src/view.rs b/lib/src/view.rs index ba8ddadeb..d3e39b50d 100644 --- a/lib/src/view.rs +++ b/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, + ) { + 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() { diff --git a/lib/tests/test_mut_repo.rs b/lib/tests/test_mut_repo.rs index 446f96a34..f37722443 100644 --- a/lib/tests/test_mut_repo.rs +++ b/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); } diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 91a3bedf1..dedc18628 100644 --- a/lib/tests/test_revset.rs +++ b/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![ diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index a83f2f556..09e337a86 100644 --- a/lib/tests/test_rewrite.rs +++ b/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()))); diff --git a/lib/tests/test_view.rs b/lib/tests/test_view.rs index c9564468d..ff1409be3 100644 --- a/lib/tests/test_view.rs +++ b/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();