From 5218591a8224c18da85d81167bb8209f03929766 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 11 Jul 2023 22:25:27 +0900 Subject: [PATCH] view: unify set/clear_git_head() functions to take Option If we migrate RefTarget to new Conflict-based type, it won't store Conflict, but Conflict>. As the Option will be internalized, new RefTarget type will also represent an absent target. The 'target: Option' argument will be replaced with new RefTarget type. I've also renamed the function for consistency with the following changes. It would be surprising if set_local_branch(name, target) could remove the branch. I feel the name set_local_branch_target() is less confusing. --- lib/src/git.rs | 4 ++-- lib/src/repo.rs | 17 +++++------------ lib/src/view.rs | 10 ++++------ lib/tests/test_revset.rs | 4 ++-- lib/tests/test_view.rs | 6 +++--- src/cli_util.rs | 2 +- 6 files changed, 17 insertions(+), 26 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index e3144dc75..b30f57527 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -203,10 +203,10 @@ pub fn import_some_refs( let head_commit = store.get_commit(&head_commit_id).unwrap(); prevent_gc(git_repo, &head_commit_id)?; mut_repo.add_head(&head_commit); - mut_repo.set_git_head(RefTarget::Normal(head_commit_id)); + mut_repo.set_git_head_target(Some(RefTarget::Normal(head_commit_id))); } } else { - mut_repo.clear_git_head(); + mut_repo.set_git_head_target(None); } let mut changed_git_refs = BTreeMap::new(); diff --git a/lib/src/repo.rs b/lib/src/repo.rs index dc24e2558..af4be2eda 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -1023,12 +1023,8 @@ impl MutableRepo { self.view.with_ref(|v| v.git_head().cloned()) } - pub fn set_git_head(&mut self, target: RefTarget) { - self.view_mut().set_git_head(target); - } - - pub fn clear_git_head(&mut self) { - self.view_mut().clear_git_head(); + pub fn set_git_head_target(&mut self, target: Option) { + self.view_mut().set_git_head_target(target); } pub fn set_view(&mut self, data: op_store::View) { @@ -1152,16 +1148,13 @@ impl MutableRepo { ); } - if let Some(new_git_head) = merge_ref_targets( + let new_git_head_target = merge_ref_targets( self.index(), self.view().git_head(), base.git_head(), other.git_head(), - ) { - self.set_git_head(new_git_head); - } else { - self.clear_git_head(); - } + ); + self.set_git_head_target(new_git_head_target); } /// Finds and records commits that were rewritten or abandoned between diff --git a/lib/src/view.rs b/lib/src/view.rs index f49d16b16..78f0c26a5 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -258,12 +258,10 @@ impl View { self.data.git_refs.remove(name); } - pub fn set_git_head(&mut self, target: RefTarget) { - self.data.git_head = Some(target); - } - - pub fn clear_git_head(&mut self) { - self.data.git_head = None; + /// Sets `HEAD@git` to point to the given target. If the target is absent, + /// the reference will be cleared. + pub fn set_git_head_target(&mut self, target: Option) { + self.data.git_head = target; } pub fn set_view(&mut self, data: op_store::View) { diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 176f3ada6..08d44d159 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -649,7 +649,7 @@ fn test_resolve_symbol_git_head() { "###); // With HEAD@git - mut_repo.set_git_head(RefTarget::Normal(commit1.id().clone())); + mut_repo.set_git_head_target(Some(RefTarget::Normal(commit1.id().clone()))); insta::assert_debug_snapshot!( resolve_symbol(mut_repo, "HEAD", None).unwrap_err(), @r###" NoSuchRevision { @@ -1677,7 +1677,7 @@ fn test_evaluate_expression_git_head(use_git: bool) { // Can get git head when it's not set assert_eq!(resolve_commit_ids(mut_repo, "git_head()"), vec![]); - mut_repo.set_git_head(RefTarget::Normal(commit1.id().clone())); + mut_repo.set_git_head_target(Some(RefTarget::Normal(commit1.id().clone()))); assert_eq!( resolve_commit_ids(mut_repo, "git_head()"), vec![commit1.id().clone()] diff --git a/lib/tests/test_view.rs b/lib/tests/test_view.rs index f2a185064..b7aa17b83 100644 --- a/lib/tests/test_view.rs +++ b/lib/tests/test_view.rs @@ -442,19 +442,19 @@ fn test_merge_views_git_heads() { let mut tx0 = repo.start_transaction(&settings, "test"); let tx0_head = write_random_commit(tx0.mut_repo(), &settings); tx0.mut_repo() - .set_git_head(RefTarget::Normal(tx0_head.id().clone())); + .set_git_head_target(Some(RefTarget::Normal(tx0_head.id().clone()))); let repo = tx0.commit(); let mut tx1 = repo.start_transaction(&settings, "test"); let tx1_head = write_random_commit(tx1.mut_repo(), &settings); tx1.mut_repo() - .set_git_head(RefTarget::Normal(tx1_head.id().clone())); + .set_git_head_target(Some(RefTarget::Normal(tx1_head.id().clone()))); tx1.commit(); let mut tx2 = repo.start_transaction(&settings, "test"); let tx2_head = write_random_commit(tx2.mut_repo(), &settings); tx2.mut_repo() - .set_git_head(RefTarget::Normal(tx2_head.id().clone())); + .set_git_head_target(Some(RefTarget::Normal(tx2_head.id().clone()))); tx2.commit(); let repo = repo.reload_at_head(&settings).unwrap(); diff --git a/src/cli_util.rs b/src/cli_util.rs index 0e862dd7e..00993f429 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -796,7 +796,7 @@ impl WorkspaceCommandHelper { let new_git_commit_id = Oid::from_bytes(first_parent_id.as_bytes()).unwrap(); let new_git_commit = git_repo.find_commit(new_git_commit_id)?; git_repo.reset(new_git_commit.as_object(), git2::ResetType::Mixed, None)?; - mut_repo.set_git_head(RefTarget::Normal(first_parent_id)); + mut_repo.set_git_head_target(Some(RefTarget::Normal(first_parent_id))); } } else { // The workspace was removed (maybe the user undid the