Skip to content

Commit 0b21bf9

Browse files
ByronByron
committed
Play it much safer when switching to the fallback algorithm
1 parent 6f78b75 commit 0b21bf9

File tree

2 files changed

+51
-33
lines changed

2 files changed

+51
-33
lines changed

‎crates/but-workspace/src/commit_engine/tree/mod.rs‎

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::commit_engine::{HunkRange, MoveSourceCommit, RejectionReason, apply_hunks};
22
use crate::{DiffSpec, HunkHeader};
3+
use anyhow::bail;
34
use bstr::{BStr, ByteSlice};
45
use but_core::{RepositoryExt, UnifiedDiff};
56
use gix::filter::plumbing::pipeline::convert::ToGitOutcome;
@@ -238,7 +239,7 @@ pub fn apply_worktree_changes<'repo>(
238239

239240
let selected_hunks = change_request.hunk_headers.drain(..);
240241
let (hunks_to_commit, rejected) =
241-
to_additive_hunks(selected_hunks, &worktree_hunks, &worktree_hunks_no_context);
242+
to_additive_hunks(selected_hunks, &worktree_hunks, &worktree_hunks_no_context)?;
242243

243244
change_request.hunk_headers = rejected;
244245
if hunks_to_commit.is_empty() && !change_request.hunk_headers.is_empty() {
@@ -364,7 +365,7 @@ fn to_additive_hunks(
364365
hunks_to_keep: impl IntoIterator<Item = HunkHeader>,
365366
worktree_hunks: &[HunkHeader],
366367
worktree_hunks_no_context: &[HunkHeader],
367-
) -> (Vec<HunkHeader>, Vec<HunkHeader>) {
368+
) -> anyhow::Result<(Vec<HunkHeader>, Vec<HunkHeader>)> {
368369
let mut hunks_to_commit = Vec::new();
369370
let mut rejected = Vec::new();
370371
let mut previous = HunkHeader {
@@ -422,43 +423,56 @@ fn to_additive_hunks(
422423
rejected.push(sh);
423424
}
424425

425-
if !hunks_to_commit
426-
.iter()
427-
.zip(hunks_to_commit.iter().skip(1))
428-
.all(|(prev, next)| prev < next)
429-
{
426+
fn in_order(hunks: &[HunkHeader]) -> bool {
427+
hunks
428+
.iter()
429+
.zip(hunks.iter().skip(1))
430+
.all(|(prev, next)| prev < next)
431+
}
432+
433+
let res = if !in_order(&hunks_to_commit) {
430434
tracing::info!("Using alternative hunk algorithm for {hunks_to_commit:?}");
431-
to_additive_hunks_fallback(
432-
hunks_to_commit.into_iter().map(
435+
let hunks: Vec<_> = hunks_to_commit
436+
.into_iter()
437+
.map(
433438
|HunkHeader {
434439
old_start,
435440
old_lines,
436441
new_start,
437442
new_lines,
438443
}| {
439444
if old_lines == 0 {
440-
HunkHeader {
445+
Ok(HunkHeader {
441446
old_start: 0,
442447
old_lines: 0,
443448
new_start,
444449
new_lines,
445-
}
446-
} else {
447-
HunkHeader {
450+
})
451+
} else if new_lines == 0 {
452+
Ok(HunkHeader {
448453
old_start,
449454
old_lines,
450455
new_start: 0,
451456
new_lines: 0,
452-
}
457+
})
458+
} else {
459+
bail!("Unexpected hunk with neither newlines or oldlines being 0");
453460
}
454461
},
455-
),
456-
worktree_hunks,
457-
worktree_hunks_no_context,
458-
)
462+
)
463+
.collect::<Result<_, _>>()?;
464+
let (hunks_to_commit, rejected) =
465+
to_additive_hunks_fallback(hunks, worktree_hunks, worktree_hunks_no_context);
466+
if !in_order(&hunks_to_commit) {
467+
bail!(
468+
"Alternative hunks algorithms still didn't produce properly ordered hunks or saw duplicate inputs: {hunks_to_commit:?}"
469+
);
470+
}
471+
(hunks_to_commit, rejected)
459472
} else {
460473
(hunks_to_commit, rejected)
461-
}
474+
};
475+
Ok(res)
462476
}
463477

464478
/// This algorithm is better when the basic one fails, but both have their merit.

‎crates/but-workspace/src/commit_engine/tree/tests.rs‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ mod to_additive_hunks {
1616
],
1717
&wth,
1818
&wth,
19-
), @r#"
19+
).unwrap(), @r#"
2020
(
2121
[],
2222
[
@@ -39,7 +39,7 @@ mod to_additive_hunks {
3939
],
4040
&wth,
4141
&wth,
42-
), @r#"
42+
).unwrap(), @r#"
4343
(
4444
[
4545
HunkHeader("-1,1", "+1,0"),
@@ -57,7 +57,7 @@ mod to_additive_hunks {
5757
],
5858
&wth,
5959
&wth,
60-
), @r#"
60+
).unwrap(), @r#"
6161
(
6262
[
6363
HunkHeader("-1,0", "+1,1"),
@@ -75,7 +75,7 @@ mod to_additive_hunks {
7575
],
7676
&wth,
7777
&wth,
78-
), @r#"
78+
).unwrap(), @r#"
7979
(
8080
[
8181
HunkHeader("-1,0", "+1,1"),
@@ -93,7 +93,7 @@ mod to_additive_hunks {
9393
],
9494
&wth,
9595
&wth,
96-
), @r#"
96+
).unwrap(), @r#"
9797
(
9898
[
9999
HunkHeader("-1,1", "+1,0"),
@@ -123,7 +123,7 @@ mod to_additive_hunks {
123123
],
124124
&wth,
125125
&wth,
126-
), @r#"
126+
).unwrap(), @r#"
127127
(
128128
[
129129
HunkHeader("-1,10", "+1,10"),
@@ -151,7 +151,7 @@ mod to_additive_hunks {
151151
],
152152
&wth,
153153
&wth,
154-
), @r#"
154+
).unwrap(), @r#"
155155
(
156156
[
157157
HunkHeader("-1,10", "+1,10"),
@@ -206,7 +206,7 @@ mod to_additive_hunks {
206206
[hunk_header("-96,1", "+0,0")],
207207
&wth,
208208
&wth0,
209-
), @r#"
209+
).unwrap(), @r#"
210210
(
211211
[
212212
HunkHeader("-96,1", "+96,0"),
@@ -218,7 +218,7 @@ mod to_additive_hunks {
218218
[hunk_header("-96,1", "+0,0"), hunk_header("-0,0", "+96,2")],
219219
&wth,
220220
&wth0,
221-
), @r#"
221+
).unwrap(), @r#"
222222
(
223223
[
224224
HunkHeader("-96,1", "+96,0"),
@@ -231,7 +231,7 @@ mod to_additive_hunks {
231231
[hunk_header("-0,0", "+96,2")],
232232
&wth,
233233
&wth0,
234-
), @r#"
234+
).unwrap(), @r#"
235235
(
236236
[
237237
HunkHeader("-96,0", "+96,2"),
@@ -265,7 +265,8 @@ mod to_additive_hunks {
265265
],
266266
&wth,
267267
&wth0,
268-
);
268+
)
269+
.unwrap();
269270
insta::assert_debug_snapshot!(actual, @r#"
270271
(
271272
[
@@ -289,7 +290,8 @@ mod to_additive_hunks {
289290
],
290291
&wth,
291292
&wth0,
292-
);
293+
)
294+
.unwrap();
293295
insta::assert_debug_snapshot!(actual, @r#"
294296
(
295297
[
@@ -317,7 +319,8 @@ mod to_additive_hunks {
317319
],
318320
&wth,
319321
&wth0,
320-
);
322+
)
323+
.unwrap();
321324
insta::assert_debug_snapshot!(actual, @r#"
322325
(
323326
[
@@ -355,7 +358,8 @@ mod to_additive_hunks {
355358
],
356359
&wth,
357360
&wth,
358-
);
361+
)
362+
.unwrap();
359363
insta::assert_debug_snapshot!(actual, @r#"
360364
(
361365
[

0 commit comments

Comments
 (0)