rebase: add compute_rebase_destination function

cli/src/commands/rebase.rs

@@ -241,60 +241,16 @@ pub(crate) fn cmd_rebase(
             EmptyBehaviour::Keep,
             "clap should forbid `-r --skip-empty`"
         );
-        let target_commits: Vec<_> = workspace_command
+        rebase_revisions(
-            .parse_union_revsets(ui, &args.revisions)?
+            ui,
-            .evaluate_to_commits()?
+            command.settings(),
-            .try_collect()?; // in reverse topological order
+            &mut workspace_command,
-        if !args.insert_after.is_empty() && !args.insert_before.is_empty() {
+            &args.revisions,
-            let after_commits =
+            &args.destination,
-                workspace_command.resolve_some_revsets_default_single(ui, &args.insert_after)?;
+            &args.insert_after,
-            let before_commits =
+            &args.insert_before,
-                workspace_command.resolve_some_revsets_default_single(ui, &args.insert_before)?;
+            &rebase_options,
-            rebase_revisions_after_before(
+        )?;
-                ui,
-                command.settings(),
-                &mut workspace_command,
-                &after_commits,
-                &before_commits,
-                &target_commits,
-                &rebase_options,
-            )?;
-        } else if !args.insert_after.is_empty() {
-            let after_commits =
-                workspace_command.resolve_some_revsets_default_single(ui, &args.insert_after)?;
-            rebase_revisions_after(
-                ui,
-                command.settings(),
-                &mut workspace_command,
-                &after_commits,
-                &target_commits,
-                &rebase_options,
-            )?;
-        } else if !args.insert_before.is_empty() {
-            let before_commits =
-                workspace_command.resolve_some_revsets_default_single(ui, &args.insert_before)?;
-            rebase_revisions_before(
-                ui,
-                command.settings(),
-                &mut workspace_command,
-                &before_commits,
-                &target_commits,
-                &rebase_options,
-            )?;
-        } else {
-            let new_parents = workspace_command
-                .resolve_some_revsets_default_single(ui, &args.destination)?
-                .into_iter()
-                .collect_vec();
-            rebase_revisions(
-                ui,
-                command.settings(),
-                &mut workspace_command,
-                &new_parents,
-                &target_commits,
-                &rebase_options,
-            )?;
-        }
     } else if !args.source.is_empty() {
         let new_parents = workspace_command
             .resolve_some_revsets_default_single(ui, &args.destination)?
@@ -332,6 +288,52 @@ pub(crate) fn cmd_rebase(
     Ok(())
 }
 
+#[allow(clippy::too_many_arguments)]
+fn rebase_revisions(
+    ui: &mut Ui,
+    settings: &UserSettings,
+    workspace_command: &mut WorkspaceCommandHelper,
+    revisions: &[RevisionArg],
+    destination: &[RevisionArg],
+    insert_after: &[RevisionArg],
+    insert_before: &[RevisionArg],
+    rebase_options: &RebaseOptions,
+) -> Result<(), CommandError> {
+    let target_commits: Vec<_> = workspace_command
+        .parse_union_revsets(ui, revisions)?
+        .evaluate_to_commits()?
+        .try_collect()?; // in reverse topological order
+    workspace_command.check_rewritable(target_commits.iter().ids())?;
+
+    let (new_parents, new_children) = compute_rebase_destination(
+        ui,
+        workspace_command,
+        destination,
+        insert_after,
+        insert_before,
+    )?;
+    if !destination.is_empty() && new_children.is_empty() {
+        for commit in &target_commits {
+            if new_parents.contains(commit) {
+                return Err(user_error(format!(
+                    "Cannot rebase {} onto itself",
+                    short_commit_hash(commit.id()),
+                )));
+            }
+        }
+    }
+    rebase_revisions_transaction(
+        ui,
+        settings,
+        workspace_command,
+        &new_parents.iter().ids().cloned().collect_vec(),
+        &new_children,
+        &target_commits,
+        None,
+        rebase_options,
+    )
+}
+
 fn rebase_branch(
     ui: &mut Ui,
     settings: &UserSettings,
@@ -451,164 +453,74 @@ fn rebase_descendants_transaction(
     tx.finish(ui, tx_description)
 }
 
-fn rebase_revisions(
+/// Computes the new parents and children given the input arguments for
+/// `destination`, `insert_after`, and `insert_before`.
+fn compute_rebase_destination(
     ui: &mut Ui,
-    settings: &UserSettings,
     workspace_command: &mut WorkspaceCommandHelper,
-    new_parents: &[Commit],
+    destination: &[RevisionArg],
-    target_commits: &[Commit],
+    insert_after: &[RevisionArg],
-    rebase_options: &RebaseOptions,
+    insert_before: &[RevisionArg],
-) -> Result<(), CommandError> {
+) -> Result<(Vec<Commit>, Vec<Commit>), CommandError> {
-    if target_commits.is_empty() {
+    let resolve_revisions = |revisions: &[RevisionArg]| -> Result<Vec<Commit>, CommandError> {
-        return Ok(());
+        if revisions.is_empty() {
-    }
+            Ok(vec![])
-
+        } else {
-    workspace_command.check_rewritable(target_commits.iter().ids())?;
+            Ok(workspace_command
-    for commit in target_commits {
+                .resolve_some_revsets_default_single(ui, revisions)?
-        if new_parents.contains(commit) {
+                .into_iter()
-            return Err(user_error(format!(
+                .collect_vec())
-                "Cannot rebase {} onto itself",
-                short_commit_hash(commit.id()),
-            )));
         }
+    };
+    let destination_commits = resolve_revisions(destination)?;
+    let after_commits = resolve_revisions(insert_after)?;
+    let before_commits = resolve_revisions(insert_before)?;
+
+    let (new_parents, new_children) = if !after_commits.is_empty() && !before_commits.is_empty() {
+        (after_commits, before_commits)
+    } else if !after_commits.is_empty() {
+        let new_children: Vec<_> =
+            RevsetExpression::commits(after_commits.iter().ids().cloned().collect_vec())
+                .children()
+                .evaluate_programmatic(workspace_command.repo().as_ref())?
+                .iter()
+                .commits(workspace_command.repo().store())
+                .try_collect()?;
+
+        (after_commits, new_children)
+    } else if !before_commits.is_empty() {
+        // Not using `RevsetExpression::parents` here to persist the order of parents
+        // specified in `before_commits`.
+        let new_parent_ids = before_commits
+            .iter()
+            .flat_map(|commit| commit.parent_ids().iter().cloned().collect_vec())
+            .unique()
+            .collect_vec();
+        let new_parents: Vec<_> = new_parent_ids
+            .iter()
+            .map(|commit_id| workspace_command.repo().store().get_commit(commit_id))
+            .try_collect()?;
+
+        (new_parents, before_commits)
+    } else {
+        (destination_commits, vec![])
+    };
+
+    if !new_children.is_empty() {
+        workspace_command.check_rewritable(new_children.iter().ids())?;
+        ensure_no_commit_loop(
+            workspace_command.repo().as_ref(),
+            &RevsetExpression::commits(new_children.iter().ids().cloned().collect_vec()),
+            &RevsetExpression::commits(new_parents.iter().ids().cloned().collect_vec()),
+        )?;
     }
 
-    move_commits_transaction(
+    Ok((new_parents, new_children))
-        ui,
-        settings,
-        workspace_command,
-        &new_parents.iter().ids().cloned().collect_vec(),
-        &[],
-        target_commits,
-        None,
-        rebase_options,
-    )
-}
-
-fn rebase_revisions_after(
-    ui: &mut Ui,
-    settings: &UserSettings,
-    workspace_command: &mut WorkspaceCommandHelper,
-    after_commits: &IndexSet<Commit>,
-    target_commits: &[Commit],
-    rebase_options: &RebaseOptions,
-) -> Result<(), CommandError> {
-    workspace_command.check_rewritable(target_commits.iter().ids())?;
-
-    let after_commit_ids = after_commits.iter().ids().cloned().collect_vec();
-    let new_parents_expression = RevsetExpression::commits(after_commit_ids.clone());
-    let new_children_expression = new_parents_expression.children();
-
-    ensure_no_commit_loop(
-        workspace_command.repo().as_ref(),
-        &new_children_expression,
-        &new_parents_expression,
-    )?;
-
-    let new_parent_ids = after_commit_ids;
-    let new_children: Vec<_> = new_children_expression
-        .evaluate_programmatic(workspace_command.repo().as_ref())?
-        .iter()
-        .commits(workspace_command.repo().store())
-        .try_collect()?;
-    workspace_command.check_rewritable(new_children.iter().ids())?;
-
-    move_commits_transaction(
-        ui,
-        settings,
-        workspace_command,
-        &new_parent_ids,
-        &new_children,
-        target_commits,
-        None,
-        rebase_options,
-    )
-}
-
-fn rebase_revisions_before(
-    ui: &mut Ui,
-    settings: &UserSettings,
-    workspace_command: &mut WorkspaceCommandHelper,
-    before_commits: &IndexSet<Commit>,
-    target_commits: &[Commit],
-    rebase_options: &RebaseOptions,
-) -> Result<(), CommandError> {
-    workspace_command.check_rewritable(target_commits.iter().ids())?;
-    let before_commit_ids = before_commits.iter().ids().cloned().collect_vec();
-    workspace_command.check_rewritable(&before_commit_ids)?;
-
-    let new_children_expression = RevsetExpression::commits(before_commit_ids);
-    let new_parents_expression = new_children_expression.parents();
-
-    ensure_no_commit_loop(
-        workspace_command.repo().as_ref(),
-        &new_children_expression,
-        &new_parents_expression,
-    )?;
-
-    // Not using `new_parents_expression` here to persist the order of parents
-    // specified in `before_commits`.
-    let new_parent_ids: IndexSet<_> = before_commits
-        .iter()
-        .flat_map(|commit| commit.parent_ids().iter().cloned().collect_vec())
-        .collect();
-    let new_parent_ids = new_parent_ids.into_iter().collect_vec();
-    let new_children = before_commits.iter().cloned().collect_vec();
-
-    move_commits_transaction(
-        ui,
-        settings,
-        workspace_command,
-        &new_parent_ids,
-        &new_children,
-        target_commits,
-        None,
-        rebase_options,
-    )
 }
 
+/// Creates a transaction for rebasing revisions.
 #[allow(clippy::too_many_arguments)]
-fn rebase_revisions_after_before(
+fn rebase_revisions_transaction(
-    ui: &mut Ui,
-    settings: &UserSettings,
-    workspace_command: &mut WorkspaceCommandHelper,
-    after_commits: &IndexSet<Commit>,
-    before_commits: &IndexSet<Commit>,
-    target_commits: &[Commit],
-    rebase_options: &RebaseOptions,
-) -> Result<(), CommandError> {
-    workspace_command.check_rewritable(target_commits.iter().ids())?;
-    let before_commit_ids = before_commits.iter().ids().cloned().collect_vec();
-    workspace_command.check_rewritable(&before_commit_ids)?;
-
-    let after_commit_ids = after_commits.iter().ids().cloned().collect_vec();
-    let new_children_expression = RevsetExpression::commits(before_commit_ids);
-    let new_parents_expression = RevsetExpression::commits(after_commit_ids.clone());
-
-    ensure_no_commit_loop(
-        workspace_command.repo().as_ref(),
-        &new_children_expression,
-        &new_parents_expression,
-    )?;
-
-    let new_parent_ids = after_commit_ids;
-    let new_children = before_commits.iter().cloned().collect_vec();
-
-    move_commits_transaction(
-        ui,
-        settings,
-        workspace_command,
-        &new_parent_ids,
-        &new_children,
-        target_commits,
-        None,
-        rebase_options,
-    )
-}
-
-/// Wraps `move_commits` in a transaction.
-#[allow(clippy::too_many_arguments)]
-fn move_commits_transaction(
     ui: &mut Ui,
     settings: &UserSettings,
     workspace_command: &mut WorkspaceCommandHelper,