diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 099b9f5628..a55c4e6877 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -247,7 +247,7 @@ impl From for CommandError { impl From for CommandError { fn from(err: ConflictResolveError) -> Self { - user_error(format!("Failed to use external tool to resolve: {err}")) + user_error(format!("Failed to resolve conflicts: {err}")) } } diff --git a/cli/src/merge_tools/internal.rs b/cli/src/merge_tools/internal.rs index bb5258f206..30ff22f7bb 100644 --- a/cli/src/merge_tools/internal.rs +++ b/cli/src/merge_tools/internal.rs @@ -5,7 +5,9 @@ use std::sync::Arc; use itertools::Itertools; use jj_lib::backend::{BackendError, FileId, ObjectId, SymlinkId, TreeId, TreeValue}; use jj_lib::diff::{find_line_ranges, Diff, DiffHunk}; +use jj_lib::files::{self, ContentHunk, MergeResult}; use jj_lib::matchers::EverythingMatcher; +use jj_lib::merge::Merge; use jj_lib::repo_path::RepoPath; use jj_lib::store::Store; use jj_lib::tree::Tree; @@ -452,8 +454,133 @@ pub fn edit_diff_internal( Ok(tree_id) } +fn make_merge_sections( + merge_result: MergeResult, + file_merge: Merge>, +) -> Result>, InternalToolError> { + let mut sections = Vec::new(); + match merge_result { + MergeResult::Resolved(ContentHunk(buf)) => { + let right_id = file_merge.adds().get(1).and_then(|side| side.clone()); + let contents = buf_to_file_contents(right_id.map(|id| id.hex()), buf); + let section = match contents { + FileContents::Absent => None, + FileContents::Text { + contents, + hash: _, + num_bytes: _, + } => Some(scm_record::Section::Unchanged { + lines: contents + .split_inclusive('\n') + .map(|line| Cow::Owned(line.to_owned())) + .collect(), + }), + FileContents::Binary { hash, num_bytes } => Some(scm_record::Section::Binary { + is_checked: false, + old_description: None, + new_description: Some(Cow::Owned(describe_binary(hash.as_deref(), num_bytes))), + }), + }; + if let Some(section) = section { + sections.push(section); + } + } + MergeResult::Conflict(hunks) => { + for hunk in hunks { + let (removes, adds) = hunk.take(); + sections.push(match adds.as_slice() { + [ContentHunk(contents)] => { + let contents = std::str::from_utf8(contents).map_err(|err| { + InternalToolError::DecodeUtf8 { + source: err, + item: "unchanged hunk", + } + })?; + scm_record::Section::Unchanged { + lines: contents + .split_inclusive('\n') + .map(|line| Cow::Owned(line.to_owned())) + .collect(), + } + } + _ => { + let lines: Vec = adds + .into_iter() + .interleave(removes) + .zip( + [ + scm_record::ChangeType::Added, + scm_record::ChangeType::Removed, + ] + .into_iter() + .cycle(), + ) + .map(|(contents, change_type)| -> Result<_, InternalToolError> { + let ContentHunk(contents) = contents; + let contents = std::str::from_utf8(&contents).map_err(|err| { + InternalToolError::DecodeUtf8 { + source: err, + item: "conflicting hunk", + } + })?; + let changed_lines = + make_section_changed_lines(contents, change_type); + Ok(changed_lines) + }) + .flatten_ok() + .try_collect()?; + scm_record::Section::Changed { lines } + } + }); + } + } + } + Ok(sections) +} + +pub fn edit_merge_internal( + tree: &Tree, + path: &RepoPath, + file_merge: Merge>, + content: Merge, +) -> Result { + let (removes, adds) = content.take(); + let removed_slices = removes + .iter() + .map(|ContentHunk(v)| v.as_slice()) + .collect_vec(); + let added_slices = adds.iter().map(|ContentHunk(v)| v.as_slice()).collect_vec(); + let merge_result = files::merge(&removed_slices, &added_slices); + + let sections = make_merge_sections(merge_result, file_merge)?; + let recorder = scm_record::Recorder::new( + scm_record::RecordState { + is_read_only: false, + files: vec![scm_record::File { + old_path: None, + path: Cow::Owned(path.to_fs_path(Path::new(""))), + file_mode: None, + sections, + }], + }, + scm_record::EventSource::Crossterm, + ); + let state = recorder.run()?; + + let file = state.files.into_iter().exactly_one().unwrap(); + apply_diff_internal( + tree.store().clone(), + tree, + tree, + vec![path.clone()], + &[file], + ) + .map_err(InternalToolError::BackendError) +} + #[cfg(test)] mod tests { + use jj_lib::conflicts::extract_as_single_hunk; use jj_lib::matchers::EverythingMatcher; use jj_lib::repo::Repo; use testutils::TestRepo; @@ -607,4 +734,105 @@ mod tests { all_changes_tree.diff_summary(&right_tree, &EverythingMatcher) ); } + + #[test] + fn test_make_merge_sections() { + let test_repo = TestRepo::init(false); + let store = test_repo.repo.store(); + + let path = RepoPath::from_internal_string("file"); + let base_tree = testutils::create_tree( + &test_repo.repo, + &[(&path, "base 1\nbase 2\nbase 3\nbase 4\nbase 5\n")], + ); + let left_tree = testutils::create_tree( + &test_repo.repo, + &[(&path, "left 1\nbase 2\nbase 3\nbase 4\nleft 5\n")], + ); + let right_tree = testutils::create_tree( + &test_repo.repo, + &[(&path, "right 1\nbase 2\nbase 3\nbase 4\nright 5\n")], + ); + + fn file_id(tree_value: TreeValue) -> FileId { + match tree_value { + TreeValue::File { id, executable: _ } => id, + other @ (TreeValue::Symlink(_) + | TreeValue::Tree(_) + | TreeValue::GitSubmodule(_) + | TreeValue::Conflict(_)) => { + panic!("tree value should have been a FileId: {other:?}") + } + } + } + let merge = Merge::new( + vec![base_tree.path_value(&path).map(file_id)], + vec![ + left_tree.path_value(&path).map(file_id), + right_tree.path_value(&path).map(file_id), + ], + ); + let content = extract_as_single_hunk(&merge, store, &path); + let removes = content + .removes() + .iter() + .map(|ContentHunk(buf)| buf.as_slice()) + .collect_vec(); + let adds = content + .adds() + .iter() + .map(|ContentHunk(buf)| buf.as_slice()) + .collect_vec(); + let merge_result = files::merge(&removes, &adds); + let sections = make_merge_sections(merge_result, merge).unwrap(); + insta::assert_debug_snapshot!(sections, @r###" + [ + Changed { + lines: [ + SectionChangedLine { + is_checked: false, + change_type: Added, + line: "left 1\n", + }, + SectionChangedLine { + is_checked: false, + change_type: Removed, + line: "base 1\n", + }, + SectionChangedLine { + is_checked: false, + change_type: Added, + line: "right 1\n", + }, + ], + }, + Unchanged { + lines: [ + "base 2\n", + "base 3\n", + "base 4\n", + ], + }, + Changed { + lines: [ + SectionChangedLine { + is_checked: false, + change_type: Added, + line: "left 5\n", + }, + SectionChangedLine { + is_checked: false, + change_type: Removed, + line: "base 5\n", + }, + SectionChangedLine { + is_checked: false, + change_type: Added, + line: "right 5\n", + }, + ], + }, + ] + "###); + } } diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index eaef0f7a4e..ab05351e9e 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -29,7 +29,7 @@ use thiserror::Error; use self::external::{edit_diff_external, DiffCheckoutError, ExternalToolError}; pub use self::external::{generate_diff, ExternalMergeTool}; -use self::internal::{edit_diff_internal, InternalToolError}; +use self::internal::{edit_diff_internal, edit_merge_internal, InternalToolError}; use crate::config::CommandNameAndArgs; use crate::ui::Ui; @@ -114,7 +114,11 @@ pub fn run_mergetool( let editor = get_merge_tool_from_settings(ui, settings)?; match editor { - MergeTool::Internal => unimplemented!("run_mergetool with internal mergetool"), + MergeTool::Internal => { + let tree_id = + edit_merge_internal(tree, repo_path, file_merge, content).map_err(Box::new)?; + Ok(tree_id) + } MergeTool::External(editor) => external::run_mergetool_external( &editor, file_merge, content, repo_path, conflict, tree, ), diff --git a/cli/tests/test_resolve_command.rs b/cli/tests/test_resolve_command.rs index ab6722680a..c02db17022 100644 --- a/cli/tests/test_resolve_command.rs +++ b/cli/tests/test_resolve_command.rs @@ -288,8 +288,8 @@ fn check_resolve_produces_input_file( // in the future. See also https://github.com/mitsuhiko/insta/issues/313. assert_eq!( &test_env.jj_cmd_failure(repo_path, &["resolve", "--config-toml", &merge_arg_config]), - "Error: Failed to use external tool to resolve: The output file is either unchanged or \ - empty after the editor quit (run with --verbose to see the exact invocation).\n" + "Error: Failed to resolve conflicts: The output file is either unchanged or empty after \ + the editor quit (run with --verbose to see the exact invocation).\n" ); } @@ -397,7 +397,7 @@ fn test_too_many_parents() { let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]); insta::assert_snapshot!(error, @r###" - Error: Failed to use external tool to resolve: The conflict at "file" has 3 sides. At most 2 sides are supported. + Error: Failed to resolve conflicts: The conflict at "file" has 3 sides. At most 2 sides are supported. "###); } @@ -472,7 +472,7 @@ fn test_file_vs_dir() { "###); 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": + Error: Failed to resolve conflicts: Only conflicts that involve normal files (not symlinks, not executable, etc.) are supported. Conflict summary for "file": Conflict: Removing file with id df967b96a579e45a18b8251732d16804b2e56a55 Adding file with id 78981922613b2afb6025042ff6bd878ac1994e85 @@ -526,7 +526,7 @@ fn test_description_with_dir_and_deletion() { "###); 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": + Error: Failed to resolve conflicts: 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