From b8ba78b99607683cbbc6960d77c4d808122008fa Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Fri, 23 Dec 2022 17:57:10 -0800 Subject: [PATCH] Extract `resolve_list` from `cmd_resolve`, tests, minor refactoring This is all in preparation for follow-up commit, which will change some of the new tests. The extracted function will be further expanded later. --- src/commands.rs | 60 ++++++++++++++++++++--------------- tests/test_resolve_command.rs | 39 ++++++++++++++++------- 2 files changed, 63 insertions(+), 36 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index ebdb01342..8171be75c 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -2383,33 +2383,21 @@ fn cmd_resolve( let tree = commit.tree(); let conflicts = tree.conflicts_matching(matcher.as_ref()); if conflicts.is_empty() { - return Err(CommandError::CliError( - "No conflicts found ".to_string() - + (if args.paths.is_empty() { - "at this revision" - } else { - "at the given path(s)" - }), - )); + return Err(CommandError::CliError(format!( + "No conflicts found {}", + if args.paths.is_empty() { + "at this revision" + } else { + "at the given path(s)" + } + ))); } if args.list { - let mut formatter = ui.stdout_formatter(); - let formatter = formatter.as_mut(); - for (repo_path, _conflict_id) in conflicts { - // 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. - writeln!( - formatter, - "{}", - &workspace_command.format_file_path(&repo_path) - )?; - } - return Ok(()); + return print_conflicted_files( + &conflicts, + ui.stdout_formatter().as_mut(), + &workspace_command, + ); }; let (repo_path, _) = conflicts.get(0).unwrap(); @@ -2426,6 +2414,28 @@ fn cmd_resolve( workspace_command.finish_transaction(ui, tx) } +fn print_conflicted_files( + conflicts: &[(RepoPath, jujutsu_lib::backend::ConflictId)], + 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. + writeln!( + formatter, + "{}", + &workspace_command.format_file_path(repo_path) + )?; + } + Ok(()) +} + fn cmd_restore( ui: &mut Ui, command: &CommandHelper, diff --git a/tests/test_resolve_command.rs b/tests/test_resolve_command.rs index 9806d4b42..24de8ecf5 100644 --- a/tests/test_resolve_command.rs +++ b/tests/test_resolve_command.rs @@ -85,7 +85,11 @@ fn test_resolution() { ["dump editor0", "write\nresolution\n"].join("\0"), ) .unwrap(); - test_env.jj_cmd_success(&repo_path, &["resolve"]); + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["resolve"]), @r###" + Working copy now at: e069f0736a79 conflict + Added 0 files, modified 1 files, removed 0 files + "###); insta::assert_snapshot!( std::fs::read_to_string(test_env.env_root().join("editor0")).unwrap(), @r###" "###); @@ -167,14 +171,19 @@ conflict .join("\0"), ) .unwrap(); - test_env.jj_cmd_success( - &repo_path, - &[ - "resolve", - "--config-toml", - "merge-tools.fake-editor.merge-tool-edits-conflict-markers=true", - ], - ); + insta::assert_snapshot!( + test_env.jj_cmd_success( + &repo_path, + &[ + "resolve", + "--config-toml", + "merge-tools.fake-editor.merge-tool-edits-conflict-markers=true", + ], + ), + @r###" + Working copy now at: 0bb40c908c8b conflict + Added 0 files, modified 1 files, removed 0 files + "###); insta::assert_snapshot!( std::fs::read_to_string(test_env.env_root().join("editor2")).unwrap(), @r###" <<<<<<< @@ -225,7 +234,11 @@ conflict .join("\0"), ) .unwrap(); - test_env.jj_cmd_success(&repo_path, &["resolve"]); + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["resolve"]), @r###" + Working copy now at: 95418cb82ab1 conflict + Added 0 files, modified 1 files, removed 0 files + "###); insta::assert_snapshot!( std::fs::read_to_string(test_env.env_root().join("editor3")).unwrap(), @r###" "###); @@ -520,7 +533,11 @@ fn test_multiple_conflicts() { // Check that we can manually pick which of the conflicts to resolve first std::fs::write(&editor_script, "expect\n\0write\nresolution file2\n").unwrap(); - test_env.jj_cmd_success(&repo_path, &["resolve", "file2"]); + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["resolve", "file2"]), @r###" + Working copy now at: 06cafc2b5489 conflict + Added 0 files, modified 1 files, removed 0 files + "###); insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]), @r###" Resolved conflict in file2: