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 Jul 15, 2024
1 parent 976aca2 commit 0d89645
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 127 deletions.
75 changes: 43 additions & 32 deletions lib/src/conflicts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,29 @@ fn diff_size(hunks: &[DiffHunk]) -> usize {
/// invalid if 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<ContentHunk>>> {
pub fn parse_merge_result(input: &[u8], num_sides: usize) -> Option<Merge<ContentHunk>> {
let hunks = parse_conflict_into_list_of_hunks(input, num_sides)?;
let mut result: Merge<Vec<u8>> = Merge::from_vec(vec![vec![]; 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.0);
}
} else {
for (content, slice) in zip(result.iter_mut(), hunk.into_iter()) {
content.extend(slice.0);
}
}
}

Some(result.into_map(ContentHunk))
}

fn parse_conflict_into_list_of_hunks(
input: &[u8],
num_sides: usize,
) -> Option<Vec<Merge<ContentHunk>>> {
if input.is_empty() {
return None;
}
Expand Down Expand Up @@ -464,37 +486,24 @@ 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[..])?;
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.0);
}
} else {
for (content, slice) in zip(contents.iter_mut(), hunk.into_iter()) {
content.extend(slice.0);
}
}
}

// If the user edited the empty placeholder for an absent side, we consider the
// conflict resolved.
if zip(contents.iter(), used_file_ids.iter())
.any(|(content, file_id)| file_id.is_none() && !content.is_empty())
.any(|(ContentHunk(content), file_id)| file_id.is_none() && !content.is_empty())
{
let file_id = store.write_file(path, &mut &content[..])?;
return Ok(Merge::normal(file_id));
Expand All @@ -503,19 +512,21 @@ pub async fn update_from_content(
// Now write the new files contents we found by parsing the file with conflict
// markers.
let new_file_ids: Vec<Option<FileId>> = zip(contents.iter(), used_file_ids.iter())
.map(|(content, file_id)| -> BackendResult<Option<FileId>> {
match file_id {
Some(_) => {
let file_id = store.write_file(path, &mut content.as_slice())?;
Ok(Some(file_id))
}
None => {
// The missing side of a conflict is still represented by
// the empty string we materialized it as
Ok(None)
.map(
|(ContentHunk(content), file_id)| -> BackendResult<Option<FileId>> {
match file_id {
Some(_) => {
let file_id = store.write_file(path, &mut content.as_slice())?;
Ok(Some(file_id))
}
None => {
// The missing side of a conflict is still represented by
// the empty string we materialized it as
Ok(None)
}
}
}
})
},
)
.try_collect()?;

// If the conflict was simplified, expand the conflict to the original
Expand Down
146 changes: 51 additions & 95 deletions lib/tests/test_conflicts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use indoc::indoc;
use jj_lib::backend::FileId;
use jj_lib::conflicts::{
extract_as_single_hunk, materialize_merge_result, parse_conflict, update_from_content,
extract_as_single_hunk, materialize_merge_result, parse_merge_result, update_from_content,
};
use jj_lib::merge::Merge;
use jj_lib::repo::Repo;
Expand Down Expand Up @@ -312,28 +312,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 @@ -362,7 +350,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 @@ -523,7 +511,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 @@ -540,7 +528,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 @@ -557,26 +545,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 @@ -593,28 +573,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 @@ -636,7 +608,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 @@ -659,28 +631,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 @@ -702,23 +666,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 @@ -727,7 +683,7 @@ fn test_parse_conflict_multi_way() {
#[test]
fn test_parse_conflict_different_wrong_arity() {
assert_eq!(
parse_conflict(
parse_merge_result(
indoc! {b"
line 1
<<<<<<<
Expand All @@ -751,7 +707,7 @@ fn test_parse_conflict_different_wrong_arity() {
fn test_parse_conflict_malformed_marker() {
// The conflict marker is missing `%%%%%%%`
assert_eq!(
parse_conflict(
parse_merge_result(
indoc! {b"
line 1
<<<<<<<
Expand All @@ -774,7 +730,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 0d89645

Please sign in to comment.