Skip to content

Commit 71b285c

Browse files
jakebrombergjakebromberg
committed
Migrate stash-tidy onto the CleanPipeline seam
Rewrite git-stash-tidy's hand-rolled clean loop to delegate iterate/filter/act/aggregate to the shared git_tidy_core::clean::run_clean seam (issue #118 step 3). The public run_clean signature and CleanResult<DroppedStash> return type are unchanged, so no callers or tests needed edits. The should_clean classification filter becomes the pure, silent decide predicate (Decision::Clean/Skip), and a single act closure owns the tool-specific work: dry-run-vs-real branching, the verbatim "would drop {ref} in {group}" / "dropped {ref} in {group}" / "error: could not drop {ref}" wording, the git.stash_drop call, and construction of the success record (Outcome::Cleaned) or failure record (Outcome::Failed(FailedItem)). run_clean is invoked once per repo group so each repo's output stays contiguous, with succeeded/failed/skipped accumulated across groups. Descending-drop-order is preserved by pre-sorting each group's stashes by descending stash@{N} index before the call, in the new order_group_items helper, because the shared loop preserves input order. Admitted stashes (should_clean true and a parseable index) sort high-index-first so dropping stash@{2} never renumbers a lower pending index; should_clean-rejected stashes are retained so the loop still counts them as skipped; and stashes that would be cleaned but whose ref does not parse are excluded entirely, exactly as the original loop silently ignored them. The index parsing reuses the unchanged parse_stash_index. clean_drops_in_descending_order and every other unit and integration test pass unchanged. Refs #118
1 parent fe86303 commit 71b285c

1 file changed

Lines changed: 81 additions & 42 deletions

File tree

‎crates/git-stash-tidy/src/clean.rs‎

Lines changed: 81 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
use std::io::Write;
22
use std::path::PathBuf;
33

4+
use git_tidy_core::clean::{Decision, Outcome, run_clean as core_run_clean};
45
use git_tidy_core::error::Error;
56
use git_tidy_core::git::GitOps;
67
use git_tidy_core::types::{CleanResult, FailedItem};
78

8-
use crate::types::{StashClassification, StashScanResult};
9+
use crate::types::{StashClassification, StashInfo, StashScanResult};
910

1011
/// Options controlling stash cleanup behavior.
1112
pub struct CleanOptions {
@@ -29,7 +30,11 @@ pub struct DroppedStash {
2930
/// Run the clean operation on a scan result.
3031
///
3132
/// Key design: drops stashes in descending index order per repo to prevent
32-
/// index renumbering side effects.
33+
/// index renumbering side effects. Because the shared [`core_run_clean`] loop
34+
/// preserves input order, each group's stashes are pre-sorted by descending
35+
/// stash index *before* the call (see [`order_group_items`]); the loop then
36+
/// drops them high-index-first while [`should_clean`] (as the `decide` filter)
37+
/// counts the rest as skipped.
3338
pub fn run_clean(
3439
git: &dyn GitOps,
3540
scan_result: &StashScanResult,
@@ -40,52 +45,55 @@ pub fn run_clean(
4045
let mut failed = Vec::new();
4146
let mut skipped = 0;
4247

48+
// One `core_run_clean` call per group keeps each repo's output contiguous
49+
// and lets the per-group descending-index pre-sort do the ordering work.
4350
for group in &scan_result.repos {
44-
// Collect stashes to drop, along with their parsed indices
45-
let mut to_drop: Vec<(usize, &str, &StashClassification)> = Vec::new();
46-
47-
for stash in &group.items {
48-
if !should_clean(&stash.classification, options) {
49-
skipped += 1;
50-
continue;
51-
}
52-
53-
if let Some(idx) = parse_stash_index(&stash.stash_ref) {
54-
to_drop.push((idx, &stash.stash_ref, &stash.classification));
55-
}
56-
}
51+
let ordered = order_group_items(&group.items, options);
52+
53+
let result = core_run_clean(
54+
ordered,
55+
|stash| {
56+
if should_clean(&stash.classification, options) {
57+
Decision::Clean
58+
} else {
59+
Decision::Skip
60+
}
61+
},
62+
|stash, out| {
63+
let stash_ref = stash.stash_ref.as_str();
5764

58-
// Sort by descending index to prevent renumbering
59-
to_drop.sort_by_key(|b| std::cmp::Reverse(b.0));
60-
61-
for (_, stash_ref, _) in &to_drop {
62-
if options.dry_run {
63-
writeln!(out, "would drop {} in {}", stash_ref, group.name)?;
64-
succeeded.push(DroppedStash {
65-
repo: group.repo_path.clone(),
66-
stash_ref: stash_ref.to_string(),
67-
});
68-
continue;
69-
}
70-
71-
match git.stash_drop(&group.repo_path, stash_ref) {
72-
Ok(()) => {
73-
writeln!(out, "dropped {} in {}", stash_ref, group.name)?;
74-
succeeded.push(DroppedStash {
65+
if options.dry_run {
66+
writeln!(out, "would drop {} in {}", stash_ref, group.name)?;
67+
return Ok(Outcome::Cleaned(DroppedStash {
7568
repo: group.repo_path.clone(),
7669
stash_ref: stash_ref.to_string(),
77-
});
70+
}));
7871
}
79-
Err(e) => {
80-
writeln!(out, "error: could not drop {}: {e}", stash_ref)?;
81-
failed.push(FailedItem {
82-
repo: group.repo_path.clone(),
83-
name: stash_ref.to_string(),
84-
reason: e.to_string(),
85-
});
72+
73+
match git.stash_drop(&group.repo_path, stash_ref) {
74+
Ok(()) => {
75+
writeln!(out, "dropped {} in {}", stash_ref, group.name)?;
76+
Ok(Outcome::Cleaned(DroppedStash {
77+
repo: group.repo_path.clone(),
78+
stash_ref: stash_ref.to_string(),
79+
}))
80+
}
81+
Err(e) => {
82+
writeln!(out, "error: could not drop {}: {e}", stash_ref)?;
83+
Ok(Outcome::Failed(FailedItem {
84+
repo: group.repo_path.clone(),
85+
name: stash_ref.to_string(),
86+
reason: e.to_string(),
87+
}))
88+
}
8689
}
87-
}
88-
}
90+
},
91+
out,
92+
)?;
93+
94+
succeeded.extend(result.succeeded);
95+
failed.extend(result.failed);
96+
skipped += result.skipped;
8997
}
9098

9199
Ok(CleanResult {
@@ -95,6 +103,37 @@ pub fn run_clean(
95103
})
96104
}
97105

106+
/// Order one group's stashes for the shared clean loop.
107+
///
108+
/// The shared loop preserves input order, so the descending-index drop order is
109+
/// established here. Admitted, drop-eligible stashes are those that both pass
110+
/// [`should_clean`] and carry a parseable `stash@{N}` index; they are sorted by
111+
/// descending index so that dropping a higher index never renumbers a lower one
112+
/// still pending. Stashes rejected by [`should_clean`] are retained so the loop
113+
/// still counts them as skipped (their relative order is irrelevant). Stashes
114+
/// that would be cleaned but whose ref does not parse are dropped from the
115+
/// list entirely — matching the original loop, which silently ignored them
116+
/// (never counting them as succeeded, failed, or skipped).
117+
fn order_group_items<'a>(items: &'a [StashInfo], options: &CleanOptions) -> Vec<&'a StashInfo> {
118+
let mut ordered: Vec<&StashInfo> = items
119+
.iter()
120+
.filter(|stash| {
121+
// Keep skip candidates regardless of parseability; for admitted
122+
// stashes, keep only those with a parseable index.
123+
!should_clean(&stash.classification, options)
124+
|| parse_stash_index(&stash.stash_ref).is_some()
125+
})
126+
.collect();
127+
128+
// Descending stash index. Skip candidates (no admitted index needed) sort by
129+
// their own parsed index or 0; their position only affects skip ordering,
130+
// which the result does not observe.
131+
ordered
132+
.sort_by_key(|stash| std::cmp::Reverse(parse_stash_index(&stash.stash_ref).unwrap_or(0)));
133+
134+
ordered
135+
}
136+
98137
/// Determine if a stash should be cleaned based on its classification and options.
99138
fn should_clean(classification: &StashClassification, options: &CleanOptions) -> bool {
100139
if options.all {

0 commit comments

Comments
 (0)