ok/jj
1
0
Fork 0
forked from mirrors/jj

merge_tools: create builtin merge editor

This commit is contained in:
Waleed Khan 2023-08-29 23:13:35 +02:00
parent f2f5ded5f0
commit 393b035498
4 changed files with 216 additions and 8 deletions

View file

@ -259,7 +259,7 @@ impl From<DiffGenerateError> for CommandError {
impl From<ConflictResolveError> 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}"))
}
}

View file

@ -5,6 +5,7 @@ use std::sync::Arc;
use itertools::Itertools;
use jj_lib::backend::{BackendError, FileId, MergedTreeId, ObjectId, 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::merged_tree::{MergedTree, MergedTreeBuilder};
@ -424,8 +425,123 @@ pub fn edit_diff_builtin(
Ok(tree_id)
}
fn make_merge_sections(
merge_result: MergeResult,
) -> Result<Vec<scm_record::Section<'static>>, BuiltinToolError> {
let mut sections = Vec::new();
match merge_result {
MergeResult::Resolved(ContentHunk(buf)) => {
let contents = buf_to_file_contents(None, 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 section = match hunk.into_resolved() {
Ok(ContentHunk(contents)) => {
let contents = std::str::from_utf8(&contents).map_err(|err| {
BuiltinToolError::DecodeUtf8 {
source: err,
item: "unchanged hunk",
}
})?;
scm_record::Section::Unchanged {
lines: contents
.split_inclusive('\n')
.map(|line| Cow::Owned(line.to_owned()))
.collect(),
}
}
Err(merge) => {
let lines: Vec<scm_record::SectionChangedLine> = merge
.iter()
.zip(
[
scm_record::ChangeType::Added,
scm_record::ChangeType::Removed,
]
.into_iter()
.cycle(),
)
.map(|(contents, change_type)| -> Result<_, BuiltinToolError> {
let ContentHunk(contents) = contents;
let contents = std::str::from_utf8(contents).map_err(|err| {
BuiltinToolError::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 }
}
};
sections.push(section);
}
}
}
Ok(sections)
}
pub fn edit_merge_builtin(
tree: &MergedTree,
path: &RepoPath,
content: Merge<ContentHunk>,
) -> Result<MergedTreeId, BuiltinToolError> {
let slices = content.map(|ContentHunk(v)| v.as_slice());
let merge_result = files::merge(slices.removes(), slices.adds());
let sections = make_merge_sections(merge_result)?;
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_builtin(
tree.store().clone(),
tree,
tree,
vec![path.clone()],
&[file],
)
.map_err(BuiltinToolError::BackendError)
}
#[cfg(test)]
mod tests {
use jj_lib::conflicts::extract_as_single_hunk;
use jj_lib::repo::Repo;
use testutils::TestRepo;
@ -572,4 +688,93 @@ mod tests {
"all-changes tree was different",
);
}
#[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 to_file_id(tree_value: Merge<Option<TreeValue>>) -> Option<FileId> {
match tree_value.into_resolved() {
Ok(Some(TreeValue::File { id, executable: _ })) => Some(id.clone()),
other => {
panic!("merge should have been a FileId: {other:?}")
}
}
}
let merge = Merge::new(
vec![to_file_id(base_tree.path_value(&path))],
vec![
to_file_id(left_tree.path_value(&path)),
to_file_id(right_tree.path_value(&path)),
],
);
let content = extract_as_single_hunk(&merge, store, &path);
let slices = content.map(|ContentHunk(buf)| buf.as_slice());
let merge_result = files::merge(slices.removes(), slices.adds());
let sections = make_merge_sections(merge_result).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",
},
],
},
]
"###);
}
}

View file

@ -27,7 +27,7 @@ use jj_lib::settings::{ConfigResultExt as _, UserSettings};
use jj_lib::working_copy::SnapshotError;
use thiserror::Error;
use self::builtin::{edit_diff_builtin, BuiltinToolError};
use self::builtin::{edit_diff_builtin, edit_merge_builtin, BuiltinToolError};
use self::external::{edit_diff_external, DiffCheckoutError, ExternalToolError};
pub use self::external::{generate_diff, ExternalMergeTool};
use crate::config::CommandNameAndArgs;
@ -113,7 +113,10 @@ pub fn run_mergetool(
let editor = get_merge_tool_from_settings(ui, settings)?;
match editor {
MergeTool::Builtin => unimplemented!("run_mergetool with builtin mergetool"),
MergeTool::Builtin => {
let tree_id = edit_merge_builtin(tree, repo_path, content).map_err(Box::new)?;
Ok(tree_id)
}
MergeTool::External(editor) => external::run_mergetool_external(
&editor, file_merge, content, repo_path, conflict, tree,
),

View file

@ -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