diff --git a/CHANGELOG.md b/CHANGELOG.md index c6640bcf7..e5131d0ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/src/commands.rs b/src/commands.rs index 6d206bad8..03d2db26a 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -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) )?; } diff --git a/tests/test_resolve_command.rs b/tests/test_resolve_command.rs index 96cba4ac8..d594dcb4e 100644 --- a/tests/test_resolve_command.rs +++ b/tests/test_resolve_command.rs @@ -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,