Skip to content

Commit e745168

Browse files
ByronByron
committed
Combine both algorithms to get the desired results.
The first one is actually better for more intuitive results in other cases, so let's combine both.
1 parent 8e07f95 commit e745168

File tree

2 files changed

+158
-44
lines changed

2 files changed

+158
-44
lines changed

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

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,11 +360,119 @@ pub(crate) fn worktree_file_to_git_in_buf(
360360
/// selections in the old or new image respectively. This is necessary to maintain the right order in the face of context lines.
361361
/// Note that the order of changes is still affected by what which selection comes first, i.e. old and new, or vice versa, if these
362362
/// selections are in the same hunk.
363-
#[allow(clippy::indexing_slicing)]
364363
fn to_additive_hunks(
365364
hunks_to_keep: impl IntoIterator<Item = HunkHeader>,
366365
worktree_hunks: &[HunkHeader],
367366
worktree_hunks_no_context: &[HunkHeader],
367+
) -> (Vec<HunkHeader>, Vec<HunkHeader>) {
368+
let mut hunks_to_commit = Vec::new();
369+
let mut rejected = Vec::new();
370+
let mut previous = HunkHeader {
371+
old_start: 1,
372+
old_lines: 0,
373+
new_start: 1,
374+
new_lines: 0,
375+
};
376+
let mut last_wh = None;
377+
for selected_hunk in hunks_to_keep {
378+
let sh = selected_hunk;
379+
if sh.new_range().is_null() {
380+
if let Some(wh) = worktree_hunks_no_context
381+
.iter()
382+
.find(|wh| wh.old_range().contains(sh.old_range()))
383+
{
384+
if last_wh != Some(*wh) {
385+
last_wh = Some(*wh);
386+
previous.new_start = wh.new_start;
387+
}
388+
hunks_to_commit.push(HunkHeader {
389+
old_start: sh.old_start,
390+
old_lines: sh.old_lines,
391+
new_start: previous.new_start,
392+
new_lines: 0,
393+
});
394+
previous.old_start = sh.old_range().end();
395+
continue;
396+
}
397+
} else if sh.old_range().is_null() {
398+
if let Some(wh) = worktree_hunks_no_context
399+
.iter()
400+
.find(|wh| wh.new_range().contains(sh.new_range()))
401+
{
402+
if last_wh != Some(*wh) {
403+
last_wh = Some(*wh);
404+
previous.old_start = wh.old_start;
405+
}
406+
hunks_to_commit.push(HunkHeader {
407+
old_start: previous.old_start,
408+
old_lines: 0,
409+
new_start: sh.new_start,
410+
new_lines: sh.new_lines,
411+
});
412+
previous.new_start = sh.new_range().end();
413+
continue;
414+
}
415+
} else if worktree_hunks.contains(&sh) {
416+
previous.old_start = sh.old_range().end();
417+
previous.new_start = sh.new_range().end();
418+
last_wh = Some(sh);
419+
hunks_to_commit.push(sh);
420+
continue;
421+
}
422+
rejected.push(sh);
423+
}
424+
425+
if !hunks_to_commit
426+
.iter()
427+
.zip(hunks_to_commit.iter().skip(1))
428+
.all(|(prev, next)| prev < next)
429+
{
430+
tracing::info!("Using alternative hunk algorithm for {hunks_to_commit:?}");
431+
to_additive_hunks_fallback(
432+
hunks_to_commit.into_iter().map(
433+
|HunkHeader {
434+
old_start,
435+
old_lines,
436+
new_start,
437+
new_lines,
438+
}| {
439+
if old_lines == 0 {
440+
HunkHeader {
441+
old_start: 0,
442+
old_lines: 0,
443+
new_start,
444+
new_lines,
445+
}
446+
} else {
447+
HunkHeader {
448+
old_start,
449+
old_lines,
450+
new_start: 0,
451+
new_lines: 0,
452+
}
453+
}
454+
},
455+
),
456+
worktree_hunks,
457+
worktree_hunks_no_context,
458+
)
459+
} else {
460+
(hunks_to_commit, rejected)
461+
}
462+
}
463+
464+
/// This algorithm is better when the basic one fails, but both have their merit.
465+
/// Right now this is a brute-force one or the other approach, but we could also apply them selectively.
466+
/// Note that what we really want to simulate is what the UI shows. But since the UI also doesn't really know hunks,
467+
/// we have to fiddle it together here, at least we know the hunks themselves.
468+
///
469+
/// Note that this algorithm is kind of the opposite of what people would expect if it's run where `to_additive_hunks()` works.
470+
/// But here we are… just making this work.
471+
#[allow(clippy::indexing_slicing)]
472+
fn to_additive_hunks_fallback(
473+
hunks_to_keep: impl IntoIterator<Item = HunkHeader>,
474+
worktree_hunks: &[HunkHeader],
475+
worktree_hunks_no_context: &[HunkHeader],
368476
) -> (Vec<HunkHeader>, Vec<HunkHeader>) {
369477
let mut rejected = Vec::new();
370478

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

Lines changed: 49 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ mod to_additive_hunks {
7878
), @r#"
7979
(
8080
[
81-
HunkHeader("-5,2", "+1,1"),
81+
HunkHeader("-1,0", "+1,1"),
82+
HunkHeader("-5,2", "+2,0"),
8283
HunkHeader("-7,0", "+10,1"),
8384
],
8485
[],
@@ -95,53 +96,15 @@ mod to_additive_hunks {
9596
), @r#"
9697
(
9798
[
98-
HunkHeader("-1,1", "+5,2"),
99+
HunkHeader("-1,1", "+1,0"),
100+
HunkHeader("-2,0", "+5,2"),
99101
HunkHeader("-10,1", "+7,0"),
100102
],
101103
[],
102104
)
103105
"#);
104106
}
105107

106-
#[test]
107-
fn only_selections_workspace_example() {
108-
let wth = vec![hunk_header("-1,10", "+1,10")];
109-
let actual = to_additive_hunks(
110-
[
111-
// commit NOT '2,3' of the old
112-
hunk_header("-2,2", "+0,0"),
113-
// commit NOT '6,7' of the old
114-
hunk_header("-6,2", "+0,0"),
115-
// commit NOT '9' of the old
116-
hunk_header("-9,1", "+0,0"),
117-
// commit NOT '10' of the old
118-
hunk_header("-10,1", "+0,0"),
119-
// commit '11' of the new
120-
hunk_header("-0,0", "+1,1"),
121-
// commit '15,16' of the new
122-
hunk_header("-0,0", "+5,2"),
123-
// commit '19,20' of the new
124-
hunk_header("-0,0", "+9,2"),
125-
],
126-
&wth,
127-
&wth,
128-
);
129-
insta::assert_debug_snapshot!(actual, @r#"
130-
(
131-
[
132-
HunkHeader("-2,2", "+1,0"),
133-
HunkHeader("-6,2", "+1,0"),
134-
HunkHeader("-9,1", "+1,0"),
135-
HunkHeader("-10,1", "+1,0"),
136-
HunkHeader("-11,0", "+1,1"),
137-
HunkHeader("-11,0", "+5,2"),
138-
HunkHeader("-11,0", "+9,2"),
139-
],
140-
[],
141-
)
142-
"#);
143-
}
144-
145108
#[test]
146109
fn selections_and_full_hunks() {
147110
let wth = vec![
@@ -156,14 +119,16 @@ mod to_additive_hunks {
156119
// partial match to same hunk
157120
hunk_header("-15,2", "+0,0"),
158121
hunk_header("-0,0", "+22,3"),
122+
// Last hunk isn't used
159123
],
160124
&wth,
161125
&wth,
162126
), @r#"
163127
(
164128
[
165129
HunkHeader("-1,10", "+1,10"),
166-
HunkHeader("-15,2", "+22,3"),
130+
HunkHeader("-15,2", "+20,0"),
131+
HunkHeader("-17,0", "+22,3"),
167132
],
168133
[],
169134
)
@@ -182,6 +147,7 @@ mod to_additive_hunks {
182147
// full match
183148
hunk_header("-1,10", "+1,10"),
184149
hunk_header("-15,5", "+20,5"),
150+
// Last hunk isn't used
185151
],
186152
&wth,
187153
&wth,
@@ -255,7 +221,8 @@ mod to_additive_hunks {
255221
), @r#"
256222
(
257223
[
258-
HunkHeader("-96,1", "+96,2"),
224+
HunkHeader("-96,1", "+96,0"),
225+
HunkHeader("-97,0", "+96,2"),
259226
],
260227
[],
261228
)
@@ -365,4 +332,43 @@ mod to_additive_hunks {
365332
)
366333
"#);
367334
}
335+
336+
#[test]
337+
fn only_selections_workspace_example() {
338+
let wth = vec![hunk_header("-1,10", "+1,10")];
339+
let actual = to_additive_hunks(
340+
[
341+
// commit NOT '2,3' of the old
342+
hunk_header("-2,2", "+0,0"),
343+
// commit NOT '6,7' of the old
344+
hunk_header("-6,2", "+0,0"),
345+
// commit NOT '9' of the old
346+
hunk_header("-9,1", "+0,0"),
347+
// commit NOT '10' of the old
348+
hunk_header("-10,1", "+0,0"),
349+
// commit '11' of the new
350+
hunk_header("-0,0", "+1,1"),
351+
// commit '15,16' of the new
352+
hunk_header("-0,0", "+5,2"),
353+
// commit '19,20' of the new
354+
hunk_header("-0,0", "+9,2"),
355+
],
356+
&wth,
357+
&wth,
358+
);
359+
insta::assert_debug_snapshot!(actual, @r#"
360+
(
361+
[
362+
HunkHeader("-2,2", "+1,0"),
363+
HunkHeader("-6,2", "+1,0"),
364+
HunkHeader("-9,1", "+1,0"),
365+
HunkHeader("-10,1", "+1,0"),
366+
HunkHeader("-11,0", "+1,1"),
367+
HunkHeader("-11,0", "+5,2"),
368+
HunkHeader("-11,0", "+9,2"),
369+
],
370+
[],
371+
)
372+
"#);
373+
}
368374
}

0 commit comments

Comments
 (0)