ok/jj
1
0
Fork 0
forked from mirrors/jj

files: add a newtype around Vec<u8> for content hunks

It's useful to have a more readable `Debug` format for `Vec<u8>`
(`"foo"` is better than `[102, 111, 111]`). It might also make types
in function signatures and elsewhere more readable.
This commit is contained in:
Martin von Zweigbergk 2023-06-27 01:36:26 -07:00 committed by Martin von Zweigbergk
parent b3946be414
commit b1f2e80349
5 changed files with 164 additions and 175 deletions

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::{ConflictHunk, MergeHunk, MergeResult};
use crate::files::{ConflictHunk, ContentHunk, MergeHunk, MergeResult};
use crate::merge::trivial_merge;
use crate::repo_path::RepoPath;
use crate::store::Store;
@ -240,18 +240,18 @@ impl Conflict<Option<TreeValue>> {
match hunk {
MergeHunk::Resolved(slice) => {
for buf in &mut removed_content {
buf.extend_from_slice(&slice);
buf.extend_from_slice(&slice.0);
}
for buf in &mut added_content {
buf.extend_from_slice(&slice);
buf.extend_from_slice(&slice.0);
}
}
MergeHunk::Conflict(ConflictHunk { removes, adds }) => {
for (i, buf) in removes.into_iter().enumerate() {
removed_content[i].extend(buf);
removed_content[i].extend(buf.0);
}
for (i, buf) in adds.into_iter().enumerate() {
added_content[i].extend(buf);
added_content[i].extend(buf.0);
}
}
}
@ -358,7 +358,7 @@ fn describe_conflict_term(value: &TreeValue) -> String {
}
}
fn get_file_contents(store: &Store, path: &RepoPath, term: &Option<FileId>) -> Vec<u8> {
fn get_file_contents(store: &Store, path: &RepoPath, term: &Option<FileId>) -> ContentHunk {
match term {
Some(id) => {
let mut content = vec![];
@ -367,11 +367,11 @@ fn get_file_contents(store: &Store, path: &RepoPath, term: &Option<FileId>) -> V
.unwrap()
.read_to_end(&mut content)
.unwrap();
content
ContentHunk(content)
}
// If the conflict had removed the file on one side, we pretend that the file
// was empty there.
None => vec![],
None => ContentHunk(vec![]),
}
}
@ -403,18 +403,26 @@ pub fn materialize_merge_result(
single_hunk: &ConflictHunk,
output: &mut dyn Write,
) -> std::io::Result<()> {
let removed_slices = single_hunk.removes.iter().map(Vec::as_slice).collect_vec();
let added_slices = single_hunk.adds.iter().map(Vec::as_slice).collect_vec();
let removed_slices = single_hunk
.removes
.iter()
.map(|hunk| hunk.0.as_slice())
.collect_vec();
let added_slices = single_hunk
.adds
.iter()
.map(|hunk| hunk.0.as_slice())
.collect_vec();
let merge_result = files::merge(&removed_slices, &added_slices);
match merge_result {
MergeResult::Resolved(content) => {
output.write_all(&content)?;
output.write_all(&content.0)?;
}
MergeResult::Conflict(hunks) => {
for hunk in hunks {
match hunk {
MergeHunk::Resolved(content) => {
output.write_all(&content)?;
output.write_all(&content.0)?;
}
MergeHunk::Conflict(ConflictHunk { removes, adds }) => {
output.write_all(CONFLICT_START_LINE)?;
@ -426,25 +434,27 @@ pub fn materialize_merge_result(
// If we have no more positive terms, emit the remaining negative
// terms as snapshots.
output.write_all(CONFLICT_MINUS_LINE)?;
output.write_all(left)?;
output.write_all(&left.0)?;
continue;
};
let diff1 = Diff::for_tokenizer(&[left, right1], &find_line_ranges)
.hunks()
.collect_vec();
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) = adds.get(add_index + 1) {
let diff2 = Diff::for_tokenizer(&[left, right2], &find_line_ranges)
.hunks()
.collect_vec();
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)?;
output.write_all(&right1.0)?;
output.write_all(CONFLICT_DIFF_LINE)?;
write_diff_hunks(&diff2, output)?;
add_index += 2;
@ -460,7 +470,7 @@ pub fn materialize_merge_result(
// Emit the remaining positive terms as snapshots.
for slice in &adds[add_index..] {
output.write_all(CONFLICT_PLUS_LINE)?;
output.write_all(slice)?;
output.write_all(&slice.0)?;
}
output.write_all(CONFLICT_END_LINE)?;
}
@ -507,7 +517,7 @@ pub fn parse_conflict(input: &[u8], num_removes: usize, num_adds: usize) -> Opti
{
let resolved_slice = &input[resolved_start..conflict_start.unwrap()];
if !resolved_slice.is_empty() {
hunks.push(MergeHunk::Resolved(resolved_slice.to_vec()));
hunks.push(MergeHunk::Resolved(ContentHunk(resolved_slice.to_vec())));
}
hunks.push(hunk);
resolved_start = pos + line.len();
@ -523,7 +533,9 @@ pub fn parse_conflict(input: &[u8], num_removes: usize, num_adds: usize) -> Opti
None
} else {
if resolved_start < input.len() {
hunks.push(MergeHunk::Resolved(input[resolved_start..].to_vec()));
hunks.push(MergeHunk::Resolved(ContentHunk(
input[resolved_start..].to_vec(),
)));
}
Some(hunks)
}
@ -543,18 +555,18 @@ fn parse_conflict_hunk(input: &[u8]) -> MergeHunk {
match line {
CONFLICT_DIFF_LINE => {
state = State::Diff;
removes.push(vec![]);
adds.push(vec![]);
removes.push(ContentHunk(vec![]));
adds.push(ContentHunk(vec![]));
continue;
}
CONFLICT_MINUS_LINE => {
state = State::Minus;
removes.push(vec![]);
removes.push(ContentHunk(vec![]));
continue;
}
CONFLICT_PLUS_LINE => {
state = State::Plus;
adds.push(vec![]);
adds.push(ContentHunk(vec![]));
continue;
}
_ => {}
@ -562,26 +574,26 @@ fn parse_conflict_hunk(input: &[u8]) -> MergeHunk {
match state {
State::Diff => {
if let Some(rest) = line.strip_prefix(b"-") {
removes.last_mut().unwrap().extend_from_slice(rest);
removes.last_mut().unwrap().0.extend_from_slice(rest);
} else if let Some(rest) = line.strip_prefix(b"+") {
adds.last_mut().unwrap().extend_from_slice(rest);
adds.last_mut().unwrap().0.extend_from_slice(rest);
} else if let Some(rest) = line.strip_prefix(b" ") {
removes.last_mut().unwrap().extend_from_slice(rest);
adds.last_mut().unwrap().extend_from_slice(rest);
removes.last_mut().unwrap().0.extend_from_slice(rest);
adds.last_mut().unwrap().0.extend_from_slice(rest);
} else {
// Doesn't look like a conflict
return MergeHunk::Resolved(vec![]);
return MergeHunk::Resolved(ContentHunk(vec![]));
}
}
State::Minus => {
removes.last_mut().unwrap().extend_from_slice(line);
removes.last_mut().unwrap().0.extend_from_slice(line);
}
State::Plus => {
adds.last_mut().unwrap().extend_from_slice(line);
adds.last_mut().unwrap().0.extend_from_slice(line);
}
State::Unknown => {
// Doesn't look like a conflict
return MergeHunk::Resolved(vec![]);
return MergeHunk::Resolved(ContentHunk(vec![]));
}
}
}

View file

@ -143,63 +143,32 @@ impl<'a> Iterator for DiffLineIterator<'a> {
}
#[derive(PartialEq, Eq, Clone)]
pub struct ConflictHunk {
pub removes: Vec<Vec<u8>>,
pub adds: Vec<Vec<u8>>,
pub struct ContentHunk(pub Vec<u8>);
impl Debug for ContentHunk {
fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> {
String::from_utf8_lossy(&self.0).fmt(f)
}
}
#[derive(PartialEq, Eq, Clone)]
#[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(Vec<u8>),
Resolved(ContentHunk),
Conflict(ConflictHunk),
}
impl Debug for MergeHunk {
fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> {
match self {
MergeHunk::Resolved(data) => f
.debug_tuple("Resolved")
.field(&String::from_utf8_lossy(data))
.finish(),
MergeHunk::Conflict(ConflictHunk { removes, adds }) => f
.debug_struct("Conflict")
.field(
"removes",
&removes
.iter()
.map(|part| String::from_utf8_lossy(part))
.collect_vec(),
)
.field(
"adds",
&adds
.iter()
.map(|part| String::from_utf8_lossy(part))
.collect_vec(),
)
.finish(),
}
}
}
#[derive(PartialEq, Eq, Clone)]
#[derive(PartialEq, Eq, Clone, Debug)]
pub enum MergeResult {
Resolved(Vec<u8>),
Resolved(ContentHunk),
Conflict(Vec<MergeHunk>),
}
impl Debug for MergeResult {
fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> {
match self {
MergeResult::Resolved(data) => f
.debug_tuple("Resolved")
.field(&String::from_utf8_lossy(data))
.finish(),
MergeResult::Conflict(hunks) => f.debug_tuple("Conflict").field(hunks).finish(),
}
}
}
/// A region where the base and two sides match.
#[derive(Debug, PartialEq, Eq, Clone)]
struct SyncRegion {
@ -218,29 +187,29 @@ pub fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult {
diff_inputs.extend(adds);
let diff = Diff::for_tokenizer(&diff_inputs, &diff::find_line_ranges);
let mut resolved_hunk: Vec<u8> = vec![];
let mut resolved_hunk = ContentHunk(vec![]);
let mut merge_hunks: Vec<MergeHunk> = vec![];
for diff_hunk in diff.hunks() {
match diff_hunk {
DiffHunk::Matching(content) => {
resolved_hunk.extend(content);
resolved_hunk.0.extend(content);
}
DiffHunk::Different(parts) => {
if let Some(resolved) = trivial_merge(&parts[..num_diffs], &parts[num_diffs..]) {
resolved_hunk.extend(*resolved);
resolved_hunk.0.extend(*resolved);
} else {
if !resolved_hunk.is_empty() {
if !resolved_hunk.0.is_empty() {
merge_hunks.push(MergeHunk::Resolved(resolved_hunk));
resolved_hunk = vec![];
resolved_hunk = ContentHunk(vec![]);
}
merge_hunks.push(MergeHunk::Conflict(ConflictHunk {
removes: parts[..num_diffs]
.iter()
.map(|part| part.to_vec())
.map(|part| ContentHunk(part.to_vec()))
.collect_vec(),
adds: parts[num_diffs..]
.iter()
.map(|part| part.to_vec())
.map(|part| ContentHunk(part.to_vec()))
.collect_vec(),
}));
}
@ -251,7 +220,7 @@ pub fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult {
if merge_hunks.is_empty() {
MergeResult::Resolved(resolved_hunk)
} else {
if !resolved_hunk.is_empty() {
if !resolved_hunk.0.is_empty() {
merge_hunks.push(MergeHunk::Resolved(resolved_hunk));
}
MergeResult::Conflict(merge_hunks)
@ -262,114 +231,115 @@ pub fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult {
mod tests {
use super::*;
fn hunk(data: &[u8]) -> ContentHunk {
ContentHunk(data.to_vec())
}
#[test]
fn test_merge_single_hunk() {
// Unchanged and empty on all sides
assert_eq!(
merge(&[b""], &[b"", b""]),
MergeResult::Resolved(b"".to_vec())
);
assert_eq!(merge(&[b""], &[b"", b""]), MergeResult::Resolved(hunk(b"")));
// Unchanged on all sides
assert_eq!(
merge(&[b"a"], &[b"a", b"a"]),
MergeResult::Resolved(b"a".to_vec())
MergeResult::Resolved(hunk(b"a"))
);
// One side removed, one side unchanged
assert_eq!(
merge(&[b"a\n"], &[b"", b"a\n"]),
MergeResult::Resolved(b"".to_vec())
MergeResult::Resolved(hunk(b""))
);
// One side unchanged, one side removed
assert_eq!(
merge(&[b"a\n"], &[b"a\n", b""]),
MergeResult::Resolved(b"".to_vec())
MergeResult::Resolved(hunk(b""))
);
// Both sides removed same line
assert_eq!(
merge(&[b"a\n"], &[b"", b""]),
MergeResult::Resolved(b"".to_vec())
MergeResult::Resolved(hunk(b""))
);
// One side modified, one side unchanged
assert_eq!(
merge(&[b"a"], &[b"a b", b"a"]),
MergeResult::Resolved(b"a b".to_vec())
MergeResult::Resolved(hunk(b"a b"))
);
// One side unchanged, one side modified
assert_eq!(
merge(&[b"a"], &[b"a", b"a b"]),
MergeResult::Resolved(b"a b".to_vec())
MergeResult::Resolved(hunk(b"a b"))
);
// All sides added same content
assert_eq!(
merge(&[b"", b""], &[b"a\n", b"a\n", b"a\n"]),
MergeResult::Resolved(b"a\n".to_vec())
MergeResult::Resolved(hunk(b"a\n"))
);
// 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![b"a".to_vec(), b"".to_vec()],
adds: vec![b"b".to_vec(), b"b".to_vec(), b"b".to_vec()]
removes: vec![hunk(b"a"), hunk(b"")],
adds: vec![hunk(b"b"), hunk(b"b"), hunk(b"b")]
})])
);
// All sides removed same content
assert_eq!(
merge(&[b"a\n", b"a\n", b"a\n"], &[b"", b"", b"", b""]),
MergeResult::Resolved(b"".to_vec())
MergeResult::Resolved(hunk(b""))
);
// 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![b"a\n".to_vec(), b"a\n".to_vec()],
adds: vec![b"b\n".to_vec(), b"".to_vec(), b"".to_vec()]
removes: vec![hunk(b"a\n"), hunk(b"a\n")],
adds: vec![hunk(b"b\n"), hunk(b""), hunk(b"")]
})])
);
// Three sides made the same change
assert_eq!(
merge(&[b"a", b"a"], &[b"b", b"b", b"b"]),
MergeResult::Resolved(b"b".to_vec())
MergeResult::Resolved(hunk(b"b"))
);
// One side removed, one side modified
assert_eq!(
merge(&[b"a\n"], &[b"", b"b\n"]),
MergeResult::Conflict(vec![MergeHunk::Conflict(ConflictHunk {
removes: vec![b"a\n".to_vec()],
adds: vec![b"".to_vec(), b"b\n".to_vec()]
removes: vec![hunk(b"a\n")],
adds: 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![b"a\n".to_vec()],
adds: vec![b"b\n".to_vec(), b"".to_vec()]
removes: vec![hunk(b"a\n")],
adds: 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![b"a".to_vec()],
adds: vec![b"b".to_vec(), b"c".to_vec()]
removes: vec![hunk(b"a")],
adds: vec![hunk(b"b"), hunk(b"c")]
})])
);
// Two of three sides don't change, third side changes
assert_eq!(
merge(&[b"a", b"a"], &[b"a", b"", b"a"]),
MergeResult::Resolved(b"".to_vec())
MergeResult::Resolved(hunk(b""))
);
// One side unchanged, two other sides make the same change
assert_eq!(
merge(&[b"a", b"a"], &[b"b", b"a", b"b"]),
MergeResult::Resolved(b"b".to_vec())
MergeResult::Resolved(hunk(b"b"))
);
// 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![b"a".to_vec(), b"a".to_vec()],
adds: vec![b"b".to_vec(), b"a".to_vec(), b"c".to_vec()]
removes: vec![hunk(b"a"), hunk(b"a")],
adds: vec![hunk(b"b"), hunk(b"a"), hunk(b"c")]
})])
);
// Merge of an unresolved conflict and another branch, where the other branch
@ -377,22 +347,22 @@ mod tests {
// first.
assert_eq!(
merge(&[b"a", b"b"], &[b"b", b"a", b"c"]),
MergeResult::Resolved(b"c".to_vec())
MergeResult::Resolved(hunk(b"c"))
);
// 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![b"a".to_vec(), b"b".to_vec()],
adds: vec![b"c".to_vec(), b"d".to_vec(), b"e".to_vec()]
removes: vec![hunk(b"a"), hunk(b"b")],
adds: 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![b"a".to_vec(), b"b".to_vec()],
adds: vec![b"c".to_vec(), b"c".to_vec(), b"c".to_vec()]
removes: vec![hunk(b"a"), hunk(b"b")],
adds: vec![hunk(b"c"), hunk(b"c"), hunk(b"c")]
})])
);
}
@ -403,28 +373,28 @@ mod tests {
assert_eq!(
merge(&[b"a\n"], &[b"a\nb\n", b"a\nc\n"]),
MergeResult::Conflict(vec![
MergeHunk::Resolved(b"a\n".to_vec()),
MergeHunk::Resolved(hunk(b"a\n")),
MergeHunk::Conflict(ConflictHunk {
removes: vec![b"".to_vec()],
adds: vec![b"b\n".to_vec(), b"c\n".to_vec()]
removes: vec![hunk(b"")],
adds: vec![hunk(b"b\n"), hunk(b"c\n")]
})
])
);
// Two sides changed different lines: no conflict
assert_eq!(
merge(&[b"a\nb\nc\n"], &[b"a2\nb\nc\n", b"a\nb\nc2\n"]),
MergeResult::Resolved(b"a2\nb\nc2\n".to_vec())
MergeResult::Resolved(hunk(b"a2\nb\nc2\n"))
);
// Conflict with non-conflicting lines around
assert_eq!(
merge(&[b"a\nb\nc\n"], &[b"a\nb1\nc\n", b"a\nb2\nc\n"]),
MergeResult::Conflict(vec![
MergeHunk::Resolved(b"a\n".to_vec()),
MergeHunk::Resolved(hunk(b"a\n")),
MergeHunk::Conflict(ConflictHunk {
removes: vec![b"b\n".to_vec()],
adds: vec![b"b1\n".to_vec(), b"b2\n".to_vec()]
removes: vec![hunk(b"b\n")],
adds: vec![hunk(b"b1\n"), hunk(b"b2\n")]
}),
MergeHunk::Resolved(b"c\n".to_vec())
MergeHunk::Resolved(hunk(b"c\n"))
])
);
// One side changes a line and adds a block after. The other side just adds the
@ -460,7 +430,7 @@ b {
"
]
),
MergeResult::Resolved(
MergeResult::Resolved(hunk(
b"\
a {
q
@ -470,8 +440,7 @@ b {
x
}
"
.to_vec()
)
))
);
}
}

View file

@ -699,7 +699,7 @@ fn try_resolve_file_conflict(
);
match merge_result {
MergeResult::Resolved(merged_content) => {
let id = store.write_file(filename, &mut merged_content.as_slice())?;
let id = store.write_file(filename, &mut merged_content.0.as_slice())?;
Ok(Some(TreeValue::File { id, executable }))
}
MergeResult::Conflict(_) => Ok(None),

View file

@ -320,27 +320,31 @@ line 5 right
@r###"
Some(
[
Conflict {
removes: [
"line 1\nline 2\n",
],
adds: [
"line 1 left\nline 2 left\n",
"line 1 right\nline 2\n",
],
},
Conflict(
ConflictHunk {
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 {
removes: [
"line 4\nline 5\n",
],
adds: [
"line 4\nline 5 left\n",
"line 4 right\nline 5 right\n",
],
},
Conflict(
ConflictHunk {
removes: [
"line 4\nline 5\n",
],
adds: [
"line 4\nline 5 left\n",
"line 4 right\nline 5 right\n",
],
},
),
],
)
"###);
@ -488,15 +492,17 @@ line 5
Resolved(
"line 1\n",
),
Conflict {
removes: [
"line 2\nline 3\nline 4\n",
],
adds: [
"line 2\nleft\nline 4\n",
"right\n",
],
},
Conflict(
ConflictHunk {
removes: [
"line 2\nline 3\nline 4\n",
],
adds: [
"line 2\nleft\nline 4\n",
"right\n",
],
},
),
Resolved(
"line 5\n",
),
@ -536,17 +542,19 @@ line 5
Resolved(
"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(
ConflictHunk {
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",
),

View file

@ -187,9 +187,9 @@ pub fn run_mergetool(
vec![]
};
let files: HashMap<&str, _> = maplit::hashmap! {
"base" => content.removes.pop().unwrap(),
"right" => content.adds.pop().unwrap(),
"left" => content.adds.pop().unwrap(),
"base" => content.removes.pop().unwrap().0,
"right" => content.adds.pop().unwrap().0,
"left" => content.adds.pop().unwrap().0,
"output" => initial_output_content.clone(),
};