From 28592779417037527771dcbdd5a885f91ab545dc Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 15 Apr 2024 21:22:00 -0700 Subject: [PATCH] rewrite: pass `CommitRewriter` into `rebase_commit_with_options()` `CommitRewriter` wraps 3 of the arguments, so I think it makes sense to pass it instead. More importantly, I hope to continue refactoring so many of the callers already have a `CommitRewriter`. --- cli/src/commands/rebase.rs | 11 ++++++----- lib/src/rewrite.rs | 25 ++++++------------------- lib/tests/test_rewrite.rs | 14 +++++--------- 3 files changed, 17 insertions(+), 33 deletions(-) diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs index e05c79e04..9ce3cb052 100644 --- a/cli/src/commands/rebase.rs +++ b/cli/src/commands/rebase.rs @@ -25,7 +25,9 @@ use jj_lib::commit::{Commit, CommitIteratorExt}; use jj_lib::object_id::ObjectId; use jj_lib::repo::{ReadonlyRepo, Repo}; use jj_lib::revset::{RevsetExpression, RevsetIteratorExt}; -use jj_lib::rewrite::{rebase_commit, rebase_commit_with_options, EmptyBehaviour, RebaseOptions}; +use jj_lib::rewrite::{ + rebase_commit, rebase_commit_with_options, CommitRewriter, EmptyBehaviour, RebaseOptions, +}; use jj_lib::settings::UserSettings; use tracing::instrument; @@ -292,16 +294,15 @@ pub fn rebase_descendants( rebase_options: RebaseOptions, ) -> Result { for old_commit in old_commits.iter() { - rebase_commit_with_options( - settings, + let rewriter = CommitRewriter::new( tx.mut_repo(), old_commit.borrow().clone(), new_parents .iter() .map(|parent| parent.id().clone()) .collect(), - &rebase_options, - )?; + ); + rebase_commit_with_options(settings, rewriter, &rebase_options)?; } let num_rebased = old_commits.len() + tx.mut_repo() diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index f9eb4731c..8fa0bb697 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -99,13 +99,8 @@ pub fn rebase_commit( old_commit: Commit, new_parents: Vec, ) -> BackendResult { - let rebased_commit = rebase_commit_with_options( - settings, - mut_repo, - old_commit, - new_parents, - &Default::default(), - )?; + let rewriter = CommitRewriter::new(mut_repo, old_commit, new_parents); + let rebased_commit = rebase_commit_with_options(settings, rewriter, &Default::default())?; match rebased_commit { RebasedCommit::Rewritten(new_commit) => Ok(new_commit), RebasedCommit::Abandoned { parent: _ } => panic!("Commit was unexpectedly abandoned"), @@ -247,13 +242,9 @@ pub enum RebasedCommit { pub fn rebase_commit_with_options( settings: &UserSettings, - mut_repo: &mut MutableRepo, - old_commit: Commit, - new_parents: Vec, + mut rewriter: CommitRewriter<'_>, options: &RebaseOptions, ) -> BackendResult { - let mut rewriter = CommitRewriter::new(mut_repo, old_commit, new_parents); - // If specified, don't create commit where one parent is an ancestor of another. if options.simplify_ancestor_merge { rewriter.simplify_ancestor_merge(); @@ -396,13 +387,9 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { return Ok(()); } - let rebased_commit: RebasedCommit = rebase_commit_with_options( - self.settings, - self.mut_repo, - old_commit, - new_parent_ids, - &self.options, - )?; + let rewriter = CommitRewriter::new(self.mut_repo, old_commit, new_parent_ids); + let rebased_commit: RebasedCommit = + rebase_commit_with_options(self.settings, rewriter, &self.options)?; let new_commit = match rebased_commit { RebasedCommit::Rewritten(new_commit) => new_commit, RebasedCommit::Abandoned { parent } => parent, diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index 9fbced49e..7ff9ed0f9 100644 --- a/lib/tests/test_rewrite.rs +++ b/lib/tests/test_rewrite.rs @@ -19,7 +19,9 @@ use jj_lib::merged_tree::MergedTree; use jj_lib::op_store::{RefTarget, RemoteRef, RemoteRefState, WorkspaceId}; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; -use jj_lib::rewrite::{rebase_commit_with_options, restore_tree, EmptyBehaviour, RebaseOptions}; +use jj_lib::rewrite::{ + rebase_commit_with_options, restore_tree, CommitRewriter, EmptyBehaviour, RebaseOptions, +}; use maplit::{hashmap, hashset}; use test_case::test_case; use testutils::{ @@ -1726,14 +1728,8 @@ fn test_rebase_abandoning_empty() { empty: EmptyBehaviour::AbandonAllEmpty, simplify_ancestor_merge: true, }; - rebase_commit_with_options( - &settings, - tx.mut_repo(), - commit_b, - vec![commit_b2.id().clone()], - &rebase_options, - ) - .unwrap(); + let rewriter = CommitRewriter::new(tx.mut_repo(), commit_b, vec![commit_b2.id().clone()]); + rebase_commit_with_options(&settings, rewriter, &rebase_options).unwrap(); let rebase_map = tx .mut_repo() .rebase_descendants_with_options_return_map(&settings, rebase_options)