Skip to content

Commit b053149

Browse files
committed
feat: when blob-merging, clarify if something would have conflicted.
1 parent 275a0c5 commit b053149

File tree

4 files changed

+129
-20
lines changed

4 files changed

+129
-20
lines changed

gix-merge/src/blob/builtin_driver/text/function.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,17 @@ pub fn merge<'a>(
3131
current: &'a [u8],
3232
ancestor: &'a [u8],
3333
other: &'a [u8],
34-
opts: Options,
34+
Options {
35+
diff_algorithm,
36+
conflict,
37+
}: Options,
3538
) -> Resolution {
3639
out.clear();
3740
input.update_before(tokens(ancestor));
3841
input.update_after(tokens(current));
3942

4043
let hunks = imara_diff::diff(
41-
opts.diff_algorithm,
44+
diff_algorithm,
4245
input,
4346
CollectHunks {
4447
side: Side::Current,
@@ -50,7 +53,7 @@ pub fn merge<'a>(
5053
input.update_after(tokens(other));
5154

5255
let mut hunks = imara_diff::diff(
53-
opts.diff_algorithm,
56+
diff_algorithm,
5457
input,
5558
CollectHunks {
5659
side: Side::Other,
@@ -86,7 +89,7 @@ pub fn merge<'a>(
8689
fill_ancestor(&extended_range, &mut current_hunks);
8790
fill_ancestor(&extended_range, &mut intersecting);
8891
}
89-
match opts.conflict {
92+
match conflict {
9093
Conflict::Keep { style, marker_size } => {
9194
let marker_size = marker_size.get();
9295
let (hunks_front_and_back, num_hunks_front) = match style {
@@ -177,7 +180,10 @@ pub fn merge<'a>(
177180
unreachable!("initial hunks are never ancestors")
178181
}
179182
};
180-
let hunks_to_write = if opts.conflict == Conflict::ResolveWithOurs {
183+
if hunks_differ_in_diff3(ConflictStyle::Diff3, our_hunks, their_hunks, input, &current_tokens) {
184+
resolution = Resolution::CompleteWithAutoResolvedConflict;
185+
}
186+
let hunks_to_write = if conflict == Conflict::ResolveWithOurs {
181187
our_hunks
182188
} else {
183189
their_hunks
@@ -201,6 +207,9 @@ pub fn merge<'a>(
201207
unreachable!("initial hunks are never ancestors")
202208
}
203209
};
210+
if hunks_differ_in_diff3(ConflictStyle::Diff3, our_hunks, their_hunks, input, &current_tokens) {
211+
resolution = Resolution::CompleteWithAutoResolvedConflict;
212+
}
204213
let (front_hunks, back_hunks) = hunks_front_and_back.split_at(num_hunks_front);
205214
let first_hunk = first_hunk(front_hunks, our_hunks, their_hunks, back_hunks);
206215
write_ancestor(input, ancestor_integrated_until, first_hunk.before.start as usize, out);

gix-merge/src/blob/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@ pub mod platform;
1515
/// Define if a merge is conflicted or not.
1616
#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
1717
pub enum Resolution {
18-
/// Everything could be resolved during the merge.
19-
///
20-
/// Conflicts may have been resolved automatically, depending on the options.
18+
/// Everything could be resolved during the merge, and there was no conflict.
2119
Complete,
20+
/// Conflicts were resolved automatically, even thought the result is complete
21+
/// and free of conflict markers.
22+
/// This can only be the case for text-file content merges.
23+
CompleteWithAutoResolvedConflict,
2224
/// A conflict is still present in the form of conflict markers.
23-
///
24-
/// Note that this won't be the case if conflicts were automatically resolved.
2525
Conflict,
2626
}
2727

gix-merge/tests/merge/blob/builtin_driver.rs

Lines changed: 105 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ fn binary() {
2525

2626
mod text {
2727
use bstr::ByteSlice;
28-
use gix_merge::blob::builtin_driver::text::Conflict;
28+
use gix_merge::blob::builtin_driver::text::{Conflict, ConflictStyle};
2929
use gix_merge::blob::{builtin_driver, Resolution};
3030
use pretty_assertions::assert_str_eq;
3131

@@ -160,10 +160,17 @@ mod text {
160160
if is_case_diverging(&case, diverging) {
161161
num_diverging += 1;
162162
} else {
163-
let expected_resolution = if case.expected.contains_str("<<<<<<<") {
164-
Resolution::Conflict
163+
if case.expected.contains_str("<<<<<<<") {
164+
assert_eq!(actual, Resolution::Conflict, "{}: resolution mismatch", case.name,);
165165
} else {
166-
Resolution::Complete
166+
assert!(
167+
matches!(
168+
actual,
169+
Resolution::Complete | Resolution::CompleteWithAutoResolvedConflict
170+
),
171+
"{}: resolution mismatch",
172+
case.name
173+
);
167174
};
168175
assert_str_eq!(
169176
out.as_bstr().to_str_lossy(),
@@ -173,7 +180,6 @@ mod text {
173180
out.as_bstr()
174181
);
175182
assert_eq!(out.as_bstr(), case.expected);
176-
assert_eq!(actual, expected_resolution, "{}: resolution mismatch", case.name,);
177183
}
178184
}
179185

@@ -191,6 +197,100 @@ mod text {
191197
Ok(())
192198
}
193199

200+
#[test]
201+
fn both_sides_same_changes_are_conflict_free() {
202+
for conflict in [
203+
builtin_driver::text::Conflict::Keep {
204+
style: ConflictStyle::Merge,
205+
marker_size: 7.try_into().unwrap(),
206+
},
207+
builtin_driver::text::Conflict::Keep {
208+
style: ConflictStyle::Diff3,
209+
marker_size: 7.try_into().unwrap(),
210+
},
211+
builtin_driver::text::Conflict::Keep {
212+
style: ConflictStyle::ZealousDiff3,
213+
marker_size: 7.try_into().unwrap(),
214+
},
215+
builtin_driver::text::Conflict::ResolveWithOurs,
216+
builtin_driver::text::Conflict::ResolveWithTheirs,
217+
builtin_driver::text::Conflict::ResolveWithUnion,
218+
] {
219+
let options = builtin_driver::text::Options {
220+
conflict,
221+
..Default::default()
222+
};
223+
let mut input = imara_diff::intern::InternedInput::default();
224+
let mut out = Vec::new();
225+
let actual = builtin_driver::text(
226+
&mut out,
227+
&mut input,
228+
Default::default(),
229+
b"1\n3\nother",
230+
b"1\n2\n3",
231+
b"1\n3\nother",
232+
options,
233+
);
234+
assert_eq!(actual, Resolution::Complete, "{conflict:?}");
235+
}
236+
}
237+
238+
#[test]
239+
fn both_differ_partially_resolution_is_conflicting() {
240+
for (conflict, expected) in [
241+
(
242+
builtin_driver::text::Conflict::Keep {
243+
style: ConflictStyle::Merge,
244+
marker_size: 7.try_into().unwrap(),
245+
},
246+
Resolution::Conflict,
247+
),
248+
(
249+
builtin_driver::text::Conflict::Keep {
250+
style: ConflictStyle::Diff3,
251+
marker_size: 7.try_into().unwrap(),
252+
},
253+
Resolution::Conflict,
254+
),
255+
(
256+
builtin_driver::text::Conflict::Keep {
257+
style: ConflictStyle::ZealousDiff3,
258+
marker_size: 7.try_into().unwrap(),
259+
},
260+
Resolution::Conflict,
261+
),
262+
(
263+
builtin_driver::text::Conflict::ResolveWithOurs,
264+
Resolution::CompleteWithAutoResolvedConflict,
265+
),
266+
(
267+
builtin_driver::text::Conflict::ResolveWithTheirs,
268+
Resolution::CompleteWithAutoResolvedConflict,
269+
),
270+
(
271+
builtin_driver::text::Conflict::ResolveWithUnion,
272+
Resolution::CompleteWithAutoResolvedConflict,
273+
),
274+
] {
275+
let options = builtin_driver::text::Options {
276+
conflict,
277+
..Default::default()
278+
};
279+
let mut input = imara_diff::intern::InternedInput::default();
280+
let mut out = Vec::new();
281+
let actual = builtin_driver::text(
282+
&mut out,
283+
&mut input,
284+
Default::default(),
285+
b"1\n3\nours",
286+
b"1\n2\n3",
287+
b"1\n3\ntheirs",
288+
options,
289+
);
290+
assert_eq!(actual, expected, "{conflict:?}");
291+
}
292+
}
293+
194294
mod baseline {
195295
use bstr::BString;
196296
use gix_merge::blob::builtin_driver::text::{Conflict, ConflictStyle};

gix-merge/tests/merge/blob/platform.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,25 +125,25 @@ theirs
125125
let res = platform_ref.merge(&mut buf, default_labels(), &Default::default())?;
126126
assert_eq!(
127127
res,
128-
(Pick::Buffer, Resolution::Complete),
129-
"it's actually unclear now if there ever was a conflict, but we *could* compute it"
128+
(Pick::Buffer, Resolution::CompleteWithAutoResolvedConflict),
129+
"we can determine that there was a conflict, despite the resolution being complete"
130130
);
131131
assert_eq!(buf.as_bstr(), "ours");
132132

133133
platform_ref.options.text.conflict = builtin_driver::text::Conflict::ResolveWithTheirs;
134134
let res = platform_ref.merge(&mut buf, default_labels(), &Default::default())?;
135-
assert_eq!(res, (Pick::Buffer, Resolution::Complete));
135+
assert_eq!(res, (Pick::Buffer, Resolution::CompleteWithAutoResolvedConflict));
136136
assert_eq!(buf.as_bstr(), "theirs");
137137

138138
platform_ref.options.text.conflict = builtin_driver::text::Conflict::ResolveWithUnion;
139139
let res = platform_ref.merge(&mut buf, default_labels(), &Default::default())?;
140-
assert_eq!(res, (Pick::Buffer, Resolution::Complete));
140+
assert_eq!(res, (Pick::Buffer, Resolution::CompleteWithAutoResolvedConflict));
141141
assert_eq!(buf.as_bstr(), "ours\ntheirs");
142142

143143
platform_ref.driver = DriverChoice::BuiltIn(BuiltinDriver::Union);
144144
platform_ref.options.text.conflict = builtin_driver::text::Conflict::default();
145145
let res = platform_ref.merge(&mut buf, default_labels(), &Default::default())?;
146-
assert_eq!(res, (Pick::Buffer, Resolution::Complete));
146+
assert_eq!(res, (Pick::Buffer, Resolution::CompleteWithAutoResolvedConflict));
147147
assert_eq!(buf.as_bstr(), "ours\ntheirs");
148148

149149
platform_ref.driver = DriverChoice::BuiltIn(BuiltinDriver::Binary);

0 commit comments

Comments
 (0)