diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 620878232..a393fc58d 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -958,14 +958,10 @@ impl WorkspaceCommandHelper { pub fn check_rewritable<'a>( &self, - commits: impl IntoIterator, + commits: impl IntoIterator, ) -> Result<(), CommandError> { - let to_rewrite_revset = RevsetExpression::commits( - commits - .into_iter() - .map(|commit| commit.id().clone()) - .collect(), - ); + let to_rewrite_revset = + RevsetExpression::commits(commits.into_iter().cloned().collect_vec()); let immutable = revset_util::parse_immutable_expression(&self.revset_parse_context()) .map_err(|e| { config_error_with_message("Invalid `revset-aliases.immutable_heads()`", e) diff --git a/cli/src/commands/abandon.rs b/cli/src/commands/abandon.rs index e46a7898e..48fcbbcbb 100644 --- a/cli/src/commands/abandon.rs +++ b/cli/src/commands/abandon.rs @@ -15,6 +15,7 @@ use std::io::Write; use itertools::Itertools as _; +use jj_lib::commit::CommitIteratorExt; use jj_lib::object_id::ObjectId; use tracing::instrument; @@ -58,7 +59,7 @@ pub(crate) fn cmd_abandon( writeln!(ui.status(), "No revisions to abandon.")?; return Ok(()); } - workspace_command.check_rewritable(&to_abandon)?; + workspace_command.check_rewritable(to_abandon.iter().ids())?; let mut tx = workspace_command.start_transaction(); for commit in &to_abandon { diff --git a/cli/src/commands/chmod.rs b/cli/src/commands/chmod.rs index dfcd0ab1f..6f2d4af7d 100644 --- a/cli/src/commands/chmod.rs +++ b/cli/src/commands/chmod.rs @@ -66,7 +66,7 @@ pub(crate) fn cmd_chmod( .map(|path| workspace_command.parse_file_path(path)) .try_collect()?; let commit = workspace_command.resolve_single_rev(&args.revision)?; - workspace_command.check_rewritable([&commit])?; + workspace_command.check_rewritable([commit.id()])?; let mut tx = workspace_command.start_transaction(); let tree = commit.tree()?; diff --git a/cli/src/commands/describe.rs b/cli/src/commands/describe.rs index b746a9282..a944eee50 100644 --- a/cli/src/commands/describe.rs +++ b/cli/src/commands/describe.rs @@ -68,7 +68,7 @@ pub(crate) fn cmd_describe( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let commit = workspace_command.resolve_single_rev(&args.revision)?; - workspace_command.check_rewritable([&commit])?; + workspace_command.check_rewritable([commit.id()])?; let description = if args.stdin { let mut buffer = String::new(); io::stdin().read_to_string(&mut buffer).unwrap(); diff --git a/cli/src/commands/diffedit.rs b/cli/src/commands/diffedit.rs index 7d1c307d0..ad0eeaf01 100644 --- a/cli/src/commands/diffedit.rs +++ b/cli/src/commands/diffedit.rs @@ -81,7 +81,7 @@ pub(crate) fn cmd_diffedit( base_commits = target_commit.parents(); diff_description = "The diff initially shows the commit's changes.".to_string(); }; - workspace_command.check_rewritable([&target_commit])?; + workspace_command.check_rewritable([target_commit.id()])?; let diff_editor = workspace_command.diff_editor(ui, args.tool.as_deref())?; let mut tx = workspace_command.start_transaction(); diff --git a/cli/src/commands/edit.rs b/cli/src/commands/edit.rs index a245bdafc..a209ce610 100644 --- a/cli/src/commands/edit.rs +++ b/cli/src/commands/edit.rs @@ -44,7 +44,7 @@ pub(crate) fn cmd_edit( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let new_commit = workspace_command.resolve_single_rev(&args.revision)?; - workspace_command.check_rewritable([&new_commit])?; + workspace_command.check_rewritable([new_commit.id()])?; if workspace_command.get_wc_commit_id() == Some(new_commit.id()) { writeln!(ui.status(), "Already editing that commit")?; } else { diff --git a/cli/src/commands/new.rs b/cli/src/commands/new.rs index eeeb50aee..592c4081d 100644 --- a/cli/src/commands/new.rs +++ b/cli/src/commands/new.rs @@ -16,7 +16,7 @@ use std::io::Write; use clap::ArgGroup; use itertools::Itertools; -use jj_lib::commit::Commit; +use jj_lib::commit::{Commit, CommitIteratorExt}; use jj_lib::repo::Repo; use jj_lib::revset::{RevsetExpression, RevsetIteratorExt}; use jj_lib::rewrite::{merge_commit_trees, rebase_commit}; @@ -107,8 +107,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", // command line, add it between the changes' parents and the changes. // The parents of the new commit will be the parents of the target commits // which are not descendants of other target commits. - tx.base_workspace_helper() - .check_rewritable(&target_commits)?; + tx.base_workspace_helper().check_rewritable(&target_ids)?; let new_children = RevsetExpression::commits(target_ids.clone()); let new_parents = new_children.parents(); if let Some(commit_id) = new_children @@ -160,7 +159,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", vec![] }; tx.base_workspace_helper() - .check_rewritable(&commits_to_rebase)?; + .check_rewritable(commits_to_rebase.iter().ids())?; let merged_tree = merge_commit_trees(tx.repo(), &target_commits)?; new_commit = tx .mut_repo() diff --git a/cli/src/commands/next.rs b/cli/src/commands/next.rs index 0bddac84a..b47bd433e 100644 --- a/cli/src/commands/next.rs +++ b/cli/src/commands/next.rs @@ -150,7 +150,7 @@ pub(crate) fn cmd_next( // We're editing, just move to the target commit. if edit { // We're editing, the target must be rewritable. - workspace_command.check_rewritable([target])?; + workspace_command.check_rewritable([target.id()])?; let mut tx = workspace_command.start_transaction(); tx.edit(target)?; tx.finish( diff --git a/cli/src/commands/prev.rs b/cli/src/commands/prev.rs index 233701873..f170008f2 100644 --- a/cli/src/commands/prev.rs +++ b/cli/src/commands/prev.rs @@ -118,7 +118,7 @@ pub(crate) fn cmd_prev( // If we're editing, just move to the revision directly. if edit { // The target must be rewritable if we're editing. - workspace_command.check_rewritable([target])?; + workspace_command.check_rewritable([target.id()])?; let mut tx = workspace_command.start_transaction(); tx.edit(target)?; tx.finish( diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs index e8de4bfe9..5a16be3cf 100644 --- a/cli/src/commands/rebase.rs +++ b/cli/src/commands/rebase.rs @@ -21,7 +21,7 @@ use clap::ArgGroup; use indexmap::IndexSet; use itertools::Itertools; use jj_lib::backend::CommitId; -use jj_lib::commit::Commit; +use jj_lib::commit::{Commit, CommitIteratorExt}; use jj_lib::object_id::ObjectId; use jj_lib::repo::{ReadonlyRepo, Repo}; use jj_lib::revset::{RevsetExpression, RevsetIteratorExt}; @@ -314,7 +314,7 @@ fn rebase_descendants_transaction( old_commits: &IndexSet, rebase_options: RebaseOptions, ) -> Result<(), CommandError> { - workspace_command.check_rewritable(old_commits)?; + workspace_command.check_rewritable(old_commits.iter().ids())?; let (skipped_commits, old_commits) = old_commits .iter() .partition::, _>(|commit| commit.parents() == new_parents); @@ -353,7 +353,7 @@ fn rebase_revision( rev_arg: &RevisionArg, ) -> Result<(), CommandError> { let old_commit = workspace_command.resolve_single_rev(rev_arg)?; - workspace_command.check_rewritable([&old_commit])?; + workspace_command.check_rewritable([old_commit.id()])?; if new_parents.contains(&old_commit) { return Err(user_error(format!( "Cannot rebase {} onto itself", @@ -370,7 +370,9 @@ fn rebase_revision( .try_collect()?; // Currently, immutable commits are defined so that a child of a rewriteable // commit is always rewriteable. - debug_assert!(workspace_command.check_rewritable(&child_commits).is_ok()); + debug_assert!(workspace_command + .check_rewritable(child_commits.iter().ids()) + .is_ok()); // First, rebase the children of `old_commit`. let mut tx = workspace_command.start_transaction(); diff --git a/cli/src/commands/resolve.rs b/cli/src/commands/resolve.rs index ada794d82..626f9f5c7 100644 --- a/cli/src/commands/resolve.rs +++ b/cli/src/commands/resolve.rs @@ -87,7 +87,7 @@ pub(crate) fn cmd_resolve( }; let (repo_path, _) = conflicts.first().unwrap(); - workspace_command.check_rewritable([&commit])?; + workspace_command.check_rewritable([commit.id()])?; let merge_editor = workspace_command.merge_editor(ui, args.tool.as_deref())?; writeln!( ui.status(), diff --git a/cli/src/commands/restore.rs b/cli/src/commands/restore.rs index 03fc4820e..49def89df 100644 --- a/cli/src/commands/restore.rs +++ b/cli/src/commands/restore.rs @@ -96,7 +96,7 @@ pub(crate) fn cmd_restore( .resolve_single_rev(args.changes_in.as_ref().unwrap_or(&RevisionArg::AT))?; from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &to_commit.parents())?; } - workspace_command.check_rewritable([&to_commit])?; + workspace_command.check_rewritable([to_commit.id()])?; let matcher = workspace_command.matcher_from_values(&args.paths)?; let to_tree = to_commit.tree()?; diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index a87119623..7521af558 100644 --- a/cli/src/commands/split.rs +++ b/cli/src/commands/split.rs @@ -67,7 +67,7 @@ pub(crate) fn cmd_split( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let commit = workspace_command.resolve_single_rev(&args.revision)?; - workspace_command.check_rewritable([&commit])?; + workspace_command.check_rewritable([commit.id()])?; let matcher = workspace_command.matcher_from_values(&args.paths)?; let diff_selector = workspace_command.diff_selector( ui, diff --git a/cli/src/commands/squash.rs b/cli/src/commands/squash.rs index 995fcb5db..0141b75dc 100644 --- a/cli/src/commands/squash.rs +++ b/cli/src/commands/squash.rs @@ -13,7 +13,7 @@ // limitations under the License. use itertools::Itertools as _; -use jj_lib::commit::Commit; +use jj_lib::commit::{Commit, CommitIteratorExt}; use jj_lib::matchers::Matcher; use jj_lib::object_id::ObjectId; use jj_lib::repo::Repo; @@ -148,7 +148,7 @@ pub fn move_diff( path_arg: &[String], ) -> Result<(), CommandError> { tx.base_workspace_helper() - .check_rewritable(sources.iter().chain(std::iter::once(destination)))?; + .check_rewritable(sources.iter().chain(std::iter::once(destination)).ids())?; // Tree diffs to apply to the destination let mut tree_diffs = vec![]; let mut abandoned_commits = vec![]; diff --git a/cli/src/commands/unsquash.rs b/cli/src/commands/unsquash.rs index ca120d9f2..389512318 100644 --- a/cli/src/commands/unsquash.rs +++ b/cli/src/commands/unsquash.rs @@ -57,13 +57,12 @@ pub(crate) fn cmd_unsquash( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let commit = workspace_command.resolve_single_rev(&args.revision)?; - workspace_command.check_rewritable([&commit])?; - let parents = commit.parents(); - if parents.len() != 1 { + workspace_command.check_rewritable([commit.id()])?; + if commit.parent_ids().len() > 1 { return Err(user_error("Cannot unsquash merge commits")); } - let parent = &parents[0]; - workspace_command.check_rewritable(&parents[..1])?; + let parent = commit.parents().pop().unwrap(); + workspace_command.check_rewritable([parent.id()])?; let interactive_editor = if args.tool.is_some() || args.interactive { Some(workspace_command.diff_editor(ui, args.tool.as_deref())?) } else { @@ -85,7 +84,7 @@ the parent commit. The changes you edited out will be moved into the child commit. If you don't make any changes, then the operation will be aborted. ", - tx.format_commit_summary(parent), + tx.format_commit_summary(&parent), tx.format_commit_summary(&commit) ); let parent_tree = parent.tree()?; @@ -105,7 +104,8 @@ aborted. // case). if new_parent_tree_id == parent_base_tree.id() { tx.mut_repo().record_abandoned_commit(parent.id().clone()); - let description = combine_messages(tx.base_repo(), &[parent], &commit, command.settings())?; + let description = + combine_messages(tx.base_repo(), &[&parent], &commit, command.settings())?; // Commit the new child on top of the parent's parents. tx.mut_repo() .rewrite_commit(command.settings(), &commit) @@ -115,7 +115,7 @@ aborted. } else { let new_parent = tx .mut_repo() - .rewrite_commit(command.settings(), parent) + .rewrite_commit(command.settings(), &parent) .set_tree_id(new_parent_tree_id) .set_predecessors(vec![parent.id().clone(), commit.id().clone()]) .write()?; diff --git a/lib/src/commit.rs b/lib/src/commit.rs index 65e2e1379..7a82aefa5 100644 --- a/lib/src/commit.rs +++ b/lib/src/commit.rs @@ -163,6 +163,19 @@ impl Commit { } } +pub trait CommitIteratorExt<'c, I> { + fn ids(self) -> impl Iterator + 'c; +} + +impl<'c, I> CommitIteratorExt<'c, I> for I +where + I: Iterator + 'c, +{ + fn ids(self) -> impl Iterator + 'c { + Box::new(self.map(|commit| commit.id())) + } +} + /// Wrapper to sort `Commit` by committer timestamp. #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub(crate) struct CommitByCommitterTimestamp(pub Commit);