Skip to content

Commit

Permalink
lib conflicts: have parse_conflict return a single hunk, rename to
Browse files Browse the repository at this point in the history 
`parse_merge_result`

Now, it matches `materialize_merge_result`
  • Loading branch information
@ilyagr
ilyagr committed Oct 13, 2024
1 parent 8c6129f commit b2f8249
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 115 deletions.
47 changes: 28 additions & 19 deletions lib/src/conflicts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,29 @@ pub fn materialized_diff_stream<'a>(
/// they don't have the expected arity.
// TODO: "parse" is not usually the opposite of "materialize", so maybe we
// should rename them to "serialize" and "deserialize"?
pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option<Vec<Merge<BString>>> {
pub fn parse_merge_result(input: &[u8], num_sides: usize) -> Option<Merge<BString>> {
let hunks = parse_conflict_into_list_of_hunks(input, num_sides)?;
let mut result: Merge<BString> = Merge::from_vec(vec![vec![].into(); num_sides * 2 - 1]);

for hunk in hunks {
if let Some(slice) = hunk.as_resolved() {
for content in result.iter_mut() {
content.extend_from_slice(slice);
}
} else {
for (content, slice) in zip(result.iter_mut(), hunk.into_iter()) {
content.extend(Vec::from(slice));
}
}
}

Some(result)
}

fn parse_conflict_into_list_of_hunks(
input: &[u8],
num_sides: usize,
) -> Option<Vec<Merge<BString>>> {
if input.is_empty() {
return None;
}
Expand Down Expand Up @@ -513,33 +535,20 @@ pub async fn update_from_content(
// conflicts initially. If unsuccessful, attempt to parse conflicts from with
// the arity of the unsimplified conflicts since such a conflict may be
// present in the working copy if written by an earlier version of jj.
let (used_file_ids, hunks) = 'hunks: {
if let Some(hunks) = parse_conflict(content, simplified_file_ids.num_sides()) {
break 'hunks (simplified_file_ids, hunks);
let (used_file_ids, contents) = 'hunks: {
if let Some(contents) = parse_merge_result(content, simplified_file_ids.num_sides()) {
break 'hunks (simplified_file_ids, contents);
};
if simplified_file_ids.num_sides() != file_ids.num_sides() {
if let Some(hunks) = parse_conflict(content, file_ids.num_sides()) {
break 'hunks (file_ids, hunks);
if let Some(contents) = parse_merge_result(content, file_ids.num_sides()) {
break 'hunks (file_ids, contents);
};
};
// Either there are no markers or they don't have the expected arity
let file_id = store.write_file(path, &mut &content[..]).await?;
return Ok(Merge::normal(file_id));
};

let mut contents = used_file_ids.map(|_| vec![]);
for hunk in hunks {
if let Some(slice) = hunk.as_resolved() {
for content in contents.iter_mut() {
content.extend_from_slice(slice);
}
} else {
for (content, slice) in zip(contents.iter_mut(), hunk.into_iter()) {
content.extend(Vec::from(slice));
}
}
}

// If the user edited the empty placeholder for an absent side, we consider the
// conflict resolved.
if zip(contents.iter(), used_file_ids.iter())
Expand Down
148 changes: 52 additions & 96 deletions lib/tests/test_conflicts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use indoc::indoc;
use jj_lib::backend::FileId;
use jj_lib::conflicts::extract_as_single_hunk;
use jj_lib::conflicts::materialize_merge_result;
use jj_lib::conflicts::parse_conflict;
use jj_lib::conflicts::parse_merge_result;
use jj_lib::conflicts::update_from_content;
use jj_lib::merge::Merge;
use jj_lib::repo::Repo;
Expand Down Expand Up @@ -313,28 +313,16 @@ fn test_materialize_parse_roundtrip() {

// The first add should always be from the left side
insta::assert_debug_snapshot!(
parse_conflict(materialized.as_bytes(), conflict.num_sides()),
parse_merge_result(materialized.as_bytes(), conflict.num_sides()),
@r###"
Some(
[
Conflicted(
[
"line 1 left\nline 2 left\n",
"line 1\nline 2\n",
"line 1 right\nline 2\n",
],
),
Resolved(
"line 3\n",
),
Conflicted(
[
"line 4\nline 5 left\n",
"line 4\nline 5\n",
"line 4 right\nline 5 right\n",
],
),
],
Conflicted(
[
"line 1 left\nline 2 left\nline 3\nline 4\nline 5 left\n",
"line 1\nline 2\nline 3\nline 4\nline 5\n",
"line 1 right\nline 2\nline 3\nline 4 right\nline 5 right\n",
],
),
)
"###);
}
Expand Down Expand Up @@ -363,7 +351,7 @@ fn test_materialize_conflict_no_newlines_at_eof() {
"###
);
// BUG(#3968): These conflict markers cannot be parsed
insta::assert_debug_snapshot!(parse_conflict(
insta::assert_debug_snapshot!(parse_merge_result(
materialized.as_bytes(),
conflict.num_sides()
),@"None");
Expand Down Expand Up @@ -524,7 +512,7 @@ fn test_materialize_conflict_two_forward_diffs() {
#[test]
fn test_parse_conflict_resolved() {
assert_eq!(
parse_conflict(
parse_merge_result(
indoc! {b"
line 1
line 2
Expand All @@ -541,7 +529,7 @@ line 5
#[test]
fn test_parse_conflict_simple() {
insta::assert_debug_snapshot!(
parse_conflict(indoc! {b"
parse_merge_result(indoc! {b"
line 1
<<<<<<<
%%%%%%%
Expand All @@ -558,26 +546,18 @@ fn test_parse_conflict_simple() {
),
@r###"
Some(
[
Resolved(
"line 1\n",
),
Conflicted(
[
"line 2\nleft\nline 4\n",
"line 2\nline 3\nline 4\n",
"right\n",
],
),
Resolved(
"line 5\n",
),
],
Conflicted(
[
"line 1\nline 2\nleft\nline 4\nline 5\n",
"line 1\nline 2\nline 3\nline 4\nline 5\n",
"line 1\nright\nline 5\n",
],
),
)
"###
);
insta::assert_debug_snapshot!(
parse_conflict(indoc! {b"
parse_merge_result(indoc! {b"
line 1
<<<<<<< Text
%%%%%%% Different text
Expand All @@ -594,28 +574,20 @@ fn test_parse_conflict_simple() {
),
@r###"
Some(
[
Resolved(
"line 1\n",
),
Conflicted(
[
"line 2\nleft\nline 4\n",
"line 2\nline 3\nline 4\n",
"right\n",
],
),
Resolved(
"line 5\n",
),
],
Conflicted(
[
"line 1\nline 2\nleft\nline 4\nline 5\n",
"line 1\nline 2\nline 3\nline 4\nline 5\n",
"line 1\nright\nline 5\n",
],
),
)
"###
);
// The conflict markers are too long and shouldn't parse (though we may
// decide to change this in the future)
insta::assert_debug_snapshot!(
parse_conflict(indoc! {b"
parse_merge_result(indoc! {b"
line 1
<<<<<<<<<<<
%%%%%%%%%%%
Expand All @@ -637,7 +609,7 @@ fn test_parse_conflict_simple() {
#[test]
fn test_parse_conflict_multi_way() {
insta::assert_debug_snapshot!(
parse_conflict(
parse_merge_result(
indoc! {b"
line 1
<<<<<<<
Expand All @@ -660,28 +632,20 @@ fn test_parse_conflict_multi_way() {
),
@r###"
Some(
[
Resolved(
"line 1\n",
),
Conflicted(
[
"line 2\nleft\nline 4\n",
"line 2\nline 3\nline 4\n",
"right\n",
"line 2\nline 3\nline 4\n",
"line 2\nforward\nline 3\nline 4\n",
],
),
Resolved(
"line 5\n",
),
],
Conflicted(
[
"line 1\nline 2\nleft\nline 4\nline 5\n",
"line 1\nline 2\nline 3\nline 4\nline 5\n",
"line 1\nright\nline 5\n",
"line 1\nline 2\nline 3\nline 4\nline 5\n",
"line 1\nline 2\nforward\nline 3\nline 4\nline 5\n",
],
),
)
"###
);
insta::assert_debug_snapshot!(
parse_conflict(indoc! {b"
parse_merge_result(indoc! {b"
line 1
<<<<<<< Random text
%%%%%%% Random text
Expand All @@ -703,23 +667,15 @@ fn test_parse_conflict_multi_way() {
),
@r###"
Some(
[
Resolved(
"line 1\n",
),
Conflicted(
[
"line 2\nleft\nline 4\n",
"line 2\nline 3\nline 4\n",
"right\n",
"line 2\nline 3\nline 4\n",
"line 2\nforward\nline 3\nline 4\n",
],
),
Resolved(
"line 5\n",
),
],
Conflicted(
[
"line 1\nline 2\nleft\nline 4\nline 5\n",
"line 1\nline 2\nline 3\nline 4\nline 5\n",
"line 1\nright\nline 5\n",
"line 1\nline 2\nline 3\nline 4\nline 5\n",
"line 1\nline 2\nforward\nline 3\nline 4\nline 5\n",
],
),
)
"###
);
Expand All @@ -729,7 +685,7 @@ fn test_parse_conflict_multi_way() {
fn test_parse_conflict_wrong_arity() {
// Valid conflict marker but it has fewer sides than the caller expected
assert_eq!(
parse_conflict(
parse_merge_result(
indoc! {b"
line 1
<<<<<<<
Expand All @@ -753,7 +709,7 @@ fn test_parse_conflict_wrong_arity() {
fn test_parse_conflict_malformed_missing_removes() {
// Right number of adds but missing removes
assert_eq!(
parse_conflict(
parse_merge_result(
indoc! {b"
line 1
<<<<<<<
Expand All @@ -774,7 +730,7 @@ fn test_parse_conflict_malformed_missing_removes() {
fn test_parse_conflict_malformed_marker() {
// The conflict marker is missing `%%%%%%%`
assert_eq!(
parse_conflict(
parse_merge_result(
indoc! {b"
line 1
<<<<<<<
Expand All @@ -797,7 +753,7 @@ fn test_parse_conflict_malformed_marker() {
fn test_parse_conflict_malformed_diff() {
// The diff part is invalid (missing space before "line 4")
assert_eq!(
parse_conflict(
parse_merge_result(
indoc! {b"
line 1
<<<<<<<
Expand Down

0 comments on commit b2f8249

Please sign in to comment.