jj resolve --list: Add descriptions of conflict complexity

The descriptions focus on adds, not the deletions. The deletions are used
to compute the number of deleted files that must be sides of the conflict.
This commit is contained in:
Ilya Grigoriev 2023-01-04 16:34:56 -08:00
parent 1f39ffce01
commit 621293d7c6
3 changed files with 116 additions and 22 deletions

View file

@ -29,6 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `jj debug config-schema` command prints out JSON schema for the jj TOML config
file format.
* `jj resolve --list` can now describe the complexity of conflicts.
* `jj resolve` now notifies the user of remaining conflicts, if any, on success.
This can be prevented by the new `--quiet` option.

View file

@ -2399,6 +2399,7 @@ fn cmd_resolve(
if args.list {
return print_conflicted_paths(
&conflicts,
&tree,
ui.stdout_formatter().as_mut(),
&workspace_command,
);
@ -2425,6 +2426,7 @@ fn cmd_resolve(
ui.write("After this operation, some files at this revision still have conflicts:\n")?;
print_conflicted_paths(
&new_conflicts,
&tree,
ui.stdout_formatter().as_mut(),
&workspace_command,
)?;
@ -2435,20 +2437,51 @@ fn cmd_resolve(
fn print_conflicted_paths(
conflicts: &[(RepoPath, jujutsu_lib::backend::ConflictId)],
tree: &Tree,
formatter: &mut dyn Formatter,
workspace_command: &WorkspaceCommandHelper,
) -> Result<(), CommandError> {
for (repo_path, _conflict_id) in conflicts.iter() {
// TODO: Similar to `jj diff --summary`, insert a few letters
// before the filename to indicate the kind of conflict.
// E.g. we could have a letter per add : `FF` is a usual conflict
// between two versions of a file, `FD` is a file vs directory,
// `FFF` for a merge of three conflicting versions. Additionally,
// if (# removes) + 1 > (# adds), this indicates the file was deleted
// in some versions of the conflict. Perhaps that should be `R` for removed.
for (repo_path, conflict_id) in conflicts.iter() {
let conflict = tree.store().read_conflict(repo_path, conflict_id)?;
let n_adds = conflict.adds.len();
let sides = n_adds.max(conflict.removes.len() + 1);
let deletions = sides - n_adds;
let mut seen_objects = BTreeSet::new(); // Sort for consistency and easier testing
if deletions > 0 {
seen_objects.insert(format!(
"{deletions} deletion{}",
if deletions > 1 { "s" } else { "" }
));
}
for object in conflict.adds.iter() {
seen_objects.insert(
match object.value {
TreeValue::File {
executable: false, ..
} => continue,
TreeValue::File {
executable: true, ..
} => "an executable",
TreeValue::Symlink(_) => "a symlink",
TreeValue::Tree(_) => "a directory",
TreeValue::GitSubmodule(_) => "a git submodule",
TreeValue::Conflict(_) => "another conflict (you found a bug!)",
}
.to_string(),
);
}
let seen_objects = seen_objects.into_iter().collect_vec();
let msg_tail = match &seen_objects[..] {
[] => "".to_string(),
[only] => format!(" including {only}"),
[first @ .., last] => format!(" including {} and {}", first.join(", "), last),
};
let msg = format!("{sides}-sided conflict{msg_tail}");
writeln!(
formatter,
"{}",
"{}: {msg}",
&workspace_command.format_file_path(repo_path)
)?;
}

View file

@ -64,7 +64,7 @@ fn test_resolution() {
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r###"
file
file: 2-sided conflict
"###);
insta::assert_snapshot!(
std::fs::read_to_string(repo_path.join("file")).unwrap()
@ -184,7 +184,7 @@ conflict
Working copy now at: 0bb40c908c8b conflict
Added 0 files, modified 1 files, removed 0 files
After this operation, some files at this revision still have conflicts:
file
file: 2-sided conflict
"###);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor2")).unwrap(), @r###"
@ -210,7 +210,7 @@ conflict
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r###"
file
file: 2-sided conflict
"###);
// Check that if merge tool leaves conflict markers in output file but
@ -311,7 +311,7 @@ fn test_normal_conflict_input_files() {
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r###"
file
file: 2-sided conflict
"###);
insta::assert_snapshot!(
std::fs::read_to_string(repo_path.join("file")).unwrap()
@ -352,7 +352,7 @@ fn test_baseless_conflict_input_files() {
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r###"
file
file: 2-sided conflict
"###);
insta::assert_snapshot!(
std::fs::read_to_string(repo_path.join("file")).unwrap()
@ -383,7 +383,7 @@ fn test_too_many_parents() {
create_commit(&test_env, &repo_path, "conflict", &["a", "b", "c"], &[]);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r###"
file
file: 3-sided conflict
"###);
let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]);
@ -416,7 +416,7 @@ fn test_edit_delete_conflict_input_files() {
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r###"
file
file: 2-sided conflict including 1 deletion
"###);
insta::assert_snapshot!(
std::fs::read_to_string(repo_path.join("file")).unwrap()
@ -449,10 +449,19 @@ fn test_file_vs_dir() {
// Without a placeholder file, `jj` ignores an empty directory
std::fs::write(repo_path.join("file").join("placeholder"), "").unwrap();
create_commit(&test_env, &repo_path, "conflict", &["a", "b"], &[]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
|\
o | b
| o a
|/
o base
o
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r###"
file
file: 2-sided conflict including a directory
"###);
let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]);
insta::assert_snapshot!(error, @r###"
@ -465,6 +474,56 @@ fn test_file_vs_dir() {
"###);
}
#[test]
fn test_description_with_dir_and_deletion() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
create_commit(&test_env, &repo_path, "base", &[], &[("file", "base\n")]);
create_commit(&test_env, &repo_path, "edit", &["base"], &[("file", "b\n")]);
create_commit(&test_env, &repo_path, "dir", &["base"], &[]);
std::fs::remove_file(repo_path.join("file")).unwrap();
std::fs::create_dir(repo_path.join("file")).unwrap();
// Without a placeholder file, `jj` ignores an empty directory
std::fs::write(repo_path.join("file").join("placeholder"), "").unwrap();
create_commit(&test_env, &repo_path, "del", &["base"], &[]);
std::fs::remove_file(repo_path.join("file")).unwrap();
create_commit(
&test_env,
&repo_path,
"conflict",
&["edit", "dir", "del"],
&[],
);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@-. conflict
|\ \
o | | del
| o | dir
|/ /
| o edit
|/
o base
o
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r###"
file: 3-sided conflict including 1 deletion and a directory
"###);
let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]);
insta::assert_snapshot!(error, @r###"
Error: Failed to use external tool to resolve: Only conflicts that involve normal files (not symlinks, not executable, etc.) are supported. Conflict summary for "file":
Conflict:
Removing file with id df967b96a579e45a18b8251732d16804b2e56a55
Removing file with id df967b96a579e45a18b8251732d16804b2e56a55
Adding file with id 61780798228d17af2d34fce4cfbdf35556832472
Adding tree with id 133bb38fc4e4bf6b551f1f04db7e48f04cac2877
"###);
}
#[test]
fn test_multiple_conflicts() {
let mut test_env = TestEnvironment::default();
@ -527,8 +586,8 @@ fn test_multiple_conflicts() {
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r###"
file1
file2
file1: 2-sided conflict
file2: 2-sided conflict
"###);
let editor_script = test_env.set_up_fake_editor();
@ -540,7 +599,7 @@ fn test_multiple_conflicts() {
Working copy now at: 06cafc2b5489 conflict
Added 0 files, modified 1 files, removed 0 files
After this operation, some files at this revision still have conflicts:
file1
file1: 2-sided conflict
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]),
@r###"
@ -555,7 +614,7 @@ fn test_multiple_conflicts() {
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r###"
file1
file1: 2-sided conflict
"###);
// Repeat the above with the `--quiet` option.
@ -591,7 +650,7 @@ fn test_multiple_conflicts() {
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list"]),
@r###"
file2
file2: 2-sided conflict
"###);
std::fs::write(
&editor_script,