view: return RefTarget by reference, clone() by caller

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,
             );
         }
 

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);
             }
         }

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())
     );
 }
 

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)?;