Skip to content

Commit

Permalink
merge_tools: pass Matcher in for interactive use
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
arxanas committed Aug 31, 2023
1 parent ea5692a commit ec3b821
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 14 deletions.
6 changes: 4 additions & 2 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1478,6 +1478,7 @@ impl WorkspaceCommandTransaction<'_> {
ui: &Ui,
left_tree: &MergedTree,
right_tree: &MergedTree,
matcher: &dyn Matcher,
instructions: &str,
) -> Result<MergedTreeId, CommandError> {
let base_ignores = self.helper.base_ignores();
Expand All @@ -1486,6 +1487,7 @@ impl WorkspaceCommandTransaction<'_> {
ui,
left_tree,
right_tree,
matcher,
instructions,
base_ignores,
settings,
Expand All @@ -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<MergedTreeId, CommandError> {
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())
Expand Down
16 changes: 11 additions & 5 deletions cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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"));
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")?;
Expand Down
5 changes: 3 additions & 2 deletions cli/src/merge_tools/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -405,10 +405,11 @@ pub fn apply_diff_builtin(
pub fn edit_diff_builtin(
left_tree: &MergedTree,
right_tree: &MergedTree,
matcher: &dyn Matcher,
) -> Result<MergedTreeId, BuiltinToolError> {
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)?;
Expand Down
9 changes: 5 additions & 4 deletions cli/src/merge_tools/external.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -233,8 +233,8 @@ fn check_out_trees(
store: &Arc<Store>,
left_tree: &MergedTree,
right_tree: &MergedTree,
output_is: Option<DiffSide>,
matcher: &dyn Matcher,
output_is: Option<DiffSide>,
) -> Result<DiffWorkingCopies, DiffCheckoutError> {
let changed_files = left_tree
.diff(right_tree, matcher)
Expand Down Expand Up @@ -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<GitIgnoreFile>,
settings: &UserSettings,
Expand All @@ -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)?;
Expand Down Expand Up @@ -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())
Expand Down
5 changes: 4 additions & 1 deletion cli/src/merge_tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -130,6 +131,7 @@ pub fn edit_diff(
ui: &Ui,
left_tree: &MergedTree,
right_tree: &MergedTree,
matcher: &dyn Matcher,
instructions: &str,
base_ignores: Arc<GitIgnoreFile>,
settings: &UserSettings,
Expand All @@ -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,
Expand Down

0 comments on commit ec3b821

Please sign in to comment.