From 61cca4b3ffd3318c583ffbf01e30a86d37281859 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 29 Feb 2024 20:51:40 +0900 Subject: [PATCH 1/5] merge_tools: move run_mergetool() to MergeEditor::edit_file() This makes sense because the editor doesn't interact with the transaction. --- cli/src/cli_util.rs | 10 ----- cli/src/commands/resolve.rs | 2 +- cli/src/merge_tools/mod.rs | 81 +++++++++++++++++++------------------ 3 files changed, 42 insertions(+), 51 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 13166011f0..00e9189dc0 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1754,16 +1754,6 @@ impl WorkspaceCommandTransaction<'_> { self.tx.mut_repo().edit(workspace_id, commit) } - // TODO: inline this - pub fn run_mergetool( - &self, - editor: &MergeEditor, - tree: &MergedTree, - repo_path: &RepoPath, - ) -> Result { - Ok(merge_tools::run_mergetool(editor, tree, repo_path)?) - } - // TODO: maybe capture parameters by diff_editor(), and inline this? pub fn edit_diff( &self, diff --git a/cli/src/commands/resolve.rs b/cli/src/commands/resolve.rs index 9422faed18..0e5370cc75 100644 --- a/cli/src/commands/resolve.rs +++ b/cli/src/commands/resolve.rs @@ -103,7 +103,7 @@ pub(crate) fn cmd_resolve( workspace_command.format_file_path(repo_path) )?; let mut tx = workspace_command.start_transaction(); - let new_tree_id = tx.run_mergetool(&merge_editor, &tree, repo_path)?; + let new_tree_id = merge_editor.edit_file(&tree, repo_path)?; let new_commit = tx .mut_repo() .rewrite_commit(command.settings(), &commit) diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index 886fd817e0..dda43b2522 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -85,46 +85,6 @@ pub enum ConflictResolveError { Backend(#[from] jj_lib::backend::BackendError), } -pub fn run_mergetool( - editor: &MergeEditor, - tree: &MergedTree, - repo_path: &RepoPath, -) -> Result { - let conflict = match tree.path_value(repo_path).into_resolved() { - Err(conflict) => conflict, - Ok(Some(_)) => return Err(ConflictResolveError::NotAConflict(repo_path.to_owned())), - Ok(None) => return Err(ConflictResolveError::PathNotFound(repo_path.to_owned())), - }; - let file_merge = conflict.to_file_merge().ok_or_else(|| { - let mut summary_bytes: Vec = vec![]; - conflict - .describe(&mut summary_bytes) - .expect("Writing to an in-memory buffer should never fail"); - ConflictResolveError::NotNormalFiles( - repo_path.to_owned(), - String::from_utf8_lossy(summary_bytes.as_slice()).to_string(), - ) - })?; - // We only support conflicts with 2 sides (3-way conflicts) - if file_merge.num_sides() > 2 { - return Err(ConflictResolveError::ConflictTooComplicated { - path: repo_path.to_owned(), - sides: file_merge.num_sides(), - }); - }; - let content = extract_as_single_hunk(&file_merge, tree.store(), repo_path).block_on(); - - match &editor.tool { - MergeTool::Builtin => { - let tree_id = edit_merge_builtin(tree, repo_path, content).map_err(Box::new)?; - Ok(tree_id) - } - MergeTool::External(editor) => { - external::run_mergetool_external(editor, file_merge, content, repo_path, conflict, tree) - } - } -} - pub fn edit_diff( editor: &DiffEditor, left_tree: &MergedTree, @@ -265,6 +225,47 @@ impl MergeEditor { } Ok(MergeEditor { tool }) } + + /// Starts a merge editor for the specified file. + pub fn edit_file( + &self, + tree: &MergedTree, + repo_path: &RepoPath, + ) -> Result { + let conflict = match tree.path_value(repo_path).into_resolved() { + Err(conflict) => conflict, + Ok(Some(_)) => return Err(ConflictResolveError::NotAConflict(repo_path.to_owned())), + Ok(None) => return Err(ConflictResolveError::PathNotFound(repo_path.to_owned())), + }; + let file_merge = conflict.to_file_merge().ok_or_else(|| { + let mut summary_bytes: Vec = vec![]; + conflict + .describe(&mut summary_bytes) + .expect("Writing to an in-memory buffer should never fail"); + ConflictResolveError::NotNormalFiles( + repo_path.to_owned(), + String::from_utf8_lossy(summary_bytes.as_slice()).to_string(), + ) + })?; + // We only support conflicts with 2 sides (3-way conflicts) + if file_merge.num_sides() > 2 { + return Err(ConflictResolveError::ConflictTooComplicated { + path: repo_path.to_owned(), + sides: file_merge.num_sides(), + }); + }; + let content = extract_as_single_hunk(&file_merge, tree.store(), repo_path).block_on(); + + match &self.tool { + MergeTool::Builtin => { + let tree_id = edit_merge_builtin(tree, repo_path, content).map_err(Box::new)?; + Ok(tree_id) + } + MergeTool::External(editor) => external::run_mergetool_external( + editor, file_merge, content, repo_path, conflict, tree, + ), + } + } } #[cfg(test)] From c4e0994761afc87c4f9fa86e7dcf8370c373075c Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 29 Feb 2024 20:55:49 +0900 Subject: [PATCH 2/5] merge_tools: capture base_ignores and "ui.diff-instructions" by DiffEditor For the same reason as the previous commit. The editor has nothing to be done with the transaction. --- cli/src/cli_util.rs | 35 +++++----------------------------- cli/src/commands/diffedit.rs | 8 +------- cli/src/commands/unsquash.rs | 3 +-- cli/src/merge_tools/mod.rs | 37 +++++++++++++++++++++++++++++++++--- 4 files changed, 41 insertions(+), 42 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 00e9189dc0..5987f89cfa 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -93,7 +93,7 @@ use crate::merge_tools::{ use crate::template_parser::{TemplateAliasesMap, TemplateParseError, TemplateParseErrorKind}; use crate::templater::Template; use crate::ui::{ColorChoice, Ui}; -use crate::{commit_templater, merge_tools, text_util}; +use crate::{commit_templater, text_util}; #[derive(Clone, Debug)] pub enum CommandError { @@ -1117,8 +1117,9 @@ impl WorkspaceCommandHelper { /// Loads diff editor from the settings. // TODO: override settings by --tool= option (#2575) - pub fn diff_editor(&self, ui: &Ui) -> Result { - DiffEditor::from_settings(ui, &self.settings) + pub fn diff_editor(&self, ui: &Ui) -> Result { + let base_ignores = self.base_ignores()?; + Ok(DiffEditor::from_settings(ui, &self.settings, base_ignores)?) } /// Loads 3-way merge editor from the settings. @@ -1754,32 +1755,6 @@ impl WorkspaceCommandTransaction<'_> { self.tx.mut_repo().edit(workspace_id, commit) } - // TODO: maybe capture parameters by diff_editor(), and inline this? - pub fn edit_diff( - &self, - editor: &DiffEditor, - left_tree: &MergedTree, - right_tree: &MergedTree, - matcher: &dyn Matcher, - instructions: Option<&str>, - ) -> Result { - let base_ignores = self.helper.base_ignores()?; - let settings = &self.helper.settings; - let instructions = if settings.diff_instructions() { - instructions - } else { - None - }; - Ok(merge_tools::edit_diff( - editor, - left_tree, - right_tree, - matcher, - instructions, - base_ignores, - )?) - } - // TODO: maybe inline or extract to free function (no dependency on self) pub fn select_diff( &self, @@ -1790,7 +1765,7 @@ impl WorkspaceCommandTransaction<'_> { instructions: Option<&str>, ) -> Result { if let Some(editor) = interactive_editor { - self.edit_diff(editor, left_tree, right_tree, matcher, instructions) + 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) diff --git a/cli/src/commands/diffedit.rs b/cli/src/commands/diffedit.rs index 7181ecd1d5..6adf43f728 100644 --- a/cli/src/commands/diffedit.rs +++ b/cli/src/commands/diffedit.rs @@ -91,13 +91,7 @@ don't make any changes, then the operation will be aborted.", ); let base_tree = merge_commit_trees(tx.repo(), base_commits.as_slice())?; let tree = target_commit.tree()?; - let tree_id = tx.edit_diff( - &diff_editor, - &base_tree, - &tree, - &EverythingMatcher, - Some(&instructions), - )?; + let tree_id = diff_editor.edit(&base_tree, &tree, &EverythingMatcher, Some(&instructions))?; if tree_id == *target_commit.tree_id() { writeln!(ui.stderr(), "Nothing changed.")?; } else { diff --git a/cli/src/commands/unsquash.rs b/cli/src/commands/unsquash.rs index c423bb92f9..db70767729 100644 --- a/cli/src/commands/unsquash.rs +++ b/cli/src/commands/unsquash.rs @@ -86,8 +86,7 @@ aborted. tx.format_commit_summary(&commit) ); let parent_tree = parent.tree()?; - new_parent_tree_id = tx.edit_diff( - diff_editor, + new_parent_tree_id = diff_editor.edit( &parent_base_tree, &parent_tree, &EverythingMatcher, diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index dda43b2522..a5eb976434 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -85,6 +85,7 @@ pub enum ConflictResolveError { Backend(#[from] jj_lib::backend::BackendError), } +// TODO: inline pub fn edit_diff( editor: &DiffEditor, left_tree: &MergedTree, @@ -186,11 +187,17 @@ pub fn get_external_tool_config( #[derive(Clone, Debug)] pub struct DiffEditor { tool: MergeTool, + base_ignores: Arc, + use_instructions: bool, } impl DiffEditor { /// Loads the default diff editor from the settings. - pub fn from_settings(ui: &Ui, settings: &UserSettings) -> Result { + pub fn from_settings( + ui: &Ui, + settings: &UserSettings, + base_ignores: Arc, + ) -> Result { let args = editor_args_from_settings(ui, settings, "ui.diff-editor")?; let tool = if let CommandNameAndArgs::String(name) = &args { get_tool_config(settings, name)? @@ -198,7 +205,30 @@ impl DiffEditor { None } .unwrap_or_else(|| MergeTool::External(ExternalMergeTool::with_edit_args(&args))); - Ok(DiffEditor { tool }) + Ok(DiffEditor { + tool, + base_ignores, + use_instructions: settings.diff_instructions(), + }) + } + + /// Starts a diff editor on the two directories. + pub fn edit( + &self, + left_tree: &MergedTree, + right_tree: &MergedTree, + matcher: &dyn Matcher, + instructions: Option<&str>, + ) -> Result { + let instructions = self.use_instructions.then_some(instructions).flatten(); + edit_diff( + self, + left_tree, + right_tree, + matcher, + instructions, + self.base_ignores.clone(), + ) } } @@ -287,7 +317,8 @@ mod tests { let config = config_from_string(text); let ui = Ui::with_config(&config).unwrap(); let settings = UserSettings::from_config(config); - DiffEditor::from_settings(&ui, &settings).map(|editor| editor.tool) + DiffEditor::from_settings(&ui, &settings, GitIgnoreFile::empty()) + .map(|editor| editor.tool) }; // Default From ac47317197c1d4c76e887a5fedea71118fb79361 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 29 Feb 2024 21:35:55 +0900 Subject: [PATCH 3/5] merge_tools: inline edit_diff() into DiffEditor::edit() --- cli/src/merge_tools/mod.rs | 51 ++++++++++++-------------------------- 1 file changed, 16 insertions(+), 35 deletions(-) diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index a5eb976434..e452134fb3 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -85,32 +85,6 @@ pub enum ConflictResolveError { Backend(#[from] jj_lib::backend::BackendError), } -// TODO: inline -pub fn edit_diff( - editor: &DiffEditor, - left_tree: &MergedTree, - right_tree: &MergedTree, - matcher: &dyn Matcher, - instructions: Option<&str>, - base_ignores: Arc, -) -> Result { - // Start a diff editor on the two directories. - match &editor.tool { - MergeTool::Builtin => { - let tree_id = edit_diff_builtin(left_tree, right_tree, matcher).map_err(Box::new)?; - Ok(tree_id) - } - MergeTool::External(editor) => edit_diff_external( - editor, - left_tree, - right_tree, - matcher, - instructions, - base_ignores, - ), - } -} - #[derive(Debug, Error)] pub enum MergeToolConfigError { #[error(transparent)] @@ -220,15 +194,22 @@ impl DiffEditor { matcher: &dyn Matcher, instructions: Option<&str>, ) -> Result { - let instructions = self.use_instructions.then_some(instructions).flatten(); - edit_diff( - self, - left_tree, - right_tree, - matcher, - instructions, - self.base_ignores.clone(), - ) + match &self.tool { + MergeTool::Builtin => { + Ok(edit_diff_builtin(left_tree, right_tree, matcher).map_err(Box::new)?) + } + MergeTool::External(editor) => { + let instructions = self.use_instructions.then_some(instructions).flatten(); + edit_diff_external( + editor, + left_tree, + right_tree, + matcher, + instructions, + self.base_ignores.clone(), + ) + } + } } } From 5d628265888458cf117316ac283768563b8fbe46 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 1 Mar 2024 15:33:45 +0900 Subject: [PATCH 4/5] merge_tools: move "ui.diff-instructions" to CLI and config/misc.toml There are no users of this option in jj-lib. Let's simplify it. --- cli/src/config/misc.toml | 1 + cli/src/merge_tools/mod.rs | 2 +- lib/src/settings.rs | 4 ---- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/cli/src/config/misc.toml b/cli/src/config/misc.toml index e247778fdc..c3daf74b99 100644 --- a/cli/src/config/misc.toml +++ b/cli/src/config/misc.toml @@ -5,6 +5,7 @@ tree-level-conflicts = true [ui] +diff-instructions = true paginate = "auto" pager = { command = ["less", "-FRX"], env = { LESSCHARSET = "utf-8" } } log-word-wrap = false diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index e452134fb3..91961e0b72 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -182,7 +182,7 @@ impl DiffEditor { Ok(DiffEditor { tool, base_ignores, - use_instructions: settings.diff_instructions(), + use_instructions: settings.config().get_bool("ui.diff-instructions")?, }) } diff --git a/lib/src/settings.rs b/lib/src/settings.rs index 6b4653f223..8ab207bcc4 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -228,10 +228,6 @@ impl UserSettings { .unwrap_or(false) } - pub fn diff_instructions(&self) -> bool { - self.config.get_bool("ui.diff-instructions").unwrap_or(true) - } - pub fn config(&self) -> &config::Config { &self.config } From d799a8e0950bcd69663559326a1b18002f164e54 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 1 Mar 2024 16:27:16 +0900 Subject: [PATCH 5/5] 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 | 57 ++++++++++++++++++++++++++------------ cli/src/commands/commit.rs | 9 ++---- cli/src/commands/move.rs | 11 ++------ cli/src/commands/split.rs | 18 ++++-------- cli/src/commands/squash.rs | 17 +++--------- 5 files changed, 54 insertions(+), 58 deletions(-) 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")); }