fix(prune): honest reclaim/skip counts; cross-ref layout coupling
Already-absent files no longer inflate reclaimedCount (outAlreadyAbsent distinguishes vanished-at-delete from real failures). Stale skip count de-dups confirmed before subtraction so duplicates aren't tallied as stale. prune_button.h rightInset and drawTailFooter vrc.right now cross-reference each other by name.
This commit is contained in:
+7
-3
@@ -556,8 +556,11 @@ void drawTailFooter(LICE_IBitmap* bmp, int w, int h) {
|
||||
// matches the left inset; DT_RIGHT keeps it clear of the left-aligned tail label
|
||||
// (the two never overlap at normal panel widths — the label is short, the readout is
|
||||
// ~10 chars, and DT_END_ELLIPSIS on both degrades gracefully if a panel is ever tiny).
|
||||
// COUPLED TO PruneButtonSpec::rightInset (prune_button.h): the prune button is
|
||||
// right-anchored at footer.right - 84, placing its right edge 76 px left of this
|
||||
// readout's right margin. If this inset (currently 8) changes, update rightInset there.
|
||||
RECT vrc = f;
|
||||
vrc.right -= 8;
|
||||
vrc.right -= 8; // COUPLED: PruneButtonSpec::rightInset in prune_button.h is 84
|
||||
SetTextColor(dc, kRgbFooterVersion);
|
||||
DrawText(dc, reasampler::appVersion().c_str(), -1, &vrc,
|
||||
DT_RIGHT | DT_VCENTER | DT_SINGLELINE | DT_END_ELLIPSIS);
|
||||
@@ -567,8 +570,9 @@ void drawTailFooter(LICE_IBitmap* bmp, int w, int h) {
|
||||
// of truth for both draw and hit-test (they never drift). Empty (button.empty()) when the
|
||||
// footer is degenerate or too narrow to place the button clear of the tail label — the
|
||||
// action stays reachable via its bindable command, so a suppressed button is graceful.
|
||||
// The version readout uses an 8 px right inset (drawTailFooter); the button's rightInset
|
||||
// clears it (~72 px) so the two never overlap at normal panel widths.
|
||||
// Clearance from the version readout: PruneButtonSpec::rightInset (84) places the button
|
||||
// right edge 76 px left of the readout's 8 px right margin — see coupling comments in
|
||||
// prune_button.h and drawTailFooter above.
|
||||
ButtonRect pruneButtonRectFor(int w, int h) {
|
||||
const RECT f = panelFooter(w, h);
|
||||
if (f.top >= f.bottom) return ButtonRect{}; // degenerate footer -> no button
|
||||
|
||||
+34
-13
@@ -79,6 +79,7 @@
|
||||
#include <string>
|
||||
#include <system_error>
|
||||
#include <unordered_map>
|
||||
#include <unordered_set>
|
||||
#include <vector>
|
||||
|
||||
// Move-to-trash surface (fork R-C, trash-preferred). On Windows the Recycle Bin is
|
||||
@@ -352,9 +353,13 @@ std::vector<std::string> ReaSamplerSession::pruneOrphanSet() const {
|
||||
namespace {
|
||||
|
||||
// Deletes ONE orphan file, trash-preferred (fork R-C, settled). Returns true iff the
|
||||
// file is gone from disk after the call (deleted here, OR already absent — an already-
|
||||
// vanished file is a success for the reclaim's purpose, not a failure). `absPath` is the
|
||||
// resolved absolute path (forward-slashed). NON-THROWING: no exception may cross the C ABI.
|
||||
// file was deleted BY THIS CALL (reclaimed here). Returns false for two distinct cases:
|
||||
// * `outAlreadyAbsent` set true — the file was already gone before we touched it;
|
||||
// the caller folds this into the stale/staleness tally, NOT reclaimedCount.
|
||||
// * `outAlreadyAbsent` left false — a real delete failure (locked, conversion error);
|
||||
// the caller folds this into skippedCount.
|
||||
// `absPath` is the resolved absolute path (forward-slashed). NON-THROWING: no exception
|
||||
// may cross the C ABI.
|
||||
//
|
||||
// Per-platform routing:
|
||||
// * Windows — SHFileOperationW(FO_DELETE, pFrom=<double-NUL path>, FOF_ALLOWUNDO |
|
||||
@@ -364,7 +369,8 @@ namespace {
|
||||
// * Other (SWELL: macOS/Linux) — no portable move-to-trash surface is available in this
|
||||
// codebase, so fall back to std::filesystem::remove (hard unlink) behind the R3
|
||||
// confirm guardrail. `outUsedTrash` left as-is (false).
|
||||
bool deleteOrphanFile(const std::string& absPath, bool& outUsedTrash) {
|
||||
bool deleteOrphanFile(const std::string& absPath, bool& outUsedTrash,
|
||||
bool& outAlreadyAbsent) {
|
||||
#ifdef _WIN32
|
||||
// Convert forward-slashed UTF-8 to a back-slashed, double-NUL-terminated wide string.
|
||||
// SHFileOperation's pFrom is a list; a single path still needs the extra terminating
|
||||
@@ -373,7 +379,7 @@ bool deleteOrphanFile(const std::string& absPath, bool& outUsedTrash) {
|
||||
for (char& c : win) if (c == '/') c = '\\';
|
||||
|
||||
const int wlen = MultiByteToWideChar(CP_UTF8, 0, win.c_str(), -1, nullptr, 0);
|
||||
if (wlen <= 0) return false; // conversion failed -> report as skip
|
||||
if (wlen <= 0) return false; // conversion failed -> real skip (outAlreadyAbsent stays false)
|
||||
std::vector<wchar_t> wbuf(static_cast<std::size_t>(wlen) + 1, L'\0'); // +1 for list NUL
|
||||
MultiByteToWideChar(CP_UTF8, 0, win.c_str(), -1, wbuf.data(), wlen);
|
||||
// wbuf now holds the path + its NUL at [wlen-1]; the extra trailing L'\0' at [wlen]
|
||||
@@ -389,20 +395,25 @@ bool deleteOrphanFile(const std::string& absPath, bool& outUsedTrash) {
|
||||
const int rv = SHFileOperationW(&op);
|
||||
if (rv == 0 && !op.fAnyOperationsAborted) {
|
||||
outUsedTrash = true;
|
||||
return true;
|
||||
return true; // deleted this call -> reclaimed
|
||||
}
|
||||
// SHFileOperation failed (e.g. file already gone yields a nonzero code on some
|
||||
// versions, or a lock). Treat "already absent" as success; otherwise a real skip.
|
||||
// versions, or a lock). Distinguish "already absent" from a real failure so the
|
||||
// caller can tally them separately (absent -> staleness skip; failure -> locked skip).
|
||||
std::error_code ec;
|
||||
return !fs::exists(absPath, ec);
|
||||
if (!fs::exists(absPath, ec)) {
|
||||
outAlreadyAbsent = true; // vanished between scan and delete -> staleness, not reclaim
|
||||
}
|
||||
return false;
|
||||
#else
|
||||
// No portable trash surface on SWELL platforms -> hard unlink behind the confirm.
|
||||
std::error_code ec;
|
||||
const bool removed = fs::remove(absPath, ec);
|
||||
if (removed) return true; // deleted this call
|
||||
if (removed) return true; // deleted this call -> reclaimed
|
||||
if (ec) return false; // a real failure (locked / permission) -> skip
|
||||
// remove returned false with no error == the file did not exist -> already gone.
|
||||
return !fs::exists(absPath, ec);
|
||||
outAlreadyAbsent = true; // vanished between scan and delete -> staleness, not reclaim
|
||||
return false;
|
||||
#endif
|
||||
}
|
||||
|
||||
@@ -423,8 +434,13 @@ PruneDeletionResult ReaSamplerSession::pruneReclaim(
|
||||
|
||||
const std::vector<std::string> plan = pruneDeletePlan(confirmed, scan.orphans);
|
||||
|
||||
// Anything the user confirmed but that is no longer a fresh orphan is a staleness skip.
|
||||
result.skippedCount += confirmed.size() - plan.size();
|
||||
// Staleness skip count: entries the user confirmed that are no longer fresh orphans
|
||||
// (vanished or became referenced between confirm and delete). pruneDeletePlan already
|
||||
// de-dups confirmed internally, so compute the unique-confirmed size to avoid counting
|
||||
// de-duplicated entries as stale — that would be dishonest.
|
||||
const std::size_t uniqueConfirmedCount =
|
||||
std::unordered_set<std::string>(confirmed.begin(), confirmed.end()).size();
|
||||
result.skippedCount += uniqueConfirmedCount - plan.size();
|
||||
|
||||
for (const std::string& rel : plan) {
|
||||
// Reconstruct the absolute path from the resolved bank dir + the entry's file name.
|
||||
@@ -437,9 +453,14 @@ PruneDeletionResult ReaSamplerSession::pruneReclaim(
|
||||
const auto szIt = scan.sizeByRel.find(rel);
|
||||
const std::uint64_t bytes = (szIt != scan.sizeByRel.end()) ? szIt->second : 0;
|
||||
|
||||
if (deleteOrphanFile(absPath, result.usedTrash)) {
|
||||
bool alreadyAbsent = false;
|
||||
if (deleteOrphanFile(absPath, result.usedTrash, alreadyAbsent)) {
|
||||
++result.reclaimedCount;
|
||||
result.reclaimedBytes += bytes;
|
||||
} else if (alreadyAbsent) {
|
||||
// File vanished between plan and delete — treat as staleness, same as the
|
||||
// confirm→plan gap above. Does NOT count as reclaimed (we didn't delete it).
|
||||
++result.skippedCount;
|
||||
} else {
|
||||
++result.skippedCount; // locked / conversion failure -> recorded, not thrown
|
||||
}
|
||||
|
||||
+7
-2
@@ -59,7 +59,12 @@ struct ButtonRect {
|
||||
// * buttonWidth — the button's fixed width.
|
||||
// * rightInset — gap from the footer's right edge to the button's right edge (the
|
||||
// button sits left of this inset, clearing the right-aligned version
|
||||
// readout which uses its own inset).
|
||||
// readout). COUPLED TO drawTailFooter (bank_panel.cpp): the version
|
||||
// readout uses `vrc.right -= 8` (8 px right margin). The button's
|
||||
// right edge lands at footer.right - 84, i.e. 76 px left of the
|
||||
// readout's right margin — enough clearance for the ~10-char label.
|
||||
// If the version readout's inset changes in drawTailFooter, update
|
||||
// this value to maintain clearance.
|
||||
// * verticalInset — top/bottom gap inside the footer (the button is shorter than the
|
||||
// strip so it reads as a raised control, not a full-height fill).
|
||||
// * minLeftInset — the button's left edge must stay at least this far from the footer
|
||||
@@ -68,7 +73,7 @@ struct ButtonRect {
|
||||
// rect (button suppressed — see header placement contract).
|
||||
struct PruneButtonSpec {
|
||||
int buttonWidth = 72;
|
||||
int rightInset = 84;
|
||||
int rightInset = 84; // COUPLED: version readout in drawTailFooter uses vrc.right -= 8
|
||||
int verticalInset = 4;
|
||||
int minLeftInset = 120;
|
||||
};
|
||||
|
||||
@@ -77,11 +77,13 @@ struct PruneReport {
|
||||
// REAPER-free / filesystem-free by design (the shell does the deletion; this is the
|
||||
// tallied outcome), so the count/byte aggregation is unit-testable outside the DAW.
|
||||
//
|
||||
// * reclaimedCount — number of files actually removed from disk (trash or unlink).
|
||||
// * reclaimedCount — number of files actually removed from disk BY THIS CALL (trash or
|
||||
// unlink). Already-absent files are NOT counted here.
|
||||
// * reclaimedBytes — sum of the on-disk sizes of the files actually removed, in bytes.
|
||||
// * skippedCount — planned files that could NOT be removed (locked, disappeared, a
|
||||
// trash/unlink failure) OR that went stale between confirm and delete
|
||||
// (dropped by the delete-plan intersection). Never an error/crash.
|
||||
// * skippedCount — files that could not be or were not reclaimed: stale entries that
|
||||
// dropped out of the fresh-orphan intersection, files that vanished
|
||||
// between the plan and the delete call (already absent), and real
|
||||
// delete failures (locked, conversion error). Never an error/crash.
|
||||
// * usedTrash — true iff the deletions were routed to the OS trash/recycle bin
|
||||
// (recoverable); false iff the platform fell back to hard unlink.
|
||||
struct PruneDeletionResult {
|
||||
|
||||
Reference in New Issue
Block a user