Skip to content

Commit

Permalink
merge_tools: create builtin merge editor
Browse files Browse the repository at this point in the history
  • Loading branch information
arxanas committed Aug 29, 2023
1 parent 82f2e6b commit f5426d2
Show file tree
Hide file tree
Showing 4 changed files with 236 additions and 8 deletions.
2 changes: 1 addition & 1 deletion cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,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}"))
}
}

Expand Down
224 changes: 224 additions & 0 deletions cli/src/merge_tools/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -424,8 +425,133 @@ pub fn edit_diff_builtin(
Ok(tree_id)
}

fn make_merge_sections(
merge_result: MergeResult,
file_merge: Merge<Option<FileId>>,
) -> Result<Vec<scm_record::Section<'static>>, BuiltinToolError> {
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| {
BuiltinToolError::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<scm_record::SectionChangedLine> = adds
.into_iter()
.interleave(removes)
.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 }
}
});
}
}
}
Ok(sections)
}

pub fn edit_merge_builtin(
tree: &MergedTree,
path: &RepoPath,
file_merge: Merge<Option<FileId>>,
content: Merge<ContentHunk>,
) -> Result<MergedTreeId, BuiltinToolError> {
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_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;

Expand Down Expand Up @@ -572,4 +698,102 @@ 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 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",
},
],
},
]
"###);
}
}
8 changes: 6 additions & 2 deletions cli/src/merge_tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -113,7 +113,11 @@ 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, 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,
),
Expand Down
10 changes: 5 additions & 5 deletions cli/tests/test_resolve_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}

Expand Down Expand Up @@ -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.
"###);
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit f5426d2

Please sign in to comment.