From 094e1a2e30e8a68cec8617f1e15a4c8b266e06a9 Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Tue, 29 Aug 2023 23:13:36 +0200 Subject: [PATCH] merge_tools: pass `Matcher` in for interactive use For `jj split --interactive`, the user will want to select changes from a subset of files. This means that we need to pass the `Matcher` object when materializing the list of changed files. I also updated the parameter lists so that the matcher always immediately follows the tree objects. --- cli/src/cli_util.rs | 6 ++++-- cli/src/commands/mod.rs | 16 +++++++++++----- cli/src/merge_tools/builtin.rs | 5 +++-- cli/src/merge_tools/external.rs | 9 +++++---- cli/src/merge_tools/mod.rs | 5 ++++- 5 files changed, 27 insertions(+), 14 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 1e9e89d9e3..a8f9d65902 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1478,6 +1478,7 @@ impl WorkspaceCommandTransaction<'_> { ui: &Ui, left_tree: &MergedTree, right_tree: &MergedTree, + matcher: &dyn Matcher, instructions: &str, ) -> Result { let base_ignores = self.helper.base_ignores(); @@ -1486,6 +1487,7 @@ impl WorkspaceCommandTransaction<'_> { ui, left_tree, right_tree, + matcher, instructions, base_ignores, settings, @@ -1497,12 +1499,12 @@ impl WorkspaceCommandTransaction<'_> { ui: &Ui, left_tree: &MergedTree, right_tree: &MergedTree, + matcher: &dyn Matcher, instructions: &str, interactive: bool, - matcher: &dyn Matcher, ) -> Result { if interactive { - self.edit_diff(ui, left_tree, right_tree, instructions) + self.edit_diff(ui, left_tree, right_tree, matcher, instructions) } else if matcher.visit(&RepoPath::root()) == Visit::AllRecursively { // Optimization for a common case Ok(right_tree.id().clone()) diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index 00e21023f8..3ba9cd29ec 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -2413,9 +2413,9 @@ from the source will be moved into the destination. ui, &parent_tree, &source_tree, + matcher.as_ref(), &instructions, args.interactive, - matcher.as_ref(), )?; if args.interactive && new_parent_tree_id == parent_tree.id() { return Err(user_error("No changes to move")); @@ -2497,9 +2497,9 @@ from the source will be moved into the parent. ui, &parent_tree, &tree, + matcher.as_ref(), &instructions, args.interactive, - matcher.as_ref(), )?; if &new_parent_tree_id == parent.tree_id() { if args.interactive { @@ -2593,7 +2593,13 @@ aborted. tx.format_commit_summary(&commit) ); let parent_tree = parent.tree()?; - new_parent_tree_id = tx.edit_diff(ui, &parent_base_tree, &parent_tree, &instructions)?; + new_parent_tree_id = tx.edit_diff( + ui, + &parent_base_tree, + &parent_tree, + &EverythingMatcher, + &instructions, + )?; if new_parent_tree_id == parent_base_tree.id() { return Err(user_error("No changes selected")); } @@ -2950,7 +2956,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(ui, &base_tree, &tree, &instructions)?; + let tree_id = tx.edit_diff(ui, &base_tree, &tree, &EverythingMatcher, &instructions)?; if tree_id == *target_commit.tree_id() { ui.write("Nothing changed.\n")?; } else { @@ -3057,9 +3063,9 @@ don't make any changes, then the operation will be aborted. ui, &base_tree, &end_tree, + matcher.as_ref(), &instructions, interactive, - matcher.as_ref(), )?; if &tree_id == commit.tree_id() && interactive { ui.write("Nothing changed.\n")?; diff --git a/cli/src/merge_tools/builtin.rs b/cli/src/merge_tools/builtin.rs index 8a30ad20af..6243b425fb 100644 --- a/cli/src/merge_tools/builtin.rs +++ b/cli/src/merge_tools/builtin.rs @@ -6,7 +6,7 @@ use itertools::Itertools; use jj_lib::backend::{BackendError, FileId, MergedTreeId, ObjectId, TreeValue}; use jj_lib::diff::{find_line_ranges, Diff, DiffHunk}; use jj_lib::files::{self, ContentHunk, MergeResult}; -use jj_lib::matchers::EverythingMatcher; +use jj_lib::matchers::Matcher; use jj_lib::merge::Merge; use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder}; use jj_lib::repo_path::RepoPath; @@ -405,10 +405,11 @@ pub fn apply_diff_builtin( pub fn edit_diff_builtin( left_tree: &MergedTree, right_tree: &MergedTree, + matcher: &dyn Matcher, ) -> Result { let store = left_tree.store().clone(); let changed_files = left_tree - .diff(right_tree, &EverythingMatcher) + .diff(right_tree, matcher) .map(|(path, _left, _right)| path) .collect_vec(); let files = make_diff_files(&store, left_tree, right_tree, &changed_files)?; diff --git a/cli/src/merge_tools/external.rs b/cli/src/merge_tools/external.rs index 5ab157c8d7..da9c2884b3 100644 --- a/cli/src/merge_tools/external.rs +++ b/cli/src/merge_tools/external.rs @@ -10,7 +10,7 @@ use itertools::Itertools; use jj_lib::backend::{FileId, MergedTreeId, TreeValue}; use jj_lib::conflicts::{self, materialize_merge_result}; use jj_lib::gitignore::GitIgnoreFile; -use jj_lib::matchers::{EverythingMatcher, Matcher}; +use jj_lib::matchers::Matcher; use jj_lib::merge::Merge; use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder}; use jj_lib::repo_path::RepoPath; @@ -233,8 +233,8 @@ fn check_out_trees( store: &Arc, left_tree: &MergedTree, right_tree: &MergedTree, - output_is: Option, matcher: &dyn Matcher, + output_is: Option, ) -> Result { let changed_files = left_tree .diff(right_tree, matcher) @@ -420,6 +420,7 @@ pub fn edit_diff_external( editor: ExternalMergeTool, left_tree: &MergedTree, right_tree: &MergedTree, + matcher: &dyn Matcher, instructions: &str, base_ignores: Arc, settings: &UserSettings, @@ -430,8 +431,8 @@ pub fn edit_diff_external( store, left_tree, right_tree, + matcher, got_output_field.then_some(DiffSide::Right), - &EverythingMatcher, )?; set_readonly_recursively(diff_wc.left_working_copy_path()) .map_err(ExternalToolError::SetUpDir)?; @@ -532,7 +533,7 @@ pub fn generate_diff( tool: &ExternalMergeTool, ) -> Result<(), DiffGenerateError> { let store = left_tree.store(); - let diff_wc = check_out_trees(store, left_tree, right_tree, None, matcher)?; + let diff_wc = check_out_trees(store, left_tree, right_tree, matcher, None)?; set_readonly_recursively(diff_wc.left_working_copy_path()) .map_err(ExternalToolError::SetUpDir)?; set_readonly_recursively(diff_wc.right_working_copy_path()) diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index 53457a55af..402a8cf6b4 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -21,6 +21,7 @@ use config::ConfigError; use jj_lib::backend::MergedTreeId; use jj_lib::conflicts::extract_as_single_hunk; use jj_lib::gitignore::GitIgnoreFile; +use jj_lib::matchers::Matcher; use jj_lib::merged_tree::MergedTree; use jj_lib::repo_path::RepoPath; use jj_lib::settings::{ConfigResultExt as _, UserSettings}; @@ -130,6 +131,7 @@ pub fn edit_diff( ui: &Ui, left_tree: &MergedTree, right_tree: &MergedTree, + matcher: &dyn Matcher, instructions: &str, base_ignores: Arc, settings: &UserSettings, @@ -138,13 +140,14 @@ pub fn edit_diff( let editor = get_diff_editor_from_settings(ui, settings)?; match editor { MergeTool::Builtin => { - let tree_id = edit_diff_builtin(left_tree, right_tree).map_err(Box::new)?; + 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, settings,