From 30fb7995c22b3ea315c57449bf9d99e8de5eba99 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 14 Oct 2023 23:54:35 +0900 Subject: [PATCH] view: make local/remote branches iterator yield RemoteRef instead of RefTarget We'll use remote_ref.tracking_target() to classify push action, but not all callers of local_remote_branches() need tracking_target() instead of target. --- cli/src/commands/git.rs | 10 +++------- lib/src/git.rs | 2 +- lib/src/refs.rs | 31 +++++++++++++++++++++---------- lib/src/view.rs | 31 ++++++++++++++++++++----------- 4 files changed, 45 insertions(+), 29 deletions(-) diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index d74fbc0cf..76a059ef1 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -732,9 +732,9 @@ 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).target, + remote_ref: repo.view().get_remote_branch(branch_name, &remote), }; - if targets.local_target.is_absent() && targets.remote_target.is_absent() { + if targets.local_target.is_absent() && targets.remote_ref.is_absent() { return Err(user_error(format!("Branch {branch_name} doesn't exist"))); } match classify_branch_update(branch_name, &remote, targets) { @@ -786,11 +786,7 @@ 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) - .target, + remote_ref: tx.repo().view().get_remote_branch(&branch_name, &remote), }; match classify_branch_update(&branch_name, &remote, targets) { Ok(Some(update)) => branch_updates.push((branch_name.clone(), update)), diff --git a/lib/src/git.rs b/lib/src/git.rs index dd996070b..634b7ce52 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -604,7 +604,7 @@ fn copy_exportable_local_branches_to_remote_view( .filter_map(|(branch, targets)| { // TODO: filter out untracked branches (if we add support for untracked @git // branches) - let old_target = targets.remote_target; + let old_target = &targets.remote_ref.target; let new_target = targets.local_target; (!new_target.has_conflict() && old_target != new_target).then_some((branch, new_target)) }) diff --git a/lib/src/refs.rs b/lib/src/refs.rs index eb21e881a..fb2296ec7 100644 --- a/lib/src/refs.rs +++ b/lib/src/refs.rs @@ -19,7 +19,7 @@ use itertools::EitherOrBoth; use crate::backend::CommitId; use crate::index::Index; use crate::merge::{trivial_merge, Merge}; -use crate::op_store::RefTarget; +use crate::op_store::{RefTarget, RemoteRef}; /// Compares `refs1` and `refs2` targets, yields entry if they differ. /// @@ -34,7 +34,7 @@ pub fn diff_named_refs<'a, 'b, K: Ord>( /// Iterates `refs1` and `refs2` target pairs by name. /// /// `refs1` and `refs2` must be sorted by `K`. -pub fn iter_named_ref_pairs<'a, 'b, K: Ord>( +fn iter_named_ref_pairs<'a, 'b, K: Ord>( refs1: impl IntoIterator, refs2: impl IntoIterator, ) -> impl Iterator { @@ -117,7 +117,7 @@ fn find_pair_to_remove( #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct TrackingRefPair<'a> { pub local_target: &'a RefTarget, - pub remote_target: &'a RefTarget, + pub remote_ref: &'a RemoteRef, } #[derive(Debug, PartialEq, Eq, Clone, Hash)] @@ -138,7 +138,7 @@ pub enum BranchPushAction { /// this branch. pub fn classify_branch_push_action(targets: TrackingRefPair) -> BranchPushAction { let local_target = targets.local_target; - let remote_target = targets.remote_target; + let remote_target = &targets.remote_ref.target; if local_target == remote_target { BranchPushAction::AlreadyMatches } else if local_target.has_conflict() { @@ -157,13 +157,21 @@ pub fn classify_branch_push_action(targets: TrackingRefPair) -> BranchPushAction mod tests { use super::*; use crate::backend::ObjectId; + use crate::op_store::RemoteRefState; + + fn tracking_remote_ref(target: RefTarget) -> RemoteRef { + RemoteRef { + target, + state: RemoteRefState::Tracking, + } + } #[test] fn test_classify_branch_push_action_unchanged() { let commit_id1 = CommitId::from_hex("11"); let targets = TrackingRefPair { local_target: &RefTarget::normal(commit_id1.clone()), - remote_target: &RefTarget::normal(commit_id1), + remote_ref: &tracking_remote_ref(RefTarget::normal(commit_id1)), }; assert_eq!( classify_branch_push_action(targets), @@ -176,7 +184,7 @@ mod tests { let commit_id1 = CommitId::from_hex("11"); let targets = TrackingRefPair { local_target: &RefTarget::normal(commit_id1.clone()), - remote_target: RefTarget::absent_ref(), + remote_ref: RemoteRef::absent_ref(), }; assert_eq!( classify_branch_push_action(targets), @@ -192,7 +200,7 @@ mod tests { let commit_id1 = CommitId::from_hex("11"); let targets = TrackingRefPair { local_target: RefTarget::absent_ref(), - remote_target: &RefTarget::normal(commit_id1.clone()), + remote_ref: &tracking_remote_ref(RefTarget::normal(commit_id1.clone())), }; assert_eq!( classify_branch_push_action(targets), @@ -209,7 +217,7 @@ mod tests { let commit_id2 = CommitId::from_hex("22"); let targets = TrackingRefPair { local_target: &RefTarget::normal(commit_id2.clone()), - remote_target: &RefTarget::normal(commit_id1.clone()), + remote_ref: &tracking_remote_ref(RefTarget::normal(commit_id1.clone())), }; assert_eq!( classify_branch_push_action(targets), @@ -226,7 +234,7 @@ mod tests { let commit_id2 = CommitId::from_hex("22"); let targets = TrackingRefPair { local_target: &RefTarget::from_legacy_form([], [commit_id1.clone(), commit_id2]), - remote_target: &RefTarget::normal(commit_id1), + remote_ref: &tracking_remote_ref(RefTarget::normal(commit_id1)), }; assert_eq!( classify_branch_push_action(targets), @@ -240,7 +248,10 @@ mod tests { let commit_id2 = CommitId::from_hex("22"); let targets = TrackingRefPair { local_target: &RefTarget::normal(commit_id1.clone()), - remote_target: &RefTarget::from_legacy_form([], [commit_id1, commit_id2]), + remote_ref: &tracking_remote_ref(RefTarget::from_legacy_form( + [], + [commit_id1, commit_id2], + )), }; assert_eq!( classify_branch_push_action(targets), diff --git a/lib/src/view.rs b/lib/src/view.rs index 859da9332..1d67d3d6c 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -17,7 +17,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt; -use itertools::Itertools; +use itertools::{EitherOrBoth, Itertools}; use crate::backend::CommitId; use crate::index::Index; @@ -25,7 +25,7 @@ use crate::op_store; use crate::op_store::{ BranchTarget, RefTarget, RefTargetOptionExt as _, RemoteRef, RemoteRefState, WorkspaceId, }; -use crate::refs::{iter_named_ref_pairs, merge_ref_targets, TrackingRefPair}; +use crate::refs::{merge_ref_targets, TrackingRefPair}; #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Hash, Debug)] pub enum RefName { @@ -263,23 +263,32 @@ impl View { } } - /// Iterates local/remote branch `(name, targets)`s of the specified remote - /// in lexicographical order. + /// Iterates local/remote branch `(name, remote_ref)`s of the specified + /// remote in lexicographical order. pub fn local_remote_branches<'a>( &'a self, remote_name: &str, ) -> impl Iterator)> + 'a { - // TODO: maybe untracked remote target can be translated to absent, and rename - // the method accordingly. - iter_named_ref_pairs( + itertools::merge_join_by( self.local_branches(), - self.remote_branches(remote_name) - .map(|(name, remote_ref)| (name, &remote_ref.target)), + self.remote_branches(remote_name), + |(name1, _), (name2, _)| name1.cmp(name2), ) - .map(|(name, (local_target, remote_target))| { + .map(|entry| match entry { + EitherOrBoth::Both((name, local_target), (_, remote_ref)) => { + (name, (local_target, remote_ref)) + } + EitherOrBoth::Left((name, local_target)) => { + (name, (local_target, RemoteRef::absent_ref())) + } + EitherOrBoth::Right((name, remote_ref)) => { + (name, (RefTarget::absent_ref(), remote_ref)) + } + }) + .map(|(name, (local_target, remote_ref))| { let targets = TrackingRefPair { local_target, - remote_target, + remote_ref, }; (name, targets) })