From d1435a29fdf5681aa0468f2b35a17754dd381caf Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Mon, 21 Aug 2023 14:13:41 -0700 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 | 22 +++++++++++++++++----- cli/src/merge_tools/external.rs | 9 +++++---- cli/src/merge_tools/internal.rs | 5 +++-- cli/src/merge_tools/mod.rs | 5 ++++- 5 files changed, 33 insertions(+), 14 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index f8eecb1e11..d80c8738d0 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1467,6 +1467,7 @@ impl WorkspaceCommandTransaction<'_> { ui: &Ui, left_tree: &Tree, right_tree: &Tree, + matcher: &dyn Matcher, instructions: &str, ) -> Result { let base_ignores = self.helper.base_ignores(); @@ -1475,6 +1476,7 @@ impl WorkspaceCommandTransaction<'_> { ui, left_tree, right_tree, + matcher, instructions, base_ignores, settings, @@ -1486,12 +1488,12 @@ impl WorkspaceCommandTransaction<'_> { ui: &Ui, left_tree: &Tree, right_tree: &Tree, + 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 2db9fd1b63..7995931ea1 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -2415,9 +2415,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")); @@ -2499,9 +2499,9 @@ from the source will be moved into the parent. ui, &parent.tree(), &commit.tree(), + matcher.as_ref(), &instructions, args.interactive, - matcher.as_ref(), )?; if &new_parent_tree_id == parent.tree_id() { if args.interactive { @@ -2594,7 +2594,13 @@ aborted. tx.format_commit_summary(parent), tx.format_commit_summary(&commit) ); - 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")); } @@ -2964,7 +2970,13 @@ don't make any changes, then the operation will be aborted.", tx.format_commit_summary(&target_commit), ); let base_tree = merge_commit_trees(tx.repo(), base_commits.as_slice())?; - let tree_id = tx.edit_diff(ui, &base_tree, &target_commit.tree(), &instructions)?; + let tree_id = tx.edit_diff( + ui, + &base_tree, + &target_commit.tree(), + &EverythingMatcher, + &instructions, + )?; if &tree_id == target_commit.tree_id() { ui.write("Nothing changed.\n")?; } else { @@ -3070,9 +3082,9 @@ don't make any changes, then the operation will be aborted. ui, &base_tree, &commit.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/external.rs b/cli/src/merge_tools/external.rs index 171fdce102..37ce21c946 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, TreeId, 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::repo_path::RepoPath; use jj_lib::settings::UserSettings; @@ -233,8 +233,8 @@ fn check_out_trees( store: &Arc, left_tree: &Tree, right_tree: &Tree, - output_is: Option, matcher: &dyn Matcher, + output_is: Option, ) -> Result { let changed_files = left_tree .diff(right_tree, matcher) @@ -423,6 +423,7 @@ pub fn edit_diff_external( editor: ExternalMergeTool, left_tree: &Tree, right_tree: &Tree, + matcher: &dyn Matcher, instructions: &str, base_ignores: Arc, settings: &UserSettings, @@ -433,8 +434,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)?; @@ -535,7 +536,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/internal.rs b/cli/src/merge_tools/internal.rs index 30ff22f7bb..bad50ca006 100644 --- a/cli/src/merge_tools/internal.rs +++ b/cli/src/merge_tools/internal.rs @@ -6,7 +6,7 @@ use itertools::Itertools; use jj_lib::backend::{BackendError, FileId, ObjectId, SymlinkId, TreeId, 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::repo_path::RepoPath; use jj_lib::store::Store; @@ -434,10 +434,11 @@ pub fn apply_diff_internal( pub fn edit_diff_internal( left_tree: &Tree, right_tree: &Tree, + 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, _value)| path) .collect_vec(); let files = make_diff_files(&store, left_tree, right_tree, &changed_files)?; diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index 6762e3ccc3..79969b4ee2 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::{TreeId, TreeValue}; use jj_lib::conflicts::extract_as_single_hunk; use jj_lib::gitignore::GitIgnoreFile; +use jj_lib::matchers::Matcher; use jj_lib::repo_path::RepoPath; use jj_lib::settings::{ConfigResultExt as _, UserSettings}; use jj_lib::tree::Tree; @@ -131,6 +132,7 @@ pub fn edit_diff( ui: &Ui, left_tree: &Tree, right_tree: &Tree, + matcher: &dyn Matcher, instructions: &str, base_ignores: Arc, settings: &UserSettings, @@ -139,13 +141,14 @@ pub fn edit_diff( let editor = get_diff_editor_from_settings(ui, settings)?; match editor { MergeTool::Internal => { - let tree_id = edit_diff_internal(left_tree, right_tree).map_err(Box::new)?; + let tree_id = edit_diff_internal(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,