From ecb0850f1aee4300051ef7f014f014d0b5378fe2 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 18 Jul 2023 19:59:37 +0900 Subject: [PATCH] view: return RefTarget by reference, clone() by caller --- lib/src/repo.rs | 12 ++++++------ lib/src/view.rs | 26 +++++++++++++------------- lib/tests/test_git.rs | 23 +++++++++++++---------- src/commands/git.rs | 4 ++-- 4 files changed, 34 insertions(+), 31 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 376d1df4a..9df2ae37a 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -966,7 +966,7 @@ impl MutableRepo { } pub fn get_local_branch(&self, name: &str) -> RefTarget { - self.view.with_ref(|v| v.get_local_branch(name)) + self.view.with_ref(|v| v.get_local_branch(name).clone()) } pub fn set_local_branch_target(&mut self, name: &str, target: RefTarget) { @@ -975,7 +975,7 @@ impl MutableRepo { pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> RefTarget { self.view - .with_ref(|v| v.get_remote_branch(name, remote_name)) + .with_ref(|v| v.get_remote_branch(name, remote_name).clone()) } pub fn set_remote_branch_target(&mut self, name: &str, remote_name: &str, target: RefTarget) { @@ -988,7 +988,7 @@ impl MutableRepo { } pub fn get_tag(&self, name: &str) -> RefTarget { - self.view.with_ref(|v| v.get_tag(name)) + self.view.with_ref(|v| v.get_tag(name).clone()) } pub fn set_tag_target(&mut self, name: &str, target: RefTarget) { @@ -996,7 +996,7 @@ impl MutableRepo { } pub fn get_git_ref(&self, name: &str) -> RefTarget { - self.view.with_ref(|v| v.get_git_ref(name)) + self.view.with_ref(|v| v.get_git_ref(name).clone()) } pub fn set_git_ref_target(&mut self, name: &str, target: RefTarget) { @@ -1127,8 +1127,8 @@ impl MutableRepo { self.view.get_mut().merge_single_ref( self.index.as_index(), &ref_name, - &base_target, - &other_target, + base_target, + other_target, ); } diff --git a/lib/src/view.rs b/lib/src/view.rs index f0e8bdbd1..863888655 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -126,7 +126,7 @@ impl View { self.data.public_head_ids.remove(head_id); } - pub fn get_ref(&self, name: &RefName) -> RefTarget { + pub fn get_ref(&self, name: &RefName) -> &RefTarget { match &name { RefName::LocalBranch(name) => self.get_local_branch(name), RefName::RemoteBranch { branch, remote } => self.get_remote_branch(branch, remote), @@ -160,11 +160,11 @@ impl View { self.data.branches.remove(name); } - pub fn get_local_branch(&self, name: &str) -> RefTarget { + pub fn get_local_branch(&self, name: &str) -> &RefTarget { if let Some(branch_target) = self.data.branches.get(name) { - branch_target.local_target.clone() + &branch_target.local_target } else { - RefTarget::absent() + RefTarget::absent_ref() } } @@ -192,12 +192,12 @@ impl View { } } - pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> RefTarget { + pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> &RefTarget { if let Some(branch_target) = self.data.branches.get(name) { let maybe_target = branch_target.remote_targets.get(remote_name); - maybe_target.flatten().clone() + maybe_target.flatten() } else { - RefTarget::absent() + RefTarget::absent_ref() } } @@ -239,8 +239,8 @@ impl View { } } - pub fn get_tag(&self, name: &str) -> RefTarget { - self.data.tags.get(name).flatten().clone() + pub fn get_tag(&self, name: &str) -> &RefTarget { + self.data.tags.get(name).flatten() } /// Sets tag to point to the given target. If the target is absent, the tag @@ -253,8 +253,8 @@ impl View { } } - pub fn get_git_ref(&self, name: &str) -> RefTarget { - self.data.git_refs.get(name).flatten().clone() + pub fn get_git_ref(&self, name: &str) -> &RefTarget { + self.data.git_refs.get(name).flatten() } /// Sets the last imported Git ref to point to the given target. If the @@ -294,8 +294,8 @@ impl View { ) { if base_target != other_target { let self_target = self.get_ref(ref_name); - let new_target = merge_ref_targets(index, &self_target, base_target, other_target); - if new_target != self_target { + let new_target = merge_ref_targets(index, self_target, base_target, other_target); + if new_target != *self_target { self.set_ref_target(ref_name, new_target); } } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 1ac1643dc..aaef4f965 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -153,27 +153,27 @@ fn test_import_refs() { assert_eq!(view.git_refs().len(), 6); assert_eq!( view.get_git_ref("refs/heads/main"), - RefTarget::normal(jj_id(&commit2)) + &RefTarget::normal(jj_id(&commit2)) ); assert_eq!( view.get_git_ref("refs/heads/feature1"), - RefTarget::normal(jj_id(&commit3)) + &RefTarget::normal(jj_id(&commit3)) ); assert_eq!( view.get_git_ref("refs/heads/feature2"), - RefTarget::normal(jj_id(&commit4)) + &RefTarget::normal(jj_id(&commit4)) ); assert_eq!( view.get_git_ref("refs/remotes/origin/main"), - RefTarget::normal(jj_id(&commit1)) + &RefTarget::normal(jj_id(&commit1)) ); assert_eq!( view.get_git_ref("refs/remotes/origin/feature3"), - RefTarget::normal(jj_id(&commit6)) + &RefTarget::normal(jj_id(&commit6)) ); assert_eq!( view.get_git_ref("refs/tags/v1.0"), - RefTarget::normal(jj_id(&commit5)) + &RefTarget::normal(jj_id(&commit5)) ); assert_eq!(view.git_head(), &RefTarget::normal(jj_id(&commit2))); } @@ -263,10 +263,13 @@ fn test_import_refs_reimport() { assert!(view.tags().is_empty()); assert_eq!(view.git_refs().len(), 3); - assert_eq!(view.get_git_ref("refs/heads/main"), commit2_target); - assert_eq!(view.get_git_ref("refs/remotes/origin/main"), commit1_target); + assert_eq!(view.get_git_ref("refs/heads/main"), &commit2_target); + assert_eq!( + view.get_git_ref("refs/remotes/origin/main"), + &commit1_target + ); let commit5_target = RefTarget::normal(jj_id(&commit5)); - assert_eq!(view.get_git_ref("refs/heads/feature2"), commit5_target); + assert_eq!(view.get_git_ref("refs/heads/feature2"), &commit5_target); } #[test] @@ -1125,7 +1128,7 @@ fn test_export_import_sequence() { ); assert_eq!( mut_repo.view().get_local_branch("main"), - RefTarget::normal(commit_c.id().clone()) + &RefTarget::normal(commit_c.id().clone()) ); } diff --git a/src/commands/git.rs b/src/commands/git.rs index 25393992b..70ed2bda3 100644 --- a/src/commands/git.rs +++ b/src/commands/git.rs @@ -437,10 +437,10 @@ fn cmd_git_clone( .repo() .view() .get_remote_branch(&default_branch, "origin"); - if let Some(commit_id) = default_branch_target.as_normal() { + if let Some(commit_id) = default_branch_target.as_normal().cloned() { let mut checkout_tx = workspace_command.start_transaction("check out git remote's default branch"); - if let Ok(commit) = checkout_tx.repo().store().get_commit(commit_id) { + if let Ok(commit) = checkout_tx.repo().store().get_commit(&commit_id) { checkout_tx.check_out(&commit)?; } checkout_tx.finish(ui)?;