Skip to content

Commit

Permalink
resolve: add --all option
Browse files Browse the repository at this point in the history
If many files are conflicted, it would be nice to be able to resolve all
conflicts at once without having to run `jj resolve` multiple times.
This is especially nice for merge tools which try to automatically
resolve conflicts without user input, but it can also be used for
regular merge editors like VS Code.

The new `--all` option invokes the tool for each file, stopping
immediately if an error occurs. This means the user can cancel the merge
partway through without finishing resolving every file. In this case,
the error is reported as a warning instead, and the already-resolved
files are saved.
  • Loading branch information
scott2000 committed Dec 16, 2024
1 parent d574259 commit 11fc840
Show file tree
Hide file tree
Showing 5 changed files with 245 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
* A new Email template type is added. `Signature.email()` now returns an Email
template type instead of a String.

* `jj resolve` now supports `--all` to resolve all conflicts instead of just one.

### Fixed bugs

* The `$NO_COLOR` environment variable must now be non-empty to be respected.
Expand Down
2 changes: 1 addition & 1 deletion cli/src/command_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ fn print_error(
Ok(())
}

fn print_error_sources(ui: &Ui, source: Option<&dyn error::Error>) -> io::Result<()> {
pub fn print_error_sources(ui: &Ui, source: Option<&dyn error::Error>) -> io::Result<()> {
let Some(err) = source else {
return Ok(());
};
Expand Down
59 changes: 49 additions & 10 deletions cli/src/commands/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,24 @@ use clap_complete::ArgValueCandidates;
use clap_complete::ArgValueCompleter;
use itertools::Itertools;
use jj_lib::object_id::ObjectId;
use jj_lib::repo::Repo;
use tracing::instrument;

use crate::cli_util::print_conflicted_paths;
use crate::cli_util::CommandHelper;
use crate::cli_util::RevisionArg;
use crate::command_error::cli_error;
use crate::command_error::print_error_sources;
use crate::command_error::CommandError;
use crate::complete;
use crate::ui::Ui;

/// Resolve a conflicted file with an external merge tool
/// Resolve conflicted files with an external merge tool
///
/// Only conflicts that can be resolved with a 3-way merge are supported. See
/// docs for merge tool configuration instructions.
/// docs for merge tool configuration instructions. By default, a single
/// conflicted file will be resolved at a time. Use `--all` to run the merge
/// tool on each conflicted file.
///
/// Note that conflicts can also be resolved without using this command. You may
/// edit the conflict markers in the conflicted file directly with a text
Expand All @@ -56,6 +60,13 @@ pub(crate) struct ResolveArgs {
// `diff --summary`, but should be more verbose.
#[arg(long, short)]
list: bool,
/// Instead of resolving one conflict, resolve all conflicts one at a time
///
/// If conflict resolution is cancelled for a file after successfully
/// resolving other files, the already-resolved files will still be
/// committed.
#[arg(long, short, conflicts_with = "list")]
all: bool,
/// Specify 3-way merge tool to be used
#[arg(long, conflicts_with = "list", value_name = "NAME")]
tool: Option<String>,
Expand Down Expand Up @@ -101,20 +112,48 @@ pub(crate) fn cmd_resolve(
);
};

let (repo_path, _) = conflicts.first().unwrap();
workspace_command.check_rewritable([commit.id()])?;
let merge_editor = workspace_command.merge_editor(ui, args.tool.as_deref())?;
writeln!(
ui.status(),
"Resolving conflicts in: {}",
workspace_command.format_file_path(repo_path)
)?;
let mut tx = workspace_command.start_transaction();
let new_tree_id = merge_editor.edit_file(&tree, repo_path)?;

let conflicts_to_resolve = if args.all {
&conflicts
} else {
&conflicts[..1]
};

let mut new_tree = tree;
for (i, (repo_path, _)) in conflicts_to_resolve.iter().enumerate() {
writeln!(
ui.status(),
"Resolving conflicts in: {}",
tx.base_workspace_helper().format_file_path(repo_path)
)?;
match merge_editor.edit_file(&new_tree, repo_path) {
Ok(tree_id) => {
new_tree = tx.repo().store().get_root_tree(&tree_id)?;
}
Err(err) if i == 0 => {
// Since no conflicts were successfully resolved, return the error
return Err(err.into());
}
Err(err) => {
// Since some conflicts were already successfully resolved, just print a warning
// and stop here
writeln!(
ui.warning_default(),
"Stopping due to error after resolving {i} conflicts"
)?;
print_error_sources(ui, Some(&err))?;
break;
}
}
}

let new_commit = tx
.repo_mut()
.rewrite_commit(command.settings(), &commit)
.set_tree_id(new_tree_id)
.set_tree_id(new_tree.id())
.write()?;
tx.finish(
ui,
Expand Down
9 changes: 6 additions & 3 deletions cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ To get started, see the tutorial at https://martinvonz.github.io/jj/latest/tutor
* `parallelize`Parallelize revisions by making them siblings
* `prev`Change the working copy revision relative to the parent revision
* `rebase`Move revisions to different parent(s)
* `resolve`Resolve a conflicted file with an external merge tool
* `resolve`Resolve conflicted files with an external merge tool
* `restore`Restore paths from another revision
* `root`Show the current workspace root directory
* `show`Show commit description and changes in a revision
Expand Down Expand Up @@ -1880,9 +1880,9 @@ commit. This is true in general; it is not specific to this command.
## `jj resolve`
Resolve a conflicted file with an external merge tool
Resolve conflicted files with an external merge tool
Only conflicts that can be resolved with a 3-way merge are supported. See docs for merge tool configuration instructions.
Only conflicts that can be resolved with a 3-way merge are supported. See docs for merge tool configuration instructions. By default, a single conflicted file will be resolved at a time. Use `--all` to run the merge tool on each conflicted file.
Note that conflicts can also be resolved without using this command. You may edit the conflict markers in the conflicted file directly with a text editor.
Expand All @@ -1898,6 +1898,9 @@ Note that conflicts can also be resolved without using this command. You may edi
Default value: `@`
* `-l`, `--list` — Instead of resolving one conflict, list all the conflicts
* `-a`, `--all` — Instead of resolving one conflict, resolve all conflicts one at a time
If conflict resolution is cancelled for a file after successfully resolving other files, the already-resolved files will still be committed.
* `--tool <NAME>` — Specify 3-way merge tool to be used
Expand Down
187 changes: 187 additions & 0 deletions cli/tests/test_resolve_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,193 @@ fn test_resolution() {
// correctly.
}

#[test]
fn test_resolution_with_all() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");

// Create two conflicted files, and one non-conflicted file
create_commit(
&test_env,
&repo_path,
"base",
&[],
&[
("file1", "base1\n"),
("file2", "base2\n"),
("file3", "base3\n"),
],
);
create_commit(
&test_env,
&repo_path,
"a",
&["base"],
&[("file1", "a1\n"), ("file2", "a2\n")],
);
create_commit(
&test_env,
&repo_path,
"b",
&["base"],
&[("file1", "b1\n"), ("file2", "b2\n")],
);
create_commit(&test_env, &repo_path, "conflict", &["a", "b"], &[]);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r#"
file1 2-sided conflict
file2 2-sided conflict
"#);
insta::assert_snapshot!(
std::fs::read_to_string(repo_path.join("file1")).unwrap(),
@r##"
<<<<<<< Conflict 1 of 1
%%%%%%% Changes from base to side #1
-base1
+a1
+++++++ Contents of side #2
b1
>>>>>>> Conflict 1 of 1 ends
"##
);
insta::assert_snapshot!(
std::fs::read_to_string(repo_path.join("file2")).unwrap(),
@r##"
<<<<<<< Conflict 1 of 1
%%%%%%% Changes from base to side #1
-base2
+a2
+++++++ Contents of side #2
b2
>>>>>>> Conflict 1 of 1 ends
"##
);
let editor_script = test_env.set_up_fake_editor();

// Test resolving both conflicts
std::fs::write(
&editor_script,
[
"write\nresolution1\n",
"next invocation\n",
"write\nresolution2\n",
]
.join("\0"),
)
.unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["resolve", "--all"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
Resolving conflicts in: file1
Resolving conflicts in: file2
Working copy now at: vruxwmqv 8fffa1fb conflict | conflict
Parent commit : zsuskuln 9db7fdfb a | a
Parent commit : royxmykx d67e26e4 b | b
Added 0 files, modified 2 files, removed 0 files
"#);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]),
@r##"
diff --git a/file1 b/file1
index 0000000000..95cc18629d 100644
--- a/file1
+++ b/file1
@@ -1,7 +1,1 @@
-<<<<<<< Conflict 1 of 1
-%%%%%%% Changes from base to side #1
--base1
-+a1
-+++++++ Contents of side #2
-b1
->>>>>>> Conflict 1 of 1 ends
+resolution1
diff --git a/file2 b/file2
index 0000000000..775f078581 100644
--- a/file2
+++ b/file2
@@ -1,7 +1,1 @@
-<<<<<<< Conflict 1 of 1
-%%%%%%% Changes from base to side #1
--base2
-+a2
-+++++++ Contents of side #2
-b2
->>>>>>> Conflict 1 of 1 ends
+resolution2
"##);
insta::assert_snapshot!(test_env.jj_cmd_cli_error(&repo_path, &["resolve", "--list"]),
@r###"
Error: No conflicts found at this revision
"###);

// Test resolving one conflict, then exiting without resolving the second one
test_env.jj_cmd_ok(&repo_path, &["undo"]);
std::fs::write(
&editor_script,
["write\nresolution1\n", "next invocation\n", "fail"].join("\0"),
)
.unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["resolve", "--all"]);
insta::assert_snapshot!(stdout, @r#""#);
insta::assert_snapshot!(stderr.replace("exit code", "exit status"), @r#"
Resolving conflicts in: file1
Resolving conflicts in: file2
Warning: Stopping due to error after resolving 1 conflicts
Caused by: Tool exited with exit status: 1 (run with --debug to see the exact invocation)
Working copy now at: vruxwmqv 22b6cad7 conflict | (conflict) conflict
Parent commit : zsuskuln 9db7fdfb a | a
Parent commit : royxmykx d67e26e4 b | b
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file2 2-sided conflict
New conflicts appeared in these commits:
vruxwmqv 22b6cad7 conflict | (conflict) conflict
To resolve the conflicts, start by updating to it:
jj new vruxwmqv
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
"#);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]),
@r##"
diff --git a/file1 b/file1
index 0000000000..95cc18629d 100644
--- a/file1
+++ b/file1
@@ -1,7 +1,1 @@
-<<<<<<< Conflict 1 of 1
-%%%%%%% Changes from base to side #1
--base1
-+a1
-+++++++ Contents of side #2
-b1
->>>>>>> Conflict 1 of 1 ends
+resolution1
"##);
insta::assert_snapshot!(
test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@"file2 2-sided conflict"
);

// Test immediately failing to resolve any conflict
test_env.jj_cmd_ok(&repo_path, &["undo"]);
std::fs::write(&editor_script, "fail").unwrap();
let stderr = test_env.jj_cmd_failure(&repo_path, &["resolve", "--all"]);
insta::assert_snapshot!(stderr.replace("exit code", "exit status"), @r#"
Resolving conflicts in: file1
Error: Failed to resolve conflicts
Caused by: Tool exited with exit status: 1 (run with --debug to see the exact invocation)
"#);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @"");
insta::assert_snapshot!(
test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r#"
file1 2-sided conflict
file2 2-sided conflict
"#
);
}

fn check_resolve_produces_input_file(
test_env: &mut TestEnvironment,
repo_path: &Path,
Expand Down

0 comments on commit 11fc840

Please sign in to comment.