diff --git a/CHANGELOG.md b/CHANGELOG.md index dc394a59aa..428fda42e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj branch list` now supports a `--conflicted/-c` option to show only conflicted branches. +* Commands which update the working copy now print the list of conflicted paths. + ### Fixed bugs ## [0.15.1] - 2024-03-06 diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index dc6012c10c..321eeb4274 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -13,7 +13,7 @@ // limitations under the License. use core::fmt; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::env::{self, ArgsOs, VarError}; use std::ffi::OsString; use std::fmt::Debug; @@ -34,13 +34,14 @@ use clap::error::{ContextKind, ContextValue}; use clap::{ArgAction, ArgMatches, Command, FromArgMatches}; use indexmap::{IndexMap, IndexSet}; use itertools::Itertools; -use jj_lib::backend::{ChangeId, CommitId, MergedTreeId}; +use jj_lib::backend::{ChangeId, CommitId, MergedTreeId, TreeValue}; use jj_lib::commit::Commit; use jj_lib::git_backend::GitBackend; use jj_lib::gitignore::{GitIgnoreError, GitIgnoreFile}; use jj_lib::hex_util::to_reverse_hex; use jj_lib::id_prefix::IdPrefixContext; use jj_lib::matchers::{EverythingMatcher, Matcher, PrefixMatcher}; +use jj_lib::merge::MergedTreeValue; use jj_lib::merged_tree::MergedTree; use jj_lib::object_id::ObjectId; use jj_lib::op_store::{OpStoreError, OperationId, WorkspaceId}; @@ -1160,6 +1161,17 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin if let Some(stats) = stats { print_checkout_stats(ui, stats, new_commit)?; } + if !self.global_args.quiet && Some(new_commit) != maybe_old_commit { + let mut formatter = ui.stderr_formatter(); + let conflicts = new_commit.tree()?.conflicts().collect_vec(); + if !conflicts.is_empty() { + writeln!( + formatter.labeled("conflict"), + "There are unresolved conflicts at these paths:" + )?; + print_conflicted_paths(&conflicts, formatter.as_mut(), &self)?; + } + } Ok(()) } @@ -1588,6 +1600,97 @@ pub fn check_stale_working_copy( } } +#[instrument(skip_all)] +pub fn print_conflicted_paths( + conflicts: &[(RepoPathBuf, MergedTreeValue)], + formatter: &mut dyn Formatter, + workspace_command: &WorkspaceCommandHelper, +) -> Result<(), CommandError> { + let formatted_paths = conflicts + .iter() + .map(|(path, _conflict)| workspace_command.format_file_path(path)) + .collect_vec(); + let max_path_len = formatted_paths.iter().map(|p| p.len()).max().unwrap_or(0); + let formatted_paths = formatted_paths + .into_iter() + .map(|p| format!("{:width$}", p, width = max_path_len.min(32) + 3)); + + for ((_, conflict), formatted_path) in std::iter::zip(conflicts.iter(), formatted_paths) { + let sides = conflict.num_sides(); + let n_adds = conflict.adds().flatten().count(); + let deletions = sides - n_adds; + + let mut seen_objects = BTreeMap::new(); // Sort for consistency and easier testing + if deletions > 0 { + 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 + // that case, `conflict.removes` should be removed below. + for term in itertools::chain(conflict.removes(), conflict.adds()).flatten() { + seen_objects.insert( + match term { + 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(), + "difficult", + ); + } + + write!(formatter, "{formatted_path} ",)?; + formatter.with_label("conflict_description", |formatter| { + let print_pair = |formatter: &mut dyn Formatter, (text, label): &(String, &str)| { + write!(formatter.labeled(label), "{text}") + }; + print_pair( + formatter, + &( + format!("{sides}-sided"), + if sides > 2 { "difficult" } else { "normal" }, + ), + )?; + write!(formatter, " conflict")?; + + if !seen_objects.is_empty() { + write!(formatter, " 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 { + write!(formatter, ", ")?; + print_pair(formatter, pair)?; + } + write!(formatter, " and ")?; + print_pair(formatter, last)?; + } + }; + } + Ok(()) + })?; + writeln!(formatter)?; + } + Ok(()) +} + pub fn print_checkout_stats( ui: &mut Ui, stats: CheckoutStats, diff --git a/cli/src/commands/resolve.rs b/cli/src/commands/resolve.rs index b91a9985ee..e83d03d89d 100644 --- a/cli/src/commands/resolve.rs +++ b/cli/src/commands/resolve.rs @@ -12,19 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::BTreeMap; use std::io::Write; use itertools::Itertools; -use jj_lib::backend::TreeValue; -use jj_lib::merge::MergedTreeValue; use jj_lib::object_id::ObjectId; -use jj_lib::repo_path::RepoPathBuf; use tracing::instrument; -use crate::cli_util::{CommandHelper, WorkspaceCommandHelper}; +use crate::cli_util::{print_conflicted_paths, CommandHelper}; use crate::command_error::{cli_error, CommandError}; -use crate::formatter::Formatter; use crate::ui::Ui; /// Resolve a conflicted file with an external merge tool @@ -51,10 +46,6 @@ pub(crate) struct ResolveArgs { // `diff --summary`, but should be more verbose. #[arg(long, short)] list: bool, - /// Do not print the list of remaining conflicts (if any) after resolving a - /// conflict - #[arg(long, short, conflicts_with = "list")] - quiet: bool, /// Specify 3-way merge tool to be used #[arg(long, conflicts_with = "list", value_name = "NAME")] tool: Option, @@ -105,8 +96,7 @@ pub(crate) fn cmd_resolve( )?; let mut tx = workspace_command.start_transaction(); let new_tree_id = merge_editor.edit_file(&tree, repo_path)?; - let new_commit = tx - .mut_repo() + tx.mut_repo() .rewrite_commit(command.settings(), &commit) .set_tree_id(new_tree_id) .write()?; @@ -114,112 +104,5 @@ pub(crate) fn cmd_resolve( ui, format!("Resolve conflicts in commit {}", commit.id().hex()), )?; - - if !args.quiet { - let new_tree = new_commit.tree()?; - let new_conflicts = new_tree.conflicts().collect_vec(); - if !new_conflicts.is_empty() { - writeln!( - ui.stderr(), - "After this operation, some files at this revision still have conflicts:" - )?; - print_conflicted_paths( - &new_conflicts, - ui.stderr_formatter().as_mut(), - &workspace_command, - )?; - } - }; - Ok(()) -} - -#[instrument(skip_all)] -pub(crate) fn print_conflicted_paths( - conflicts: &[(RepoPathBuf, MergedTreeValue)], - formatter: &mut dyn Formatter, - workspace_command: &WorkspaceCommandHelper, -) -> Result<(), CommandError> { - let formatted_paths = conflicts - .iter() - .map(|(path, _conflict)| workspace_command.format_file_path(path)) - .collect_vec(); - let max_path_len = formatted_paths.iter().map(|p| p.len()).max().unwrap_or(0); - let formatted_paths = formatted_paths - .into_iter() - .map(|p| format!("{:width$}", p, width = max_path_len.min(32) + 3)); - - for ((_, conflict), formatted_path) in std::iter::zip(conflicts.iter(), formatted_paths) { - let sides = conflict.num_sides(); - let n_adds = conflict.adds().flatten().count(); - let deletions = sides - n_adds; - - let mut seen_objects = BTreeMap::new(); // Sort for consistency and easier testing - if deletions > 0 { - 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 - // that case, `conflict.removes` should be removed below. - for term in itertools::chain(conflict.removes(), conflict.adds()).flatten() { - seen_objects.insert( - match term { - 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(), - "difficult", - ); - } - - write!(formatter, "{formatted_path} ",)?; - formatter.with_label("conflict_description", |formatter| { - let print_pair = |formatter: &mut dyn Formatter, (text, label): &(String, &str)| { - write!(formatter.labeled(label), "{text}") - }; - print_pair( - formatter, - &( - format!("{sides}-sided"), - if sides > 2 { "difficult" } else { "normal" }, - ), - )?; - write!(formatter, " conflict")?; - - if !seen_objects.is_empty() { - write!(formatter, " 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 { - write!(formatter, ", ")?; - print_pair(formatter, pair)?; - } - write!(formatter, " and ")?; - print_pair(formatter, last)?; - } - }; - } - Ok(()) - })?; - writeln!(formatter)?; - } Ok(()) } diff --git a/cli/src/commands/status.rs b/cli/src/commands/status.rs index 982570843a..5c190e6a62 100644 --- a/cli/src/commands/status.rs +++ b/cli/src/commands/status.rs @@ -18,8 +18,7 @@ use jj_lib::repo::Repo; use jj_lib::rewrite::merge_commit_trees; use tracing::instrument; -use super::resolve; -use crate::cli_util::CommandHelper; +use crate::cli_util::{print_conflicted_paths, CommandHelper}; use crate::command_error::CommandError; use crate::diff_util; use crate::ui::Ui; @@ -72,7 +71,7 @@ pub(crate) fn cmd_status( formatter.labeled("conflict"), "There are unresolved conflicts at these paths:" )?; - resolve::print_conflicted_paths(&conflicts, formatter, &workspace_command)? + print_conflicted_paths(&conflicts, formatter, &workspace_command)? } let template = workspace_command.commit_summary_template(); diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 04a8ddb9b9..f0836f2b8d 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1530,10 +1530,6 @@ Note that conflicts can also be resolved without using this command. You may edi Possible values: `true`, `false` -* `-q`, `--quiet` — Do not print the list of remaining conflicts (if any) after resolving a conflict - - Possible values: `true`, `false` - * `--tool ` — Specify 3-way merge tool to be used diff --git a/cli/tests/test_chmod_command.rs b/cli/tests/test_chmod_command.rs index 3da40b7bba..a43fa15fc8 100644 --- a/cli/tests/test_chmod_command.rs +++ b/cli/tests/test_chmod_command.rs @@ -234,6 +234,8 @@ fn test_chmod_file_dir_deletion_conflicts() { Parent commit : zsuskuln c51c9c55 file | file Parent commit : royxmykx 6b18b3c1 deletion | deletion Added 0 files, modified 1 files, removed 0 files + There are unresolved conflicts at these paths: + file 2-sided conflict including 1 deletion and an executable "###); let stdout = test_env.jj_cmd_success(&repo_path, &["debug", "tree", "-r=file_deletion"]); insta::assert_snapshot!(stdout, diff --git a/cli/tests/test_diffedit_command.rs b/cli/tests/test_diffedit_command.rs index 4d3cc96944..b1ea6f1f97 100644 --- a/cli/tests/test_diffedit_command.rs +++ b/cli/tests/test_diffedit_command.rs @@ -384,6 +384,8 @@ fn test_diffedit_merge() { Working copy now at: yqosqzyt 1de824f2 (conflict) (empty) (no description set) Parent commit : royxmykx b90654a0 (conflict) merge Added 0 files, modified 0 files, removed 1 files + There are unresolved conflicts at these paths: + file2 2-sided conflict "###); let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s", "-r", "@-"]); insta::assert_snapshot!(stdout, @r###" diff --git a/cli/tests/test_repo_change_report.rs b/cli/tests/test_repo_change_report.rs index 54a566e748..333b769320 100644 --- a/cli/tests/test_repo_change_report.rs +++ b/cli/tests/test_repo_change_report.rs @@ -43,6 +43,8 @@ fn test_report_conflicts() { Working copy now at: zsuskuln 7dc9bf15 (conflict) (empty) (no description set) Parent commit : kkmpptxz 9baab11e (conflict) C Added 0 files, modified 1 files, removed 0 files + There are unresolved conflicts at these paths: + file 2-sided conflict including 1 deletion "###); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-d=description(A)"]); @@ -75,6 +77,8 @@ fn test_report_conflicts() { Working copy now at: zsuskuln 83074dac (conflict) (empty) (no description set) Parent commit : kkmpptxz 4f0eeaa6 (conflict) C Added 0 files, modified 1 files, removed 0 files + There are unresolved conflicts at these paths: + file 2-sided conflict "###); // Resolve one of the conflicts by (mostly) following the instructions @@ -84,6 +88,8 @@ fn test_report_conflicts() { Working copy now at: vruxwmqv 2ec0b4c3 (conflict) (empty) (no description set) Parent commit : rlvkpnrz e93270ab (conflict) B Added 0 files, modified 1 files, removed 0 files + There are unresolved conflicts at these paths: + file 2-sided conflict including 1 deletion "###); std::fs::write(repo_path.join("file"), "resolved\n").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash"]); @@ -129,6 +135,8 @@ fn test_report_conflicts_with_divergent_commits() { Working copy now at: zsuskuln?? cdae4322 (conflict) C2 Parent commit : kkmpptxz b76d6a88 (conflict) B Added 0 files, modified 1 files, removed 0 files + There are unresolved conflicts at these paths: + file 2-sided conflict including 1 deletion "###); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-d=description(A)"]); @@ -160,6 +168,8 @@ fn test_report_conflicts_with_divergent_commits() { Working copy now at: zsuskuln?? 33752e7e (conflict) C2 Parent commit : zzzzzzzz 00000000 (empty) (no description set) Added 0 files, modified 1 files, removed 0 files + There are unresolved conflicts at these paths: + file 2-sided conflict including 1 deletion "###); let (stdout, stderr) = diff --git a/cli/tests/test_resolve_command.rs b/cli/tests/test_resolve_command.rs index 057b76164f..4be0ba9a74 100644 --- a/cli/tests/test_resolve_command.rs +++ b/cli/tests/test_resolve_command.rs @@ -233,7 +233,7 @@ fn test_resolution() { Parent commit : zsuskuln aa493daf a | a Parent commit : royxmykx db6a4daf b | b Added 0 files, modified 1 files, removed 0 files - After this operation, some files at this revision still have conflicts: + There are unresolved conflicts at these paths: file 2-sided conflict "###); insta::assert_snapshot!( @@ -704,7 +704,7 @@ fn test_multiple_conflicts() { Parent commit : zsuskuln de7553ef a | a Parent commit : royxmykx f68bc2f0 b | b Added 0 files, modified 1 files, removed 0 files - After this operation, some files at this revision still have conflicts: + There are unresolved conflicts at these paths: this_file_has_a_very_long_name_to_test_padding 2-sided conflict "###); insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), diff --git a/cli/tests/test_restore_command.rs b/cli/tests/test_restore_command.rs index 0c8f1a2e3e..6bc86a5b62 100644 --- a/cli/tests/test_restore_command.rs +++ b/cli/tests/test_restore_command.rs @@ -71,6 +71,8 @@ fn test_restore() { Working copy now at: kkmpptxz 761deaef (conflict) (no description set) Parent commit : rlvkpnrz e25100af (empty) (no description set) Added 0 files, modified 1 files, removed 0 files + There are unresolved conflicts at these paths: + file2 2-sided conflict including 1 deletion "###); let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s", "-r=@-"]); insta::assert_snapshot!(stdout, @""); @@ -202,6 +204,8 @@ fn test_restore_conflicted_merge() { Parent commit : zsuskuln aa493daf a | a Parent commit : royxmykx db6a4daf b | b Added 0 files, modified 1 files, removed 0 files + There are unresolved conflicts at these paths: + file 2-sided conflict "###); insta::assert_snapshot!( std::fs::read_to_string(repo_path.join("file")).unwrap() @@ -241,6 +245,8 @@ fn test_restore_conflicted_merge() { Parent commit : zsuskuln aa493daf a | a Parent commit : royxmykx db6a4daf b | b Added 0 files, modified 1 files, removed 0 files + There are unresolved conflicts at these paths: + file 2-sided conflict "###); insta::assert_snapshot!( std::fs::read_to_string(repo_path.join("file")).unwrap()