Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge_tools: pass Matcher in for interactive use #2129

Merged
merged 1 commit into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1506,6 +1506,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 @@ -1514,6 +1515,7 @@ impl WorkspaceCommandTransaction<'_> {
ui,
left_tree,
right_tree,
matcher,
instructions,
base_ignores,
settings,
Expand All @@ -1525,12 +1527,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 @@ -2624,9 +2624,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 @@ -2708,9 +2708,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 @@ -2804,7 +2804,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 @@ -3161,7 +3167,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 @@ -3268,9 +3274,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 @@ -129,6 +130,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 @@ -137,13 +139,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