view: make get_remote_branch() return RemoteRef instead of RefTarget

I'm going to add tracking state to RemoteRef, and we should compare both target and state in the tests.
jj-vcs · Oct 15, 2023 · 35ea607 · 35ea607
1 parent d730b25
commit 35ea607
Show file tree

Hide file tree

Showing 7 changed files with 143 additions and 63 deletions.
diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs
@@ -491,11 +491,11 @@ fn cmd_git_clone(
 
     let (mut workspace_command, git_repo, stats) = clone_result?;
     if let Some(default_branch) = &stats.default_branch {
-        let default_branch_target = workspace_command
+        let default_branch_remote_ref = workspace_command
             .repo()
             .view()
             .get_remote_branch(default_branch, "origin");
-        if let Some(commit_id) = default_branch_target.as_normal().cloned() {
+        if let Some(commit_id) = default_branch_remote_ref.target.as_normal().cloned() {
             let mut checkout_tx =
                 workspace_command.start_transaction("check out git remote's default branch");
             if args.colocate {
@@ -730,7 +730,7 @@ fn cmd_git_push(
             }
             let targets = TrackingRefPair {
                 local_target: repo.view().get_local_branch(branch_name),
-                remote_target: repo.view().get_remote_branch(branch_name, &remote),
+                remote_target: &repo.view().get_remote_branch(branch_name, &remote).target,
             };
             if targets.local_target.is_absent() && targets.remote_target.is_absent() {
                 return Err(user_error(format!("Branch {branch_name} doesn't exist")));
@@ -784,7 +784,11 @@ fn cmd_git_push(
                 .set_local_branch_target(&branch_name, RefTarget::normal(commit.id().clone()));
             let targets = TrackingRefPair {
                 local_target: tx.repo().view().get_local_branch(&branch_name),
-                remote_target: tx.repo().view().get_remote_branch(&branch_name, &remote),
+                remote_target: &tx
+                    .repo()
+                    .view()
+                    .get_remote_branch(&branch_name, &remote)
+                    .target,
             };
             match classify_branch_update(&branch_name, &remote, targets) {
                 Ok(Some(update)) => branch_updates.push((branch_name.clone(), update)),

diff --git a/lib/src/repo.rs b/lib/src/repo.rs
@@ -42,7 +42,7 @@ use crate::git_backend::GitBackend;
 use crate::index::{HexPrefix, Index, IndexStore, MutableIndex, PrefixResolution, ReadonlyIndex};
 use crate::local_backend::LocalBackend;
 use crate::op_heads_store::{self, OpHeadResolutionError, OpHeadsStore};
-use crate::op_store::{OpStore, OpStoreError, OperationId, RefTarget, WorkspaceId};
+use crate::op_store::{OpStore, OpStoreError, OperationId, RefTarget, RemoteRef, WorkspaceId};
 use crate::operation::Operation;
 use crate::refs::{diff_named_refs, merge_ref_targets};
 use crate::revset::{self, ChangeIdIndex, Revset, RevsetExpression};
@@ -998,7 +998,7 @@ impl MutableRepo {
         self.view_mut().set_local_branch_target(name, target);
     }
 
-    pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> RefTarget {
+    pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> RemoteRef {
         self.view
             .with_ref(|v| v.get_remote_branch(name, remote_name).clone())
     }

diff --git a/lib/src/revset.rs b/lib/src/revset.rs
@@ -2018,7 +2018,7 @@ fn resolve_remote_branch(repo: &dyn Repo, name: &str, remote: &str) -> Option<Ve
     let view = repo.view();
     let target = match (name, remote) {
         ("HEAD", git::REMOTE_NAME_FOR_LOCAL_GIT_REPO) => view.git_head(),
-        (name, remote) => view.get_remote_branch(name, remote),
+        (name, remote) => &view.get_remote_branch(name, remote).target,
     };
     target
         .is_present()

diff --git a/lib/src/view.rs b/lib/src/view.rs
@@ -130,7 +130,9 @@ impl View {
     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),
+            RefName::RemoteBranch { branch, remote } => {
+                &self.get_remote_branch(branch, remote).target
+            }
             RefName::Tag(name) => self.get_tag(name),
             RefName::GitRef(name) => self.get_git_ref(name),
         }
@@ -211,16 +213,11 @@ impl View {
             .flatten()
     }
 
-    pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> &RefTarget {
-        // TODO: maybe return RemoteRef instead of RefTarget?
+    pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> &RemoteRef {
         if let Some(remote_view) = self.data.remote_views.get(remote_name) {
-            let maybe_target = remote_view
-                .branches
-                .get(name)
-                .map(|remote_ref| &remote_ref.target);
-            maybe_target.flatten()
+            remote_view.branches.get(name).flatten()
         } else {
-            RefTarget::absent_ref()
+            RemoteRef::absent_ref()
         }
     }