cli: add thin wrapper that runs --interactive diff editor conditionally

I considered inlining tx.select_diff(), but that looked a bit cryptic because
the arguments orders are reasonably different. This thin wrapper will help
enforce the common interactive editing behavior.

cli/src/cli_util.rs

@@ -1093,6 +1093,16 @@ impl WorkspaceCommandHelper {
         Ok(DiffEditor::from_settings(ui, &self.settings, base_ignores)?)
     }
 
+    /// Conditionally loads diff editor from the settings.
+    // TODO: override settings by --tool= option (#2575)
+    pub fn diff_selector(&self, ui: &Ui, interactive: bool) -> Result<DiffSelector, CommandError> {
+        if interactive {
+            Ok(DiffSelector::Interactive(self.diff_editor(ui)?))
+        } else {
+            Ok(DiffSelector::NonInteractive)
+        }
+    }
+
     /// Loads 3-way merge editor from the settings.
     // TODO: override settings by --tool= option (#2575)
     pub fn merge_editor(&self, ui: &Ui) -> Result<MergeEditor, MergeToolConfigError> {
@@ -1726,23 +1736,6 @@ impl WorkspaceCommandTransaction<'_> {
         self.tx.mut_repo().edit(workspace_id, commit)
     }
 
-    // TODO: maybe inline or extract to free function (no dependency on self)
-    pub fn select_diff(
-        &self,
-        interactive_editor: Option<&DiffEditor>,
-        left_tree: &MergedTree,
-        right_tree: &MergedTree,
-        matcher: &dyn Matcher,
-        instructions: Option<&str>,
-    ) -> Result<MergedTreeId, CommandError> {
-        if let Some(editor) = interactive_editor {
-            Ok(editor.edit(left_tree, right_tree, matcher, instructions)?)
-        } else {
-            let new_tree_id = restore_tree(right_tree, left_tree, matcher)?;
-            Ok(new_tree_id)
-        }
-    }
-
     pub fn format_commit_summary(&self, commit: &Commit) -> String {
         let mut output = Vec::new();
         self.write_commit_summary(&mut PlainTextFormatter::new(&mut output), commit)
@@ -2370,6 +2363,36 @@ pub fn short_operation_hash(operation_id: &OperationId) -> String {
     operation_id.hex()[0..12].to_string()
 }
 
+/// Wrapper around a `DiffEditor` to conditionally start interactive session.
+#[derive(Clone, Debug)]
+pub enum DiffSelector {
+    NonInteractive,
+    Interactive(DiffEditor),
+}
+
+impl DiffSelector {
+    pub fn is_interactive(&self) -> bool {
+        matches!(self, DiffSelector::Interactive(_))
+    }
+
+    /// Restores diffs from the `right_tree` to the `left_tree` by using an
+    /// interactive editor if enabled.
+    pub fn select(
+        &self,
+        left_tree: &MergedTree,
+        right_tree: &MergedTree,
+        matcher: &dyn Matcher,
+        instructions: Option<&str>,
+    ) -> Result<MergedTreeId, CommandError> {
+        match self {
+            DiffSelector::NonInteractive => Ok(restore_tree(right_tree, left_tree, matcher)?),
+            DiffSelector::Interactive(editor) => {
+                Ok(editor.edit(left_tree, right_tree, matcher, instructions)?)
+            }
+        }
+    }
+}
+
 #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
 pub struct RemoteBranchName {
     pub branch: String,

cli/src/commands/commit.rs

@@ -49,11 +49,7 @@ pub(crate) fn cmd_commit(
         .ok_or_else(|| user_error("This command requires a working copy"))?;
     let commit = workspace_command.repo().store().get_commit(commit_id)?;
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
-    let interactive_editor = if args.interactive {
-        Some(workspace_command.diff_editor(ui)?)
-    } else {
-        None
-    };
+    let diff_selector = workspace_command.diff_selector(ui, args.interactive)?;
     let mut tx = workspace_command.start_transaction();
     let base_tree = merge_commit_trees(tx.repo(), &commit.parents())?;
     let instructions = format!(
@@ -66,8 +62,7 @@ new working-copy commit.
 ",
         tx.format_commit_summary(&commit)
     );
-    let tree_id = tx.select_diff(
-        interactive_editor.as_ref(),
+    let tree_id = diff_selector.select(
         &base_tree,
         &commit.tree()?,
         matcher.as_ref(),

cli/src/commands/move.rs

@@ -69,11 +69,7 @@ pub(crate) fn cmd_move(
     }
     workspace_command.check_rewritable([&source, &destination])?;
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
-    let interactive_editor = if args.interactive {
-        Some(workspace_command.diff_editor(ui)?)
-    } else {
-        None
-    };
+    let diff_selector = workspace_command.diff_selector(ui, args.interactive)?;
     let mut tx = workspace_command.start_transaction();
     let parent_tree = merge_commit_trees(tx.repo(), &source.parents())?;
     let source_tree = source.tree()?;
@@ -93,14 +89,13 @@ from the source will be moved into the destination.
         tx.format_commit_summary(&source),
         tx.format_commit_summary(&destination)
     );
-    let new_parent_tree_id = tx.select_diff(
-        interactive_editor.as_ref(),
+    let new_parent_tree_id = diff_selector.select(
         &parent_tree,
         &source_tree,
         matcher.as_ref(),
         Some(&instructions),
     )?;
-    if interactive_editor.is_some() && new_parent_tree_id == parent_tree.id() {
+    if diff_selector.is_interactive() && new_parent_tree_id == parent_tree.id() {
         return Err(user_error("No changes to move"));
     }
     let new_parent_tree = tx.repo().store().get_root_tree(&new_parent_tree_id)?;

cli/src/commands/split.rs

@@ -58,11 +58,8 @@ pub(crate) fn cmd_split(
     let commit = workspace_command.resolve_single_rev(&args.revision)?;
     workspace_command.check_rewritable([&commit])?;
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
-    let interactive_editor = if args.interactive || args.paths.is_empty() {
-        Some(workspace_command.diff_editor(ui)?)
-    } else {
-        None
-    };
+    let diff_selector =
+        workspace_command.diff_selector(ui, args.interactive || args.paths.is_empty())?;
     let mut tx = workspace_command.start_transaction();
     let end_tree = commit.tree()?;
     let base_tree = merge_commit_trees(tx.repo(), &commit.parents())?;
@@ -80,14 +77,9 @@ don't make any changes, then the operation will be aborted.
     );
 
     // Prompt the user to select the changes they want for the first commit.
-    let selected_tree_id = tx.select_diff(
-        interactive_editor.as_ref(),
-        &base_tree,
-        &end_tree,
-        matcher.as_ref(),
-        Some(&instructions),
-    )?;
-    if &selected_tree_id == commit.tree_id() && interactive_editor.is_some() {
+    let selected_tree_id =
+        diff_selector.select(&base_tree, &end_tree, matcher.as_ref(), Some(&instructions))?;
+    if &selected_tree_id == commit.tree_id() && diff_selector.is_interactive() {
         // The user selected everything from the original commit.
         writeln!(ui.stderr(), "Nothing changed.")?;
         return Ok(());

cli/src/commands/squash.rs

@@ -66,11 +66,7 @@ pub(crate) fn cmd_squash(
     let parent = &parents[0];
     workspace_command.check_rewritable(&parents[..1])?;
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
-    let interactive_editor = if args.interactive {
-        Some(workspace_command.diff_editor(ui)?)
-    } else {
-        None
-    };
+    let diff_selector = workspace_command.diff_selector(ui, args.interactive)?;
     let mut tx = workspace_command.start_transaction();
     let instructions = format!(
         "\
@@ -90,15 +86,10 @@ from the source will be moved into the parent.
     );
     let parent_tree = parent.tree()?;
     let tree = commit.tree()?;
-    let new_parent_tree_id = tx.select_diff(
-        interactive_editor.as_ref(),
-        &parent_tree,
-        &tree,
-        matcher.as_ref(),
-        Some(&instructions),
-    )?;
+    let new_parent_tree_id =
+        diff_selector.select(&parent_tree, &tree, matcher.as_ref(), Some(&instructions))?;
     if &new_parent_tree_id == parent.tree_id() {
-        if interactive_editor.is_some() {
+        if diff_selector.is_interactive() {
             return Err(user_error("No changes selected"));
         }