Skip to content

Commit

Permalink
files: make MergeHunk::Conflict be a Conflict<ContentHunk>
Browse files Browse the repository at this point in the history
The `ConflictHunk` type doesn't add anything over
`Conflict<ContentHunk>`.
  • Loading branch information
martinvonz committed Jun 27, 2023
1 parent 35e4d5f commit c625e93
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 76 deletions.
31 changes: 15 additions & 16 deletions lib/src/conflicts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{ConflictHunk, ContentHunk, MergeHunk, MergeResult};
use crate::files::{ContentHunk, MergeHunk, MergeResult};
use crate::merge::trivial_merge;
use crate::repo_path::RepoPath;
use crate::store::Store;
Expand Down Expand Up @@ -251,7 +251,8 @@ impl Conflict<Option<TreeValue>> {
buf.extend_from_slice(&slice.0);
}
}
MergeHunk::Conflict(ConflictHunk { removes, adds }) => {
MergeHunk::Conflict(conflict) => {
let (removes, adds) = conflict.take();
for (i, buf) in removes.into_iter().enumerate() {
removed_content[i].extend(buf.0);
}
Expand Down Expand Up @@ -315,7 +316,7 @@ impl Conflict<Option<TreeValue>> {
}

impl Conflict<Option<FileId>> {
pub fn extract_as_single_hunk(&self, store: &Store, path: &RepoPath) -> ConflictHunk {
pub fn extract_as_single_hunk(&self, store: &Store, path: &RepoPath) -> Conflict<ContentHunk> {
let removes_content = self
.removes()
.iter()
Expand All @@ -327,10 +328,7 @@ impl Conflict<Option<FileId>> {
.map(|term| get_file_contents(store, path, term))
.collect_vec();

ConflictHunk {
removes: removes_content,
adds: adds_content,
}
Conflict::new(removes_content, adds_content)
}
}

Expand Down Expand Up @@ -405,7 +403,7 @@ fn write_diff_hunks(hunks: &[DiffHunk], file: &mut dyn Write) -> std::io::Result
}

pub fn materialize_merge_result(
single_hunk: &ConflictHunk,
single_hunk: &Conflict<ContentHunk>,
output: &mut dyn Write,
) -> std::io::Result<()> {
let removed_slices = single_hunk
Expand All @@ -429,11 +427,11 @@ pub fn materialize_merge_result(
MergeHunk::Resolved(content) => {
output.write_all(&content.0)?;
}
MergeHunk::Conflict(ConflictHunk { removes, adds }) => {
MergeHunk::Conflict(conflict) => {
output.write_all(CONFLICT_START_LINE)?;
let mut add_index = 0;
for left in &removes {
let right1 = if let Some(right1) = adds.get(add_index) {
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
Expand All @@ -449,7 +447,7 @@ 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 let Some(right2) = adds.get(add_index + 1) {
if let Some(right2) = conflict.adds().get(add_index + 1) {
let diff2 =
Diff::for_tokenizer(&[&left.0, &right2.0], &find_line_ranges)
.hunks()
Expand All @@ -473,7 +471,7 @@ pub fn materialize_merge_result(
}

// Emit the remaining positive terms as snapshots.
for slice in &adds[add_index..] {
for slice in &conflict.adds()[add_index..] {
output.write_all(CONFLICT_PLUS_LINE)?;
output.write_all(&slice.0)?;
}
Expand Down Expand Up @@ -517,8 +515,9 @@ pub fn parse_conflict(input: &[u8], num_removes: usize, num_adds: usize) -> Opti
let conflict_body = &input[conflict_start.unwrap() + CONFLICT_START_LINE.len()..pos];
let hunk = parse_conflict_hunk(conflict_body);
match &hunk {
MergeHunk::Conflict(ConflictHunk { removes, adds })
if removes.len() == num_removes && adds.len() == num_adds =>
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() {
Expand Down Expand Up @@ -603,7 +602,7 @@ fn parse_conflict_hunk(input: &[u8]) -> MergeHunk {
}
}

MergeHunk::Conflict(ConflictHunk { removes, adds })
MergeHunk::Conflict(Conflict::new(removes, adds))
}

#[cfg(test)]
Expand Down
99 changes: 47 additions & 52 deletions lib/src/files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use std::ops::Range;

use itertools::Itertools;

use crate::conflicts::Conflict;
use crate::diff;
use crate::diff::{Diff, DiffHunk};
use crate::merge::trivial_merge;
Expand Down Expand Up @@ -151,16 +152,10 @@ impl Debug for ContentHunk {
}
}

#[derive(PartialEq, Eq, Clone, Debug)]
pub struct ConflictHunk {
pub removes: Vec<ContentHunk>,
pub adds: Vec<ContentHunk>,
}

#[derive(PartialEq, Eq, Clone, Debug)]
pub enum MergeHunk {
Resolved(ContentHunk),
Conflict(ConflictHunk),
Conflict(Conflict<ContentHunk>),
}

#[derive(PartialEq, Eq, Clone, Debug)]
Expand Down Expand Up @@ -202,16 +197,16 @@ pub fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult {
merge_hunks.push(MergeHunk::Resolved(resolved_hunk));
resolved_hunk = ContentHunk(vec![]);
}
merge_hunks.push(MergeHunk::Conflict(ConflictHunk {
removes: parts[..num_diffs]
merge_hunks.push(MergeHunk::Conflict(Conflict::new(
parts[..num_diffs]
.iter()
.map(|part| ContentHunk(part.to_vec()))
.collect_vec(),
adds: parts[num_diffs..]
parts[num_diffs..]
.iter()
.map(|part| ContentHunk(part.to_vec()))
.collect_vec(),
}));
)));
}
}
}
Expand Down Expand Up @@ -277,10 +272,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(ConflictHunk {
removes: vec![hunk(b"a"), hunk(b"")],
adds: vec![hunk(b"b"), hunk(b"b"), hunk(b"b")]
})])
MergeResult::Conflict(vec![MergeHunk::Conflict(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!(
Expand All @@ -290,10 +285,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(ConflictHunk {
removes: vec![hunk(b"a\n"), hunk(b"a\n")],
adds: vec![hunk(b"b\n"), hunk(b""), hunk(b"")]
})])
MergeResult::Conflict(vec![MergeHunk::Conflict(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!(
Expand All @@ -303,26 +298,26 @@ mod tests {
// One side removed, one side modified
assert_eq!(
merge(&[b"a\n"], &[b"", b"b\n"]),
MergeResult::Conflict(vec![MergeHunk::Conflict(ConflictHunk {
removes: vec![hunk(b"a\n")],
adds: vec![hunk(b""), hunk(b"b\n")]
})])
MergeResult::Conflict(vec![MergeHunk::Conflict(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(ConflictHunk {
removes: vec![hunk(b"a\n")],
adds: vec![hunk(b"b\n"), hunk(b"")]
})])
MergeResult::Conflict(vec![MergeHunk::Conflict(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(ConflictHunk {
removes: vec![hunk(b"a")],
adds: vec![hunk(b"b"), hunk(b"c")]
})])
MergeResult::Conflict(vec![MergeHunk::Conflict(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!(
Expand All @@ -337,10 +332,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(ConflictHunk {
removes: vec![hunk(b"a"), hunk(b"a")],
adds: vec![hunk(b"b"), hunk(b"a"), hunk(b"c")]
})])
MergeResult::Conflict(vec![MergeHunk::Conflict(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
Expand All @@ -352,18 +347,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(ConflictHunk {
removes: vec![hunk(b"a"), hunk(b"b")],
adds: vec![hunk(b"c"), hunk(b"d"), hunk(b"e")]
})])
MergeResult::Conflict(vec![MergeHunk::Conflict(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(ConflictHunk {
removes: vec![hunk(b"a"), hunk(b"b")],
adds: vec![hunk(b"c"), hunk(b"c"), hunk(b"c")]
})])
MergeResult::Conflict(vec![MergeHunk::Conflict(Conflict::new(
vec![hunk(b"a"), hunk(b"b")],
vec![hunk(b"c"), hunk(b"c"), hunk(b"c")]
))])
);
}

Expand All @@ -374,10 +369,10 @@ mod tests {
merge(&[b"a\n"], &[b"a\nb\n", b"a\nc\n"]),
MergeResult::Conflict(vec![
MergeHunk::Resolved(hunk(b"a\n")),
MergeHunk::Conflict(ConflictHunk {
removes: vec![hunk(b"")],
adds: vec![hunk(b"b\n"), hunk(b"c\n")]
})
MergeHunk::Conflict(Conflict::new(
vec![hunk(b"")],
vec![hunk(b"b\n"), hunk(b"c\n")]
))
])
);
// Two sides changed different lines: no conflict
Expand All @@ -390,10 +385,10 @@ mod tests {
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(ConflictHunk {
removes: vec![hunk(b"b\n")],
adds: vec![hunk(b"b1\n"), hunk(b"b2\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"))
])
);
Expand Down Expand Up @@ -428,7 +423,7 @@ b {
x
}
"
]
],
),
MergeResult::Resolved(hunk(
b"\
Expand Down
8 changes: 4 additions & 4 deletions lib/tests/test_conflicts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ line 5 right
Some(
[
Conflict(
ConflictHunk {
Conflict {
removes: [
"line 1\nline 2\n",
],
Expand All @@ -335,7 +335,7 @@ line 5 right
"line 3\n",
),
Conflict(
ConflictHunk {
Conflict {
removes: [
"line 4\nline 5\n",
],
Expand Down Expand Up @@ -493,7 +493,7 @@ line 5
"line 1\n",
),
Conflict(
ConflictHunk {
Conflict {
removes: [
"line 2\nline 3\nline 4\n",
],
Expand Down Expand Up @@ -543,7 +543,7 @@ line 5
"line 1\n",
),
Conflict(
ConflictHunk {
Conflict {
removes: [
"line 2\nline 3\nline 4\n",
"line 2\nline 3\nline 4\n",
Expand Down
9 changes: 5 additions & 4 deletions src/merge_tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ pub fn run_mergetool(
sides: file_conflict.adds().len(),
});
};
let mut content = file_conflict.extract_as_single_hunk(tree.store(), repo_path);
let content = file_conflict.extract_as_single_hunk(tree.store(), repo_path);

let editor = get_merge_tool_from_settings(ui, settings)?;
let initial_output_content: Vec<u8> = if editor.merge_tool_edits_conflict_markers {
Expand All @@ -186,10 +186,11 @@ pub fn run_mergetool(
} else {
vec![]
};
let (mut removes, mut adds) = content.take();
let files: HashMap<&str, _> = maplit::hashmap! {
"base" => content.removes.pop().unwrap().0,
"right" => content.adds.pop().unwrap().0,
"left" => content.adds.pop().unwrap().0,
"base" => removes.pop().unwrap().0,
"right" => adds.pop().unwrap().0,
"left" => adds.pop().unwrap().0,
"output" => initial_output_content.clone(),
};

Expand Down

0 comments on commit c625e93

Please sign in to comment.