ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

conflicts: fix off-by-one error in materialize_merge_result()

Browse source 
This should fix #1304. I think the added test simulates the behavior of
multiple rebase conflicts, but I don't have expertise around this.

add_index could be replaced with a peekable iterator, but the iterator version
wouldn't be as readable as the current implementation.
This commit is contained in:
Yuya Nishihara 2023-02-24 13:04:26 +09:00
parent 0224b52ffc
commit da16bf340c
2 changed files with 136 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
lib/src/conflicts.rs
View file

@ -210,13 +210,11 @@ pub fn materialize_merge_result(
// Check if the diff against the next positive term is better. Since
// we want to preserve the order of the terms, we don't match against
// any later positive terms.
if add_index < adds.len() {
let diff2 = Diff::for_tokenizer(
&[&left, &adds[add_index + 1]],
&find_line_ranges,
)
.hunks()
.collect_vec();
if let Some(right2) = adds.get(add_index + 1) {
let diff2 =
Diff::for_tokenizer(&[&left, right2], &find_line_ranges)
.hunks()
.collect_vec();
if diff_size(&diff2) < diff_size(&diff1) {
// If the next positive term is a better match, emit
// the current positive term as a snapshot and the next

131
lib/tests/test_conflicts.rs
View file

@ -114,6 +114,137 @@ line 5
);
}
#[test]
fn test_materialize_conflict_multi_rebase_conflicts() {
let test_repo = TestRepo::init(false);
let store = test_repo.repo.store();
// Create changes (a, b, c) on top of the base, and linearize them.
let path = RepoPath::from_internal_string("file");
let base_id = testutils::write_file(
store,
&path,
"line 1
line 2 base
line 3
",
);
let a_id = testutils::write_file(
store,
&path,
"line 1
line 2 a.1
line 2 a.2
line 2 a.3
line 3
",
);
let b_id = testutils::write_file(
store,
&path,
"line 1
line 2 b.1
line 2 b.2
line 3
",
);
let c_id = testutils::write_file(
store,
&path,
"line 1
line 2 c.1
line 3
",
);
// The order of (a, b, c) should be preserved. For all cases, the "a" side
// should be a snapshot.
let conflict = Conflict {
removes: vec![file_conflict_term(&base_id), file_conflict_term(&base_id)],
adds: vec![
file_conflict_term(&a_id),
file_conflict_term(&b_id),
file_conflict_term(&c_id),
],
};
insta::assert_snapshot!(
&materialize_conflict_string(store, &path, &conflict),
@r###"
line 1
<<<<<<<
+++++++
line 2 a.1
line 2 a.2
line 2 a.3
%%%%%%%
-line 2 base
+line 2 b.1
+line 2 b.2
%%%%%%%
-line 2 base
+line 2 c.1
>>>>>>>
line 3
"###
);
let conflict = Conflict {
removes: vec![file_conflict_term(&base_id), file_conflict_term(&base_id)],
adds: vec![
file_conflict_term(&c_id),
file_conflict_term(&b_id),
file_conflict_term(&a_id),
],
};
insta::assert_snapshot!(
&materialize_conflict_string(store, &path, &conflict),
@r###"
line 1
<<<<<<<
%%%%%%%
-line 2 base
+line 2 c.1
%%%%%%%
-line 2 base
+line 2 b.1
+line 2 b.2
+++++++
line 2 a.1
line 2 a.2
line 2 a.3
>>>>>>>
line 3
"###
);
let conflict = Conflict {
removes: vec![file_conflict_term(&base_id), file_conflict_term(&base_id)],
adds: vec![
file_conflict_term(&c_id),
file_conflict_term(&a_id),
file_conflict_term(&b_id),
],
};
insta::assert_snapshot!(
&materialize_conflict_string(store, &path, &conflict),
@r###"
line 1
<<<<<<<
%%%%%%%
-line 2 base
+line 2 c.1
+++++++
line 2 a.1
line 2 a.2
line 2 a.3
%%%%%%%
-line 2 base
+line 2 b.1
+line 2 b.2
>>>>>>>
line 3
"###
);
}
#[test]
fn test_materialize_parse_roundtrip() {
let test_repo = TestRepo::init(false);