From fc9795e902e90d70b8d49c2664fd815e2e54a9be Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Sat, 7 Jan 2023 10:21:38 -0800 Subject: [PATCH] `jj resolve --list`: make output colorful The description is usually in yellow, which looks nice and neutral. If the conflict has more than 2 sides or has special files, that's marked in red. --- src/commands.rs | 61 ++++++++++++++++++++++++++--------- src/config/colors.toml | 3 ++ tests/test_resolve_command.rs | 20 ++++++++++-- 3 files changed, 67 insertions(+), 17 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index 457092834..591fb0d08 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::{BTreeSet, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::fmt::Debug; use std::io::{Read, Seek, SeekFrom, Write}; use std::ops::Deref; @@ -2448,12 +2448,16 @@ fn print_conflicted_paths( 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 + let mut seen_objects = BTreeMap::new(); // Sort for consistency and easier testing if deletions > 0 { - seen_objects.insert(format!( - "{deletions} deletion{}", - if deletions > 1 { "s" } else { "" } - )); + seen_objects.insert( + format!( + // Starting with a number sorts this first + "{deletions} deletion{}", + if deletions > 1 { "s" } else { "" } + ), + "normal", // Deletions don't interfere with `jj resolve` or diff display + ); } // TODO: We might decide it's OK for `jj resolve` to ignore special files in the // `removes` of a conflict (see e.g. https://github.com/martinvonz/jj/pull/978). In @@ -2473,21 +2477,48 @@ fn print_conflicted_paths( TreeValue::Conflict(_) => "another conflict (you found a bug!)", } .to_string(), + "difficult", ); } - 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!( + write!( formatter, - "{}\t{msg}", + "{}\t", &workspace_command.format_file_path(repo_path) )?; + formatter.with_label("conflict_description", |formatter| { + let print_pair = |formatter: &mut dyn Formatter, (text, label): &(String, &str)| { + formatter.with_label(label, |fmt| fmt.write_str(text)) + }; + print_pair( + formatter, + &( + format!("{sides}-sided"), + if sides > 2 { "difficult" } else { "normal" }, + ), + )?; + formatter.write_str(" conflict")?; + + if !seen_objects.is_empty() { + formatter.write_str(" including ")?; + let seen_objects = seen_objects.into_iter().collect_vec(); + match &seen_objects[..] { + [] => unreachable!(), + [only] => print_pair(formatter, only)?, + [first, middle @ .., last] => { + print_pair(formatter, first)?; + for pair in middle { + formatter.write_str(", ")?; + print_pair(formatter, pair)?; + } + formatter.write_str(" and ")?; + print_pair(formatter, last)?; + } + }; + } + Ok(()) + })?; + writeln!(formatter)?; } Ok(()) } diff --git a/src/config/colors.toml b/src/config/colors.toml index 48da0d5af..670c73e62 100644 --- a/src/config/colors.toml +++ b/src/config/colors.toml @@ -3,6 +3,9 @@ "warning" = "yellow" "hint" = "blue" +"conflict_description" = "yellow" +"conflict_description difficult" = "red" + "commit_id" = "blue" "change_id" = "magenta" "author" = "yellow" diff --git a/tests/test_resolve_command.rs b/tests/test_resolve_command.rs index a262554a7..314d899ce 100644 --- a/tests/test_resolve_command.rs +++ b/tests/test_resolve_command.rs @@ -375,6 +375,11 @@ 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"]), @"file 3-sided conflict"); + // Test warning color + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list", "--color=always"]), + @r###" + file 3-sided conflict + "###); let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]); insta::assert_snapshot!(error, @r###" @@ -502,6 +507,11 @@ fn test_description_with_dir_and_deletion() { @r###" file 3-sided conflict including 1 deletion and a directory "###); + // Test warning color. The deletion is fine, so it's not highlighted + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list", "--color=always"]), + @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": @@ -579,6 +589,12 @@ fn test_multiple_conflicts() { file1 2-sided conflict file2 2-sided conflict "###); + // Test colors + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list", "--color=always"]), + @r###" + file1 2-sided conflict + file2 2-sided conflict + "###); let editor_script = test_env.set_up_fake_editor(); @@ -586,7 +602,7 @@ fn test_multiple_conflicts() { std::fs::write(&editor_script, "expect\n\0write\nresolution file2\n").unwrap(); insta::assert_snapshot!( test_env.jj_cmd_success(&repo_path, &["resolve", "file2"]), @r###" - Working copy now at: 06cafc2b5489 conflict + Working copy now at: 5b4465fc6c31 conflict Added 0 files, modified 1 files, removed 0 files After this operation, some files at this revision still have conflicts: file1 2-sided conflict @@ -610,7 +626,7 @@ fn test_multiple_conflicts() { std::fs::write(&editor_script, "expect\n\0write\nresolution file2\n").unwrap(); insta::assert_snapshot!( test_env.jj_cmd_success(&repo_path, &["resolve", "--quiet", "file2"]), @r###" - Working copy now at: 02326c070aa4 conflict + Working copy now at: afb1b8512e98 conflict Added 0 files, modified 1 files, removed 0 files "###);