diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 5987f89cfa..03268e77f8 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1122,6 +1122,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 { + 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 { @@ -1755,23 +1765,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 { - 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) @@ -2399,6 +2392,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 { + 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, diff --git a/cli/src/commands/commit.rs b/cli/src/commands/commit.rs index fa33f5bfd5..a6f4906ae0 100644 --- a/cli/src/commands/commit.rs +++ b/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(), diff --git a/cli/src/commands/move.rs b/cli/src/commands/move.rs index 02bc0e4602..ea47374c9c 100644 --- a/cli/src/commands/move.rs +++ b/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)?; diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index 58e3062b45..ff3d918629 100644 --- a/cli/src/commands/split.rs +++ b/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(()); diff --git a/cli/src/commands/squash.rs b/cli/src/commands/squash.rs index 00fd40baf2..b66e4c7724 100644 --- a/cli/src/commands/squash.rs +++ b/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")); }