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

files: replace uses of MergeHunk by Conflict<ContentHunk>

Browse source 
Since `Conflict`s can represent the resolved state, so
`Conflict<ContentHunk>` can represent the states that we use
`MergeHunk` for. `MergeHunk` does force the user to handle the
resolved case, which may be useful. I suppose one could use the same
argument for making `Conflict` an enum, i.e. if we think that
`MergeHunk`'s two variants are beneficial, then we should consider
making `Conflict` an enum with those two variants.
This commit is contained in:
Martin von Zweigbergk 2023-06-27 12:56:44 -07:00 committed by Martin von Zweigbergk
parent c625e9352d
commit 779b8ba318
3 changed files with 173 additions and 183 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

161
lib/src/conflicts.rs
View file

@ -19,7 +19,7 @@ use itertools::Itertools;
use crate::backend::{BackendResult, FileId, ObjectId, TreeValue};
use crate::diff::{find_line_ranges, Diff, DiffHunk};
use crate::files::{ContentHunk, MergeHunk, MergeResult};
use crate::files::{ContentHunk, MergeResult};
use crate::merge::trivial_merge;
use crate::repo_path::RepoPath;
use crate::store::Store;
@ -48,6 +48,10 @@ impl<T> Conflict<T> {
Conflict { removes, adds }
}
pub fn resolved(value: T) -> Self {
Conflict::new(vec![], vec![value])
}
/// Create a `Conflict` from a `removes` and `adds`, padding with `None` to
/// make sure that there is exactly one more `adds` than `removes`.
pub fn from_legacy_form(removes: Vec<T>, adds: Vec<T>) -> Conflict<Option<T>> {
@ -242,23 +246,20 @@ impl Conflict<Option<TreeValue>> {
return Ok(None);
};
for hunk in hunks {
match hunk {
MergeHunk::Resolved(slice) => {
for buf in &mut removed_content {
buf.extend_from_slice(&slice.0);
}
for buf in &mut added_content {
buf.extend_from_slice(&slice.0);
}
if let Some(slice) = hunk.as_resolved() {
for buf in &mut removed_content {
buf.extend_from_slice(&slice.0);
}
MergeHunk::Conflict(conflict) => {
let (removes, adds) = conflict.take();
for (i, buf) in removes.into_iter().enumerate() {
removed_content[i].extend(buf.0);
}
for (i, buf) in adds.into_iter().enumerate() {
added_content[i].extend(buf.0);
}
for buf in &mut added_content {
buf.extend_from_slice(&slice.0);
}
} else {
let (removes, adds) = hunk.take();
for (i, buf) in removes.into_iter().enumerate() {
removed_content[i].extend(buf.0);
}
for (i, buf) in adds.into_iter().enumerate() {
added_content[i].extend(buf.0);
}
}
}
@ -423,60 +424,56 @@ pub fn materialize_merge_result(
}
MergeResult::Conflict(hunks) => {
for hunk in hunks {
match hunk {
MergeHunk::Resolved(content) => {
output.write_all(&content.0)?;
}
MergeHunk::Conflict(conflict) => {
output.write_all(CONFLICT_START_LINE)?;
let mut add_index = 0;
for left in conflict.removes() {
let right1 = if let Some(right1) = conflict.adds().get(add_index) {
right1
} else {
// If we have no more positive terms, emit the remaining negative
// terms as snapshots.
output.write_all(CONFLICT_MINUS_LINE)?;
output.write_all(&left.0)?;
continue;
};
let diff1 =
Diff::for_tokenizer(&[&left.0, &right1.0], &find_line_ranges)
if let Some(content) = hunk.as_resolved() {
output.write_all(&content.0)?;
} else {
output.write_all(CONFLICT_START_LINE)?;
let mut add_index = 0;
for left in hunk.removes() {
let right1 = if let Some(right1) = hunk.adds().get(add_index) {
right1
} else {
// If we have no more positive terms, emit the remaining negative
// terms as snapshots.
output.write_all(CONFLICT_MINUS_LINE)?;
output.write_all(&left.0)?;
continue;
};
let diff1 = Diff::for_tokenizer(&[&left.0, &right1.0], &find_line_ranges)
.hunks()
.collect_vec();
// 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 let Some(right2) = hunk.adds().get(add_index + 1) {
let diff2 =
Diff::for_tokenizer(&[&left.0, &right2.0], &find_line_ranges)
.hunks()
.collect_vec();
// 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 let Some(right2) = conflict.adds().get(add_index + 1) {
let diff2 =
Diff::for_tokenizer(&[&left.0, &right2.0], &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
// positive term as a diff.
output.write_all(CONFLICT_PLUS_LINE)?;
output.write_all(&right1.0)?;
output.write_all(CONFLICT_DIFF_LINE)?;
write_diff_hunks(&diff2, output)?;
add_index += 2;
continue;
}
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
// positive term as a diff.
output.write_all(CONFLICT_PLUS_LINE)?;
output.write_all(&right1.0)?;
output.write_all(CONFLICT_DIFF_LINE)?;
write_diff_hunks(&diff2, output)?;
add_index += 2;
continue;
}
output.write_all(CONFLICT_DIFF_LINE)?;
write_diff_hunks(&diff1, output)?;
add_index += 1;
}
// Emit the remaining positive terms as snapshots.
for slice in &conflict.adds()[add_index..] {
output.write_all(CONFLICT_PLUS_LINE)?;
output.write_all(&slice.0)?;
}
output.write_all(CONFLICT_END_LINE)?;
output.write_all(CONFLICT_DIFF_LINE)?;
write_diff_hunks(&diff1, output)?;
add_index += 1;
}
// Emit the remaining positive terms as snapshots.
for slice in &hunk.adds()[add_index..] {
output.write_all(CONFLICT_PLUS_LINE)?;
output.write_all(&slice.0)?;
}
output.write_all(CONFLICT_END_LINE)?;
}
}
}
@ -500,7 +497,11 @@ fn diff_size(hunks: &[DiffHunk]) -> usize {
/// will be considered 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_removes: usize, num_adds: usize) -> Option<Vec<MergeHunk>> {
pub fn parse_conflict(
input: &[u8],
num_removes: usize,
num_adds: usize,
) -> Option<Vec<Conflict<ContentHunk>>> {
if input.is_empty() {
return None;
}
@ -514,19 +515,13 @@ pub fn parse_conflict(input: &[u8], num_removes: usize, num_adds: usize) -> Opti
} else if conflict_start.is_some() && line == CONFLICT_END_LINE {
let conflict_body = &input[conflict_start.unwrap() + CONFLICT_START_LINE.len()..pos];
let hunk = parse_conflict_hunk(conflict_body);
match &hunk {
MergeHunk::Conflict(conflict)
if conflict.removes().len() == num_removes
&& conflict.adds().len() == num_adds =>
{
let resolved_slice = &input[resolved_start..conflict_start.unwrap()];
if !resolved_slice.is_empty() {
hunks.push(MergeHunk::Resolved(ContentHunk(resolved_slice.to_vec())));
}
hunks.push(hunk);
resolved_start = pos + line.len();
if hunk.removes().len() == num_removes && hunk.adds().len() == num_adds {
let resolved_slice = &input[resolved_start..conflict_start.unwrap()];
if !resolved_slice.is_empty() {
hunks.push(Conflict::resolved(ContentHunk(resolved_slice.to_vec())));
}
_ => {}
hunks.push(hunk);
resolved_start = pos + line.len();
}
conflict_start = None;
}
@ -537,7 +532,7 @@ pub fn parse_conflict(input: &[u8], num_removes: usize, num_adds: usize) -> Opti
None
} else {
if resolved_start < input.len() {
hunks.push(MergeHunk::Resolved(ContentHunk(
hunks.push(Conflict::resolved(ContentHunk(
input[resolved_start..].to_vec(),
)));
}
@ -545,7 +540,7 @@ pub fn parse_conflict(input: &[u8], num_removes: usize, num_adds: usize) -> Opti
}
}
fn parse_conflict_hunk(input: &[u8]) -> MergeHunk {
fn parse_conflict_hunk(input: &[u8]) -> Conflict<ContentHunk> {
enum State {
Diff,
Minus,
@ -586,7 +581,7 @@ fn parse_conflict_hunk(input: &[u8]) -> MergeHunk {
adds.last_mut().unwrap().0.extend_from_slice(rest);
} else {
// Doesn't look like a conflict
return MergeHunk::Resolved(ContentHunk(vec![]));
return Conflict::resolved(ContentHunk(vec![]));
}
}
State::Minus => {
@ -597,12 +592,12 @@ fn parse_conflict_hunk(input: &[u8]) -> MergeHunk {
}
State::Unknown => {
// Doesn't look like a conflict
return MergeHunk::Resolved(ContentHunk(vec![]));
return Conflict::resolved(ContentHunk(vec![]));
}
}
}
MergeHunk::Conflict(Conflict::new(removes, adds))
Conflict::new(removes, adds)
}
#[cfg(test)]

66
lib/src/files.rs
View file

@ -152,16 +152,10 @@ impl Debug for ContentHunk {
}
}
#[derive(PartialEq, Eq, Clone, Debug)]
pub enum MergeHunk {
Resolved(ContentHunk),
Conflict(Conflict<ContentHunk>),
}
#[derive(PartialEq, Eq, Clone, Debug)]
pub enum MergeResult {
Resolved(ContentHunk),
Conflict(Vec<MergeHunk>),
Conflict(Vec<Conflict<ContentHunk>>),
}
/// A region where the base and two sides match.
@ -183,7 +177,7 @@ pub fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult {
let diff = Diff::for_tokenizer(&diff_inputs, &diff::find_line_ranges);
let mut resolved_hunk = ContentHunk(vec![]);
let mut merge_hunks: Vec<MergeHunk> = vec![];
let mut merge_hunks: Vec<Conflict<ContentHunk>> = vec![];
for diff_hunk in diff.hunks() {
match diff_hunk {
DiffHunk::Matching(content) => {
@ -194,10 +188,10 @@ pub fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult {
resolved_hunk.0.extend(*resolved);
} else {
if !resolved_hunk.0.is_empty() {
merge_hunks.push(MergeHunk::Resolved(resolved_hunk));
merge_hunks.push(Conflict::resolved(resolved_hunk));
resolved_hunk = ContentHunk(vec![]);
}
merge_hunks.push(MergeHunk::Conflict(Conflict::new(
merge_hunks.push(Conflict::new(
parts[..num_diffs]
.iter()
.map(|part| ContentHunk(part.to_vec()))
@ -206,7 +200,7 @@ pub fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult {
.iter()
.map(|part| ContentHunk(part.to_vec()))
.collect_vec(),
)));
));
}
}
}
@ -216,7 +210,7 @@ pub fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult {
MergeResult::Resolved(resolved_hunk)
} else {
if !resolved_hunk.0.is_empty() {
merge_hunks.push(MergeHunk::Resolved(resolved_hunk));
merge_hunks.push(Conflict::resolved(resolved_hunk));
}
MergeResult::Conflict(merge_hunks)
}
@ -272,10 +266,10 @@ mod tests {
// One side modified, two sides added
assert_eq!(
merge(&[b"a", b""], &[b"b", b"b", b"b"]),
MergeResult::Conflict(vec![MergeHunk::Conflict(Conflict::new(
MergeResult::Conflict(vec![Conflict::new(
vec![hunk(b"a"), hunk(b"")],
vec![hunk(b"b"), hunk(b"b"), hunk(b"b")]
))])
)])
);
// All sides removed same content
assert_eq!(
@ -285,10 +279,10 @@ mod tests {
// One side modified, two sides removed
assert_eq!(
merge(&[b"a\n", b"a\n"], &[b"b\n", b"", b""]),
MergeResult::Conflict(vec![MergeHunk::Conflict(Conflict::new(
MergeResult::Conflict(vec![Conflict::new(
vec![hunk(b"a\n"), hunk(b"a\n")],
vec![hunk(b"b\n"), hunk(b""), hunk(b"")]
))])
)])
);
// Three sides made the same change
assert_eq!(
@ -298,26 +292,26 @@ mod tests {
// One side removed, one side modified
assert_eq!(
merge(&[b"a\n"], &[b"", b"b\n"]),
MergeResult::Conflict(vec![MergeHunk::Conflict(Conflict::new(
MergeResult::Conflict(vec![Conflict::new(
vec![hunk(b"a\n")],
vec![hunk(b""), hunk(b"b\n")]
))])
)])
);
// One side modified, one side removed
assert_eq!(
merge(&[b"a\n"], &[b"b\n", b""]),
MergeResult::Conflict(vec![MergeHunk::Conflict(Conflict::new(
MergeResult::Conflict(vec![Conflict::new(
vec![hunk(b"a\n")],
vec![hunk(b"b\n"), hunk(b"")]
))])
)])
);
// Two sides modified in different ways
assert_eq!(
merge(&[b"a"], &[b"b", b"c"]),
MergeResult::Conflict(vec![MergeHunk::Conflict(Conflict::new(
MergeResult::Conflict(vec![Conflict::new(
vec![hunk(b"a")],
vec![hunk(b"b"), hunk(b"c")]
))])
)])
);
// Two of three sides don't change, third side changes
assert_eq!(
@ -332,10 +326,10 @@ mod tests {
// One side unchanged, two other sides make the different change
assert_eq!(
merge(&[b"a", b"a"], &[b"b", b"a", b"c"]),
MergeResult::Conflict(vec![MergeHunk::Conflict(Conflict::new(
MergeResult::Conflict(vec![Conflict::new(
vec![hunk(b"a"), hunk(b"a")],
vec![hunk(b"b"), hunk(b"a"), hunk(b"c")]
))])
)])
);
// Merge of an unresolved conflict and another branch, where the other branch
// undid the change from one of the inputs to the unresolved conflict in the
@ -347,18 +341,18 @@ mod tests {
// Merge of an unresolved conflict and another branch.
assert_eq!(
merge(&[b"a", b"b"], &[b"c", b"d", b"e"]),
MergeResult::Conflict(vec![MergeHunk::Conflict(Conflict::new(
MergeResult::Conflict(vec![Conflict::new(
vec![hunk(b"a"), hunk(b"b")],
vec![hunk(b"c"), hunk(b"d"), hunk(b"e")]
))])
)])
);
// Two sides made the same change, third side made a different change
assert_eq!(
merge(&[b"a", b"b"], &[b"c", b"c", b"c"]),
MergeResult::Conflict(vec![MergeHunk::Conflict(Conflict::new(
MergeResult::Conflict(vec![Conflict::new(
vec![hunk(b"a"), hunk(b"b")],
vec![hunk(b"c"), hunk(b"c"), hunk(b"c")]
))])
)])
);
}
@ -368,11 +362,8 @@ mod tests {
assert_eq!(
merge(&[b"a\n"], &[b"a\nb\n", b"a\nc\n"]),
MergeResult::Conflict(vec![
MergeHunk::Resolved(hunk(b"a\n")),
MergeHunk::Conflict(Conflict::new(
vec![hunk(b"")],
vec![hunk(b"b\n"), hunk(b"c\n")]
))
Conflict::resolved(hunk(b"a\n")),
Conflict::new(vec![hunk(b"")], vec![hunk(b"b\n"), hunk(b"c\n")])
])
);
// Two sides changed different lines: no conflict
@ -384,12 +375,9 @@ mod tests {
assert_eq!(
merge(&[b"a\nb\nc\n"], &[b"a\nb1\nc\n", b"a\nb2\nc\n"]),
MergeResult::Conflict(vec![
MergeHunk::Resolved(hunk(b"a\n")),
MergeHunk::Conflict(Conflict::new(
vec![hunk(b"b\n")],
vec![hunk(b"b1\n"), hunk(b"b2\n")]
)),
MergeHunk::Resolved(hunk(b"c\n"))
Conflict::resolved(hunk(b"a\n")),
Conflict::new(vec![hunk(b"b\n")], vec![hunk(b"b1\n"), hunk(b"b2\n")]),
Conflict::resolved(hunk(b"c\n"))
])
);
// One side changes a line and adds a block after. The other side just adds the

129
lib/tests/test_conflicts.rs
View file

@ -320,31 +320,30 @@ line 5 right
@r###"
Some(
[
Conflict(
Conflict {
removes: [
"line 1\nline 2\n",
],
adds: [
"line 1 left\nline 2 left\n",
"line 1 right\nline 2\n",
],
},
),
Resolved(
"line 3\n",
),
Conflict(
Conflict {
removes: [
"line 4\nline 5\n",
],
adds: [
"line 4\nline 5 left\n",
"line 4 right\nline 5 right\n",
],
},
),
Conflict {
removes: [
"line 1\nline 2\n",
],
adds: [
"line 1 left\nline 2 left\n",
"line 1 right\nline 2\n",
],
},
Conflict {
removes: [],
adds: [
"line 3\n",
],
},
Conflict {
removes: [
"line 4\nline 5\n",
],
adds: [
"line 4\nline 5 left\n",
"line 4 right\nline 5 right\n",
],
},
],
)
"###);
@ -489,23 +488,27 @@ line 5
@r###"
Some(
[
Resolved(
"line 1\n",
),
Conflict(
Conflict {
removes: [
"line 2\nline 3\nline 4\n",
],
adds: [
"line 2\nleft\nline 4\n",
"right\n",
],
},
),
Resolved(
"line 5\n",
),
Conflict {
removes: [],
adds: [
"line 1\n",
],
},
Conflict {
removes: [
"line 2\nline 3\nline 4\n",
],
adds: [
"line 2\nleft\nline 4\n",
"right\n",
],
},
Conflict {
removes: [],
adds: [
"line 5\n",
],
},
],
)
"###
@ -539,25 +542,29 @@ line 5
@r###"
Some(
[
Resolved(
"line 1\n",
),
Conflict(
Conflict {
removes: [
"line 2\nline 3\nline 4\n",
"line 2\nline 3\nline 4\n",
],
adds: [
"line 2\nleft\nline 4\n",
"right\n",
"line 2\nforward\nline 3\nline 4\n",
],
},
),
Resolved(
"line 5\n",
),
Conflict {
removes: [],
adds: [
"line 1\n",
],
},
Conflict {
removes: [
"line 2\nline 3\nline 4\n",
"line 2\nline 3\nline 4\n",
],
adds: [
"line 2\nleft\nline 4\n",
"right\n",
"line 2\nforward\nline 3\nline 4\n",
],
},
Conflict {
removes: [],
adds: [
"line 5\n",
],
},
],
)
"###