From 7b53ba16f632459f214d115ee4a938bd4b418604 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 26 Jul 2026 18:47:02 -0400 Subject: [PATCH 1/2] =?UTF-8?q?feat(prune):=20R3=20guarded=20deletion=20?= =?UTF-8?q?=E2=80=94=20confirm-to-delete=20+=20panel=20button?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend BANK_PRUNE_FOLDER from report-only to dry-run→confirm-with-manifest→ delete exactly the pure-core orphan set (fresh recompute, stale entries skipped). Windows routes to Recycle Bin (SHFileOperation+FOF_ALLOWUNDO), else unlink. New pure prune_button module + footer button dispatching the action id. No ext-state write, no undo point (deletion is not REAPER-undoable). --- CMakeLists.txt | 17 +++- src/actions.cpp | 60 +++++++++--- src/actions.h | 6 ++ src/bank_panel.cpp | 55 +++++++++++ src/persist.cpp | 169 +++++++++++++++++++++++++++++---- src/persist.h | 32 +++++++ src/prune_button.cpp | 39 ++++++++ src/prune_button.h | 91 ++++++++++++++++++ src/prune_reconcile.cpp | 17 ++++ src/prune_reconcile.h | 40 ++++++++ tests/test_prune_button.cpp | 132 +++++++++++++++++++++++++ tests/test_prune_reconcile.cpp | 65 +++++++++++++ 12 files changed, 695 insertions(+), 28 deletions(-) create mode 100644 src/prune_button.cpp create mode 100644 src/prune_button.h create mode 100644 tests/test_prune_button.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 889dbe1..8e3063f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -226,6 +226,17 @@ target_include_directories(owned_manifest PUBLIC src) add_library(prune_reconcile STATIC src/prune_reconcile.cpp) target_include_directories(prune_reconcile PUBLIC src) +# --------------------------------------------------------------------------- +# 2g'''') Pure prune_button layout — NO REAPER, NO SWELL. The Phase R (Reclaim) +# Wave-3 (R3, fork R-E) prune-button geometry: footer rect -> right-anchored +# button rect (suppressed when the footer is too narrow), and point -> in/out +# hit-test. Split out so the button's placement + hit-test math is unit-tested +# outside the DAW; the bank_panel footer that draws it and dispatches the +# "Prune bank folder" command is DAW-verified. Mirror of mode_switch / tab_strip. +# --------------------------------------------------------------------------- +add_library(prune_button STATIC src/prune_button.cpp) +target_include_directories(prune_button PUBLIC src) + # --------------------------------------------------------------------------- # 2g) Pure realtime_record library — NO REAPER, NO SWELL. The M8 realtime-record # logic: capture scope + FX-tap point -> I_RECMODE / I_RECMODE_FLAGS values, @@ -354,6 +365,10 @@ add_executable(prune_reconcile_tests tests/test_prune_reconcile.cpp) target_link_libraries(prune_reconcile_tests PRIVATE prune_reconcile bank_book) add_test(NAME prune_reconcile_tests COMMAND prune_reconcile_tests) +add_executable(prune_button_tests tests/test_prune_button.cpp) +target_link_libraries(prune_button_tests PRIVATE prune_button) +add_test(NAME prune_button_tests COMMAND prune_button_tests) + add_executable(app_version_tests tests/test_app_version.cpp) target_link_libraries(app_version_tests PRIVATE app_version) add_test(NAME app_version_tests COMMAND app_version_tests) @@ -402,7 +417,7 @@ add_library(reaper_reasampler MODULE src/bank_book.cpp src/owned_manifest.cpp ) -target_link_libraries(reaper_reasampler PRIVATE bank_model capture_paths peaks bank_grid mode_switch tab_strip view_mode_model insert_plan render_settings tail_control realtime_record bank_book wav_trim owned_manifest prune_reconcile app_version provenance) +target_link_libraries(reaper_reasampler PRIVATE bank_model capture_paths peaks bank_grid mode_switch tab_strip view_mode_model insert_plan render_settings tail_control realtime_record bank_book wav_trim owned_manifest prune_reconcile prune_button app_version provenance) target_include_directories(reaper_reasampler PRIVATE ${SDK_INC} ${WDL_INC}) # OUTPUT_NAME is channel-derived (Phase V, V4): "reaper_reasampler" (stable, default) or # "reaper_reasampler_beta" (beta). REAPER dlopen's any reaper_* module, so both channels' diff --git a/src/actions.cpp b/src/actions.cpp index 0cbaec2..1868561 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -797,29 +797,65 @@ void doBankRemoveSelected() { if (removed > 0) persistBankOp("ReaSampler: remove sample(s)"); } -// Prune bank folder — Phase R (Reclaim), R2: REPORT-ONLY dry run. Asks the session for -// the orphan set of the resolved CURRENT bank folder ((owned ∩ present) − referenced, -// unioned across every bank) and prints the truthful reclaim report — count, reclaimable -// bytes, and the file list. DELETES NOTHING, writes no ext-state, opens no undo point -// (pruneDryRun is read-only across the persist seam). R3 extends this SAME action id to -// confirm-and-delete behind the dry-run guardrail; the report path here is what R3 wraps. +// Prune bank folder — Phase R (Reclaim), R3: the guarded DESTRUCTIVE step, and the SOLE +// file-deletion entry in ReaSampler. Dry-run FIRST (compute the orphan set, read-only), +// then — only when orphans exist — a blocking CONFIRM showing the SPECIFIC manifest +// (count + reclaimable bytes + the file list, truncated consistent with the 64-cap), then +// on explicit Yes delete EXACTLY that set (session->pruneReclaim, which recomputes the +// pure core fresh and deletes confirmed ∩ freshOrphans — trash-preferred, unlink fallback). +// Zero orphans => informational only, NO confirm ever shown. Cancel deletes nothing. +// +// The full (untruncated) orphan set is captured here for the delete; the dry-run's +// truncated list is only the confirm's readout. No ext-state is written and no undo point +// is opened (file deletion is not REAPER-undoable and pruneReclaim mutates no project +// state) — a Ctrl-Z after a prune correctly cannot claim to restore deleted files. void doBankPruneFolder() { const PruneReport report = g_session->pruneDryRun(); if (report.count == 0) { - ShowConsoleMsg("ReaSampler prune (dry run): no orphaned files to reclaim.\n"); + ShowConsoleMsg("ReaSampler prune: no orphaned files to reclaim.\n"); return; } - std::string msg = "ReaSampler prune (dry run): " + std::to_string(report.count) + - " orphaned file(s), " + std::to_string(report.totalBytes) + - " bytes reclaimable. (Dry run -- nothing deleted.)\n"; + // The EXACT set the delete will target — full, untruncated, so what the confirm + // summarises (count + bytes) matches what pruneReclaim reclaims. Captured before the + // confirm so the confirm and the delete reason about the same enumeration. + const std::vector orphanSet = g_session->pruneOrphanSet(); + + // Confirm-with-manifest: count + bytes exact; the file list is the dry-run's 64-capped + // list (the same clip the R2 readout used), with a "N more not shown" tail when clipped. + std::string msg = + "ReaSampler prune will PERMANENTLY reclaim " + std::to_string(report.count) + + " orphaned file(s), freeing " + std::to_string(report.totalBytes) + " bytes.\n\n" + "These files are no longer referenced by any bank and were created by ReaSampler.\n" + "They will be moved to the Recycle Bin on Windows (recoverable), or deleted on " + "other platforms.\n\n"; for (const std::string& rel : report.orphans) msg += " " + rel + "\n"; if (report.truncated) { msg += " ... (" + std::to_string(report.count - report.orphans.size()) + " more not shown)\n"; } - ShowConsoleMsg(msg.c_str()); + msg += "\nReclaim these files now?"; + + const int r = ShowMessageBox(msg.c_str(), "ReaSampler: prune bank folder", 4); + if (r != 6) { // 6 == YES; anything else cancels -> delete NOTHING (SDK ~6544) + ShowConsoleMsg("ReaSampler prune: cancelled -- nothing deleted.\n"); + return; + } + + // Confirmed -> delete exactly the confirmed set (recomputed fresh, stale entries skipped). + const PruneDeletionResult del = g_session->pruneReclaim(orphanSet); + + std::string done = "ReaSampler prune: reclaimed " + + std::to_string(del.reclaimedCount) + " file(s), " + + std::to_string(del.reclaimedBytes) + " bytes" + + (del.usedTrash ? " (to Recycle Bin)" : " (deleted)") + "."; + if (del.skippedCount > 0) { + done += " " + std::to_string(del.skippedCount) + + " file(s) skipped (locked, or changed since the report)."; + } + done += "\n"; + ShowConsoleMsg(done.c_str()); } } // namespace @@ -907,6 +943,8 @@ bool bankHandleCommand(int command) { return false; // not ours — caller's hookcommand keeps looking } +int bankPruneCommandId() { return g_cmdBankPruneFolder; } + void bankUnregisterActions(reaper_plugin_info_t* rec) { // Mirror-unregister with '-'-prefixed strings, reverse of registration order. Each // '-command_id' re-presents the same interned channel-qualified id (channelIdFor). diff --git a/src/actions.h b/src/actions.h index ebe0eab..4ca2a74 100644 --- a/src/actions.h +++ b/src/actions.h @@ -66,6 +66,12 @@ bool bankHandleCommand(int command); // rec==nullptr (before g_session is torn down). void bankUnregisterActions(reaper_plugin_info_t* rec); +// The registered command id for the "Prune bank folder" action (Phase R, R3), or 0 before +// registration. The bank_panel prune button fires the action THROUGH this id via +// Main_OnCommand (fork R-E: the button dispatches the command, it does not call the session +// directly) so the panel affordance and the bindable action share one guarded code path. +int bankPruneCommandId(); + // Persists a completed bank-index verb as a single REAPER undo point (R-B). // Wraps persistBook() (= SetProjExtState) in a Begin/End block with UNDO_STATE_MISCCFG // so the bank op is one Ctrl-Z. On an unsaved / no-active project persistBook() no-ops diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 2e798f9..6275925 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -57,6 +57,7 @@ #include "mode_switch.h" #include "peaks.h" #include "persist.h" +#include "prune_button.h" // footer prune-button layout + hit-test (pure, R3) #include "tab_strip.h" #include "tail_control.h" // TailSetting, cycleTailMode, tailToggleLabel (pure) #include "track_guid.h" // guidString — canonical track GUID key (D2 Wave 2) @@ -102,6 +103,7 @@ #define REAPERAPI_WANT_StopPreview #define REAPERAPI_WANT_GetUserInputs #define REAPERAPI_WANT_ShowMessageBox +#define REAPERAPI_WANT_Main_OnCommand // fire the prune action by command id (R3 button) #define REAPERAPI_WANT_genGuid #define REAPERAPI_WANT_guidToString #include "reaper_plugin_functions.h" @@ -152,6 +154,13 @@ const COLORREF kRgbFooterText = RGB(190, 205, 198); // not an interactive control, so it recedes visually (V3 unobtrusive placement). const COLORREF kRgbFooterVersion = RGB(120, 128, 124); +// Prune button (R3): a raised control in the footer that fires the "Prune bank folder" +// action. A muted warm tone so it reads as a distinct-but-not-alarming affordance (the +// destructive confirm lives behind it, not on the button itself). +const LICE_pixel kColPruneBtnBg = LICE_RGBA(62, 46, 42, 255); +const LICE_pixel kColPruneBtnBorder = LICE_RGBA(96, 72, 66, 255); +const COLORREF kRgbPruneBtnText = RGB(210, 188, 180); + // --- Vertical split + region headers + tab strip (Phase B4) ------------------- // // The client area, top to bottom: mode-switch header (kHeaderHeight) | split body | @@ -554,6 +563,37 @@ void drawTailFooter(LICE_IBitmap* bmp, int w, int h) { DT_RIGHT | DT_VCENTER | DT_SINGLELINE | DT_END_ELLIPSIS); } +// The prune button's rect within the footer, derived from the client size. SINGLE source +// 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. +ButtonRect pruneButtonRectFor(int w, int h) { + const RECT f = panelFooter(w, h); + if (f.top >= f.bottom) return ButtonRect{}; // degenerate footer -> no button + const FooterRect footer{f.left, f.top, f.right - f.left, f.bottom - f.top}; + return computePruneButton(footer, PruneButtonSpec{}); +} + +// Draws the prune button into the footer (called after drawTailFooter fills the strip). +// No-op when the button is suppressed (footer too narrow). READ-ONLY: draws only. +void drawPruneButton(LICE_IBitmap* bmp, int w, int h) { + const ButtonRect b = pruneButtonRectFor(w, h); + if (b.empty()) return; + + LICE_FillRect(bmp, b.x, b.y, b.width, b.height, kColPruneBtnBg, 1.0f, 0); + LICE_DrawRect(bmp, b.x, b.y, b.width, b.height, kColPruneBtnBorder, 1.0f, 0); + + HDC dc = bmp->getDC(); + if (!dc) return; + RECT rc{b.x, b.y, b.x + b.width, b.y + b.height}; + SetBkMode(dc, TRANSPARENT); + SetTextColor(dc, kRgbPruneBtnText); + DrawText(dc, "Prune", -1, &rc, + DT_CENTER | DT_VCENTER | DT_SINGLELINE | DT_END_ELLIPSIS); +} + // True iff client-relative (x, y) falls inside the (non-degenerate) footer strip. // Shared by the footer click (cycle mode) and the scroll-wheel (Manual fine-adjust) // so both agree on the hit target. @@ -902,6 +942,7 @@ void paintPanel(HWND hwnd, HDC hdc) { drawModeSwitch(&bmp, w); drawTailFooter(&bmp, w, h); + drawPruneButton(&bmp, w, h); // R3: raised over the footer strip BitBlt(hdc, 0, 0, w, h, bmp.getDC(), 0, 0, SRCCOPY); } @@ -1596,6 +1637,20 @@ void handleClick(int x, int y) { } } + // Prune button (R3): checked BEFORE the footer's tail-cycle so a click on the button + // fires prune, not a tail cycle. Fires the "Prune bank folder" action THROUGH its + // registered command id (fork R-E: dispatch the command, do not call the session + // directly), so the panel affordance and the bindable action share the one guarded + // dry-run/confirm/delete path in doBankPruneFolder. A 0 id (pre-registration) no-ops. + { + const ButtonRect pb = pruneButtonRectFor(w, h); + if (hitTestPruneButton(x, y, pb)) { + const int cmd = bankPruneCommandId(); + if (cmd != 0) Main_OnCommand(cmd, 0); + return; + } + } + // Tail footer: a click anywhere in the bottom strip cycles the tail mode // (None -> Auto -> Manual -> None) and repaints. It mutates the SESSION's tail // setting (which the capture actions read and persist saves with the project) and diff --git a/src/persist.cpp b/src/persist.cpp index 0d24c2c..7ceb5ab 100644 --- a/src/persist.cpp +++ b/src/persist.cpp @@ -81,6 +81,17 @@ #include #include +// Move-to-trash surface (fork R-C, trash-preferred). On Windows the Recycle Bin is +// reached via SHFileOperationW + FOF_ALLOWUNDO (verified against the Windows SDK +// shellapi.h: SHFILEOPSTRUCTW { hwnd, wFunc, pFrom(double-NUL list), pTo, fFlags, ... }, +// FO_DELETE=0x3, FOF_ALLOWUNDO=0x40). No portable move-to-trash exists on the SWELL +// (macOS/Linux) side of this codebase, so those platforms fall back to unlink behind the +// R3 dry-run/confirm guardrail — see deleteOrphanFile below for the per-platform routing. +#ifdef _WIN32 +#include +#include +#endif + #include "app_version.h" #include "capture_paths.h" #include "prune_reconcile.h" @@ -253,14 +264,32 @@ namespace { // choose its own presentation; this is purely the Wave-2 dry-run readout ceiling. constexpr std::size_t kPruneListDisplayCap = 64; -} // namespace +// A fresh enumerate + pure-core prune compute for the active project. Shared by the +// dry-run report (pruneDryRun), the full-set query (pruneOrphanSet), and the deletion +// (pruneReclaim) so all three agree on ONE resolution + enumeration + set-algebra path +// (no divergence between what is shown and what is deleted). REAPER-facing (resolves the +// active project, enumerates the folder) but writes nothing. +// +// * bankDirAbs — the resolved CURRENT bank folder (absolute, forward-slashed). Empty +// when there is no active/saved project, no project dir, or no folder on +// disk yet -> the caller treats an empty dir as "nothing to reclaim". +// * orphans — the FULL orphan set (owned ∩ present) − referenced, in enumeration +// order, untruncated. The pure core decides; this only supplies inputs. +// * sizeByRel — per-orphan-relative on-disk byte size (0 when it could not be stat'd). +struct PruneScan { + std::string bankDirAbs; + std::vector orphans; + std::unordered_map sizeByRel; +}; -PruneReport ReaSamplerSession::pruneDryRun() const { - PruneReport report; +// Non-throwing: readActiveProject + resolveBankFile are pure/string; every filesystem +// call below uses an error_code form so no std::filesystem_error crosses REAPER's C ABI. +PruneScan scanPruneOrphans(const BankBook& book, const OwnedFileManifest& owned) { + PruneScan scan; std::string rppPath; void* proj = readActiveProject(rppPath); - if (!proj || rppPath.empty()) return report; // no active/saved project -> nothing + if (!proj || rppPath.empty()) return scan; // no active/saved project -> empty scan // Resolve the CURRENT bank folder the same way the index does (M4): project dir of // the live .rpp + the fixed bank subfolder. Never a stored absolute path, so a @@ -268,11 +297,11 @@ PruneReport ReaSamplerSession::pruneDryRun() const { // arithmetic; feeding it the bank subfolder as the "relative path" yields the folder. const std::string projectDir = projectDirOf(rppPath); const std::string bankDir = resolveBankFile(projectDir, kBankSubfolder); - if (bankDir.empty()) return report; // unresolvable (no project dir) -> nothing + if (bankDir.empty()) return scan; // unresolvable (no project dir) -> empty scan std::error_code ec; if (!fs::exists(bankDir, ec) || !fs::is_directory(bankDir, ec)) { - return report; // no bank folder captured yet -> nothing to reclaim + return scan; // no bank folder captured yet -> nothing to reclaim } // Enumerate the folder into project-relative index-spelled paths, spelled the SAME @@ -285,7 +314,6 @@ PruneReport ReaSamplerSession::pruneDryRun() const { // failure (file removed, permission flip) breaks out with a best-effort partial list // rather than propagating std::filesystem_error across REAPER's C ABI. std::vector present; - std::unordered_map sizeByRel; fs::directory_iterator it(bankDir, ec); for (; !ec && it != fs::directory_iterator{}; it.increment(ec)) { const auto& entry = *it; @@ -297,17 +325,126 @@ PruneReport ReaSamplerSession::pruneDryRun() const { present.push_back(rel); std::error_code sz_ec; const std::uintmax_t sz = entry.file_size(sz_ec); - sizeByRel[rel] = sz_ec ? 0 : static_cast(sz); + scan.sizeByRel[rel] = sz_ec ? 0 : static_cast(sz); } - // The decision lives in the pure core — read-only inputs from the session's book and - // manifest (NO save, NO MarkProjectDirty, NO mutation). referencedPaths() unions - // across the whole book (pool included); owned().paths() is the manifest set. The - // count / byte-sum / display-truncation tally is the pure buildPruneReport, so this - // shell only enumerates, resolves, and stats — no report logic re-implemented here. - const std::vector orphans = - pruneOrphans(present, book_.referencedPaths(), owned_.paths()); - return buildPruneReport(orphans, sizeByRel, kPruneListDisplayCap); + // The decision lives in the pure core — read-only inputs from the book and manifest. + // referencedPaths() unions across the whole book (pool included); owned().paths() is + // the manifest set. This shell only enumerates, resolves, and stats. + scan.bankDirAbs = bankDir; + scan.orphans = pruneOrphans(present, book.referencedPaths(), owned.paths()); + return scan; +} + +} // namespace + +PruneReport ReaSamplerSession::pruneDryRun() const { + const PruneScan scan = scanPruneOrphans(book_, owned_); + // buildPruneReport tallies count / byte-sum / display-truncation — no report logic + // re-implemented here. An empty scan (no project / no folder) yields a zero report. + return buildPruneReport(scan.orphans, scan.sizeByRel, kPruneListDisplayCap); +} + +std::vector ReaSamplerSession::pruneOrphanSet() const { + return scanPruneOrphans(book_, owned_).orphans; // FULL set, untruncated +} + +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. +// +// Per-platform routing: +// * Windows — SHFileOperationW(FO_DELETE, pFrom=, FOF_ALLOWUNDO | +// FOF_NOCONFIRMATION | FOF_SILENT | FOF_NOERRORUI). FOF_ALLOWUNDO routes to the +// Recycle Bin (recoverable); the no-UI flags suppress REAPER-blocking dialogs (our +// own confirm already happened). Verified against shellapi.h. `outUsedTrash` set true. +// * 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) { +#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 + // NUL. Backslashes are required (shell APIs reject forward slashes in some cases). + std::string win = absPath; + 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 + std::vector wbuf(static_cast(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] + // makes it the double-NUL-terminated single-element list SHFileOperation wants. + + SHFILEOPSTRUCTW op{}; + op.hwnd = nullptr; + op.wFunc = FO_DELETE; + op.pFrom = wbuf.data(); + op.pTo = nullptr; + op.fFlags = static_cast(FOF_ALLOWUNDO | FOF_NOCONFIRMATION | + FOF_SILENT | FOF_NOERRORUI); + const int rv = SHFileOperationW(&op); + if (rv == 0 && !op.fAnyOperationsAborted) { + outUsedTrash = true; + return true; + } + // 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. + std::error_code ec; + return !fs::exists(absPath, ec); +#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 (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); +#endif +} + +} // namespace + +PruneDeletionResult ReaSamplerSession::pruneReclaim( + const std::vector& confirmed) const { + PruneDeletionResult result; + + // Re-enumerate + run the pure core FRESH (never a stale set): the deletion targets + // exactly `confirmed ∩ freshOrphans` (pruneDeletePlan). A file that vanished or became + // referenced between confirm and delete drops out of freshOrphans and is skipped; a + // newly-appeared orphan not in `confirmed` is never swept without its own confirm. + // Because freshOrphans is itself a pure-core output, the plan can contain NO referenced + // and NO hand-dropped file — the R-C/R-D safety survives the recompute. + const PruneScan scan = scanPruneOrphans(book_, owned_); + if (scan.bankDirAbs.empty()) return result; // no project / no folder -> nothing + + const std::vector 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(); + + for (const std::string& rel : plan) { + // Reconstruct the absolute path from the resolved bank dir + the entry's file name. + // rel is index-spelled "/"; the name is the tail after '/'. + const std::string::size_type slash = rel.find_last_of('/'); + const std::string name = (slash == std::string::npos) ? rel : rel.substr(slash + 1); + if (name.empty()) { ++result.skippedCount; continue; } + const std::string absPath = scan.bankDirAbs + "/" + name; + + const auto szIt = scan.sizeByRel.find(rel); + const std::uint64_t bytes = (szIt != scan.sizeByRel.end()) ? szIt->second : 0; + + if (deleteOrphanFile(absPath, result.usedTrash)) { + ++result.reclaimedCount; + result.reclaimedBytes += bytes; + } else { + ++result.skippedCount; // locked / conversion failure -> recorded, not thrown + } + } + return result; } namespace { diff --git a/src/persist.h b/src/persist.h index 72ccac2..b4f1c10 100644 --- a/src/persist.h +++ b/src/persist.h @@ -195,6 +195,38 @@ public: // folder on disk yet — an unsaved or never-captured project has nothing to reclaim. PruneReport pruneDryRun() const; + // The FULL (untruncated) prune orphan set for the ACTIVE project — the same fresh + // enumerate + pure-core compute pruneDryRun() runs, but returning EVERY orphan (no + // 64-cap display clip) as project-relative index-spelled paths, in enumeration order. + // The R3 action calls this to obtain the exact set it will CONFIRM and then delete + // (pruneDryRun's truncated list is for the console readout; the delete set must be + // complete). READ-ONLY — no ext-state, no save, no file mutation. Empty when there is + // no active/saved project or no bank folder yet. + std::vector pruneOrphanSet() const; + + // Phase R (Reclaim), R3: DELETE the confirmed orphan set — the SOLE file-deletion path + // in ReaSampler, callable ONLY after an explicit user confirm of a specific manifest. + // Given the orphan set the user was shown and confirmed (`confirmed`, typically the + // full pruneOrphanSet() captured moments earlier), this re-enumerates the folder, runs + // the pure core FRESH, and deletes exactly `confirmed ∩ freshOrphans` (pruneDeletePlan) + // so a file that vanished or became referenced between confirm and delete is skipped, + // never wrongly deleted — and a newly-appeared orphan the user did NOT see is never + // swept. Deletion routes to the OS trash where a portable move-to-trash is verified + // (Windows Recycle Bin via SHFileOperation + FOF_ALLOWUNDO); elsewhere it falls back to + // std::filesystem unlink behind this confirm guardrail (see persist.cpp for per-platform + // routing). Non-throwing: every filesystem call uses error_code forms; a per-file + // failure (locked, already gone) is recorded and skipped, never thrown across the C ABI. + // + // Does NOT modify the BankIndex/book (orphans are unreferenced by definition) and does + // NOT modify the OwnedFileManifest (a reclaimed file drops out of the (owned ∩ present) + // algebra naturally once it is off disk — no persist write, so no undo-point question + // and no risk to the referenced/owned safety). Writes NO ext-state at all. + // + // No-ops (empty result) when there is no active/saved project, no bank folder, or the + // delete plan is empty (everything went stale). The caller is responsible for having + // shown the confirm; this method does NOT prompt. + PruneDeletionResult pruneReclaim(const std::vector& confirmed) const; + // Poll the active project. Detects a project load (active project changed) // and a Save-As (active project's .rpp path changed) and reacts accordingly. // Intended to be driven by REAPER's "timer" register. Idempotent per tick. diff --git a/src/prune_button.cpp b/src/prune_button.cpp new file mode 100644 index 0000000..424c327 --- /dev/null +++ b/src/prune_button.cpp @@ -0,0 +1,39 @@ +#include "prune_button.h" + +// prune_button implementation — right-anchored button placement in the footer strip, +// with a left-collision suppression rule. Trivially auditable arithmetic; the safety +// property (a suppressed/empty button never claims a click) is a pure predicate tested +// outside the DAW. + +namespace reasampler { + +ButtonRect computePruneButton(const FooterRect& footer, const PruneButtonSpec& spec) { + if (footer.width <= 0 || footer.height <= 0) return ButtonRect{}; // degenerate footer + if (spec.buttonWidth <= 0) return ButtonRect{}; // nothing to place + + // Right-anchored: right edge inset from the footer's right; width fixed. + const int right = footer.x + footer.width - spec.rightInset; + const int left = right - spec.buttonWidth; + + // Suppress if the button would encroach past the reserved left inset (tail label room) + // or spill off the left of the footer entirely. + if (left < footer.x + spec.minLeftInset) return ButtonRect{}; + + // Vertically centred by the inset; clamp so a thin footer never yields a negative height. + int top = footer.y + spec.verticalInset; + int height = footer.height - 2 * spec.verticalInset; + if (height <= 0) { + top = footer.y; + height = footer.height; + } + + return ButtonRect{left, top, spec.buttonWidth, height}; +} + +bool hitTestPruneButton(int px, int py, const ButtonRect& button) { + if (button.empty()) return false; // suppressed button claims nothing + return px >= button.x && px < button.x + button.width && + py >= button.y && py < button.y + button.height; +} + +} // namespace reasampler diff --git a/src/prune_button.h b/src/prune_button.h new file mode 100644 index 0000000..d58eec0 --- /dev/null +++ b/src/prune_button.h @@ -0,0 +1,91 @@ +#pragma once +// prune_button — the REAPER-free layout math behind the bank_panel's Prune button +// (Phase R, Wave 3 — R3, fork R-E). A single labelled button drawn in the panel's +// tail-footer strip that fires the "Prune bank folder" command. The panel shell +// (bank_panel.cpp) owns the SWELL window, LICE drawing, and the Main_OnCommand +// dispatch of the registered command id — all REAPER-bound, DAW-verified. What is +// NOT DAW-bound — WHERE the button sits in the footer and whether a click lands on +// it — lives here so it is unit-tested outside the DAW (CLAUDE.md §load-bearing +// split). Mirror of mode_switch / tab_strip. +// +// PURE MODULE: NO REAPER types, NO SWELL, NO vendor/ includes. Standard library +// only. Builds and unit-tests without REAPER. +// +// -- Placement contract -------------------------------------------------------- +// +// The footer already hosts a LEFT-aligned tail-mode label and a RIGHT-aligned +// version readout (bank_panel drawTailFooter). The prune button is a fixed-width +// button anchored to the RIGHT of the footer, inset from the right edge, sitting +// just LEFT of the version readout's inset region. It never overlaps the tail label +// at the left. When the footer is too narrow to fit the button without colliding +// with the left inset, the button is suppressed (empty rect) rather than drawn on +// top of the label — the action is always reachable via its bindable command, so a +// hidden button is a graceful degradation, not a lost affordance. + +namespace reasampler { + +// The footer strip the button is drawn into, top-left origin (SWELL/LICE +// convention). (x, y) is the top-left corner; width/height are the strip extents. +// bank_panel derives this from panelFooter() and passes it here. +struct FooterRect { + int x = 0; + int y = 0; + int width = 0; + int height = 0; + + bool operator==(const FooterRect& o) const { + return x == o.x && y == o.y && width == o.width && height == o.height; + } +}; + +// A button's pixel rectangle within the footer, top-left origin. A zero-area rect +// (width <= 0 or height <= 0) means "no button" — the footer is too narrow to place +// it, or the footer itself is degenerate; the caller must not draw or hit-test it. +struct ButtonRect { + int x = 0; + int y = 0; + int width = 0; + int height = 0; + + bool empty() const { return width <= 0 || height <= 0; } + + bool operator==(const ButtonRect& o) const { + return x == o.x && y == o.y && width == o.width && height == o.height; + } +}; + +// Layout inputs for the prune button, in pixels. Defaults match the bank_panel footer +// metrics; the shell passes its own so draw and hit-test share one source of truth. +// * 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). +// * 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 +// left edge (reserving room for the left-aligned tail label). If the +// button would encroach past this, computePruneButton yields an empty +// rect (button suppressed — see header placement contract). +struct PruneButtonSpec { + int buttonWidth = 72; + int rightInset = 84; + int verticalInset = 4; + int minLeftInset = 120; +}; + +// Computes the prune button's rect within `footer` per `spec`. Right-anchored: the +// button's right edge is footer.x + footer.width - rightInset, its width is buttonWidth, +// and it is vertically centred by verticalInset. Returns an EMPTY rect (button +// suppressed) when: the footer is degenerate (width/height <= 0), OR the resulting left +// edge would fall closer to the footer left than minLeftInset (too narrow to place +// without colliding with the tail label). The action stays reachable via its command in +// that case — a suppressed button is graceful, not a lost feature. +ButtonRect computePruneButton(const FooterRect& footer, const PruneButtonSpec& spec); + +// True iff the point (px, py) (SWELL/LICE top-left client coords) falls inside `button`. +// Half-open bounds [x, x+width) x [y, y+height) — matches computePruneButton so draw and +// hit-test agree on the same pixels. An empty button never claims a point (always false), +// so a suppressed button cannot be accidentally clicked. +bool hitTestPruneButton(int px, int py, const ButtonRect& button); + +} // namespace reasampler diff --git a/src/prune_reconcile.cpp b/src/prune_reconcile.cpp index 9a21d36..2d79670 100644 --- a/src/prune_reconcile.cpp +++ b/src/prune_reconcile.cpp @@ -52,4 +52,21 @@ PruneReport buildPruneReport( return report; } +std::vector pruneDeletePlan(const std::vector& confirmed, + const std::vector& freshOrphans) { + // fresh is a pure-core output (referenced/hand-dropped already excluded), so keeping a + // confirmed path iff it is still a fresh orphan can never re-admit an unsafe file. Walk + // `confirmed` to preserve the confirm's listing order; emitted de-dups repeats. + const std::unordered_set freshSet(freshOrphans.begin(), freshOrphans.end()); + + std::vector plan; + std::unordered_set emitted; + for (const std::string& path : confirmed) { + if (freshSet.count(path) == 0) continue; // stale: vanished / now-referenced -> skip + if (!emitted.insert(path).second) continue; // already emitted + plan.push_back(path); + } + return plan; +} + } // namespace reasampler diff --git a/src/prune_reconcile.h b/src/prune_reconcile.h index 58d83b7..99f7e2e 100644 --- a/src/prune_reconcile.h +++ b/src/prune_reconcile.h @@ -71,6 +71,26 @@ struct PruneReport { bool truncated = false; }; +// The outcome of an actual prune DELETION (Phase R, Wave 3 — R3). The prune shell fills +// this as it deletes the confirmed orphan set, reporting what it ACTUALLY reclaimed (not +// what it intended to) so a locked/vanished file shows up as a skip, never a false claim. +// 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). +// * 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. +// * 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 { + std::size_t reclaimedCount = 0; + std::uint64_t reclaimedBytes = 0; + std::size_t skippedCount = 0; + bool usedTrash = false; +}; + // Computes the prune orphan set: (owned ∩ present) − referenced. // // Returns the subset of `present` that is BOTH owned AND unreferenced, in the ORDER @@ -105,4 +125,24 @@ PruneReport buildPruneReport(const std::vector& orphans, const std::unordered_map& sizeByPath, std::size_t displayCap); +// Computes the confirm-time delete plan: the intersection of the set the user was SHOWN +// and confirmed (`confirmed`) with a FRESH pure-core orphan output (`freshOrphans`) taken +// at delete time. Returns exactly `confirmed ∩ freshOrphans`, in the order of `confirmed` +// (deterministic — the same order the confirm listed). +// +// This is the R3 staleness guard, and it protects in BOTH directions so that "what was +// shown is what is deleted" holds no matter what changed between confirm and delete: +// * A confirmed path that is NO LONGER a fresh orphan — a file that vanished (gone from +// `present`), or that some bank now references (gone from `− referenced`), or whose +// ownership changed — is DROPPED (a skip, never an error, never a wrongful delete of a +// now-referenced file). Because `freshOrphans` is itself a pure-core output, the plan +// can never contain a referenced or hand-dropped file: the guard survives recompute. +// * A path that became an orphan AFTER the confirm (in `freshOrphans` but not `confirmed`) +// is NOT deleted — it was never shown, so it is never swept without its own confirm. +// +// PURE: no I/O, no REAPER. Duplicate spellings in `confirmed` are de-duplicated in the +// result (mirrors pruneOrphans; a confirmed set from a real scan holds distinct names). +std::vector pruneDeletePlan(const std::vector& confirmed, + const std::vector& freshOrphans); + } // namespace reasampler diff --git a/tests/test_prune_button.cpp b/tests/test_prune_button.cpp new file mode 100644 index 0000000..19ff0aa --- /dev/null +++ b/tests/test_prune_button.cpp @@ -0,0 +1,132 @@ +// Standalone tests for reasampler::prune_button — no REAPER, no test framework. +// Same fast loop as the sibling pure tests (mode_switch / tab_strip): assert the +// footer prune-button placement math and hit-testing directly. +// +// Covers (R3 brief §button pure module): right-anchored layout in a wide footer; +// vertical inset; SUPPRESSION (empty rect) when the footer is too narrow to clear the +// tail-label inset or is degenerate; hit-test in/out/edge (half-open bounds); a +// suppressed/empty button claims no point; draw and hit-test agree over the whole rect. + +#include "../src/prune_button.h" + +#include + +using namespace reasampler; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +// --- Layout: wide footer, right-anchored ------------------------------------- + +// Footer 400 wide at origin (0, 100), height 26. Default spec: buttonWidth 72, +// rightInset 84, verticalInset 4, minLeftInset 120. Right edge = 0+400-84 = 316, +// left = 316-72 = 244 (>= 0+120, so placed). Top = 100+4 = 104, height = 26-8 = 18. +static void testWideFooterRightAnchored() { + FooterRect f{0, 100, 400, 26}; + const ButtonRect b = computePruneButton(f, PruneButtonSpec{}); + CHECK(!b.empty()); + CHECK((b == ButtonRect{244, 104, 72, 18})); + // Right edge sits at the rightInset from the footer's right. + CHECK(b.x + b.width == f.x + f.width - 84); + // Left edge clears the reserved tail-label inset. + CHECK(b.x >= f.x + 120); +} + +// Origin offset is honoured (button anchors to THIS footer's right, not 0). +static void testOffsetFooterAnchors() { + FooterRect f{10, 200, 400, 26}; + const ButtonRect b = computePruneButton(f, PruneButtonSpec{}); + CHECK(!b.empty()); + CHECK(b.x + b.width == f.x + f.width - 84); // = 10+400-84 = 326 + CHECK(b.x == 254); +} + +// --- Suppression: too narrow / degenerate ------------------------------------ + +// A footer just wide enough that the button's left edge would fall past the +// minLeftInset is suppressed (empty). left = x + width - rightInset - buttonWidth. +// Need left < x + minLeftInset -> width < rightInset + buttonWidth + minLeftInset +// = 84 + 72 + 120 = 276. Width 275 suppresses; 276 places (boundary). +static void testNarrowFooterSuppressed() { + CHECK(computePruneButton(FooterRect{0, 0, 275, 26}, PruneButtonSpec{}).empty()); + CHECK(!computePruneButton(FooterRect{0, 0, 276, 26}, PruneButtonSpec{}).empty()); +} + +static void testDegenerateFooterSuppressed() { + CHECK(computePruneButton(FooterRect{0, 0, 0, 26}, PruneButtonSpec{}).empty()); // no width + CHECK(computePruneButton(FooterRect{0, 0, 400, 0}, PruneButtonSpec{}).empty()); // no height + PruneButtonSpec zero{}; zero.buttonWidth = 0; + CHECK(computePruneButton(FooterRect{0, 0, 400, 26}, zero).empty()); // zero button +} + +// A very thin footer (height <= 2*verticalInset) still places a button but clamps its +// height to the footer's own, rather than yielding a negative height. +static void testThinFooterClampsHeight() { + FooterRect f{0, 0, 400, 6}; // 6 <= 2*4, so height would be negative -> clamp + const ButtonRect b = computePruneButton(f, PruneButtonSpec{}); + CHECK(!b.empty()); + CHECK(b.y == f.y); + CHECK(b.height == f.height); +} + +// --- Hit-test ---------------------------------------------------------------- + +static void testHitTestInside() { + FooterRect f{0, 100, 400, 26}; + const ButtonRect b = computePruneButton(f, PruneButtonSpec{}); // {244,104,72,18} + CHECK(hitTestPruneButton(b.x, b.y, b)); // top-left corner (inclusive) + CHECK(hitTestPruneButton(b.x + b.width - 1, b.y + b.height - 1, b)); // bottom-right inclusive + CHECK(hitTestPruneButton(b.x + b.width / 2, b.y + b.height / 2, b)); // centre +} + +// Half-open bounds: the far edges (x+width, y+height) are EXCLUDED, matching the draw. +static void testHitTestEdgesExcluded() { + FooterRect f{0, 100, 400, 26}; + const ButtonRect b = computePruneButton(f, PruneButtonSpec{}); + CHECK(!hitTestPruneButton(b.x - 1, b.y, b)); // just left + CHECK(!hitTestPruneButton(b.x + b.width, b.y, b)); // right edge excluded + CHECK(!hitTestPruneButton(b.x, b.y - 1, b)); // just above + CHECK(!hitTestPruneButton(b.x, b.y + b.height, b)); // bottom edge excluded +} + +// A suppressed (empty) button never claims a point — a click in the footer where the +// button would have been falls through to the tail cycle, never a phantom prune. +static void testEmptyButtonClaimsNothing() { + ButtonRect empty{}; + CHECK(!hitTestPruneButton(0, 0, empty)); + CHECK(!hitTestPruneButton(5, 5, empty)); + // A "no button" from a narrow footer also claims nothing at any point. + const ButtonRect suppressed = computePruneButton(FooterRect{0, 0, 200, 26}, PruneButtonSpec{}); + CHECK(suppressed.empty()); + CHECK(!hitTestPruneButton(150, 13, suppressed)); +} + +// Draw/hit-test agreement: every point inside the computed rect hit-tests true, and the +// four immediate outside neighbours hit-test false (the load-bearing consistency). +static void testHitTestMatchesLayout() { + FooterRect f{3, 7, 377, 22}; // awkward origin/size + const ButtonRect b = computePruneButton(f, PruneButtonSpec{}); + CHECK(!b.empty()); + for (int py = b.y; py < b.y + b.height; ++py) + for (int px = b.x; px < b.x + b.width; ++px) + CHECK(hitTestPruneButton(px, py, b)); + CHECK(!hitTestPruneButton(b.x - 1, b.y, b)); + CHECK(!hitTestPruneButton(b.x + b.width, b.y, b)); +} + +int main() { + testWideFooterRightAnchored(); + testOffsetFooterAnchors(); + testNarrowFooterSuppressed(); + testDegenerateFooterSuppressed(); + testThinFooterClampsHeight(); + testHitTestInside(); + testHitTestEdgesExcluded(); + testEmptyButtonClaimsNothing(); + testHitTestMatchesLayout(); + + if (g_fail == 0) std::printf("prune_button: all tests passed\n"); + else std::printf("prune_button: %d CHECK(s) FAILED\n", g_fail); + return g_fail == 0 ? 0 : 1; +} diff --git a/tests/test_prune_reconcile.cpp b/tests/test_prune_reconcile.cpp index cb547cd..fe48216 100644 --- a/tests/test_prune_reconcile.cpp +++ b/tests/test_prune_reconcile.cpp @@ -324,6 +324,65 @@ static void testReportEmptyOrphanSet() { CHECK(!r.truncated); } +// --- pruneDeletePlan (R3 staleness guard) ------------------------------------ +// +// plan = confirmed ∩ freshOrphans, in confirmed order. Guards BOTH directions so +// "what was shown is what is deleted" holds after a recompute at delete time. + +// No change between confirm and delete: the plan is the confirmed set exactly, in order. +static void testDeletePlanStableEqualsConfirmed() { + const std::vector confirmed{"b/a.wav", "b/b.wav", "b/c.wav"}; + const std::vector fresh{"b/a.wav", "b/b.wav", "b/c.wav"}; + const std::vector plan = pruneDeletePlan(confirmed, fresh); + CHECK((plan == confirmed)); // exact set AND order +} + +// A confirmed file that VANISHED (gone from fresh present -> not a fresh orphan) is a +// skip: it drops out of the plan. Would FAIL if the plan ignored freshOrphans. +static void testDeletePlanSkipsVanishedFile() { + const std::vector confirmed{"b/a.wav", "b/gone.wav", "b/c.wav"}; + const std::vector fresh{"b/a.wav", "b/c.wav"}; // gone.wav disappeared + const std::vector plan = pruneDeletePlan(confirmed, fresh); + CHECK((plan == std::vector{"b/a.wav", "b/c.wav"})); + CHECK(!contains(plan, "b/gone.wav")); +} + +// A confirmed file that became REFERENCED between confirm and delete drops OUT of the +// fresh orphan set (pruneOrphans excludes it), so the plan skips it — never deletes a +// now-referenced file. Modelled here as its absence from `fresh`. Load-bearing safety. +static void testDeletePlanSkipsNowReferencedFile() { + const std::vector confirmed{"b/x.wav", "b/y.wav"}; + const std::vector fresh{"b/x.wav"}; // y.wav now referenced -> not a fresh orphan + const std::vector plan = pruneDeletePlan(confirmed, fresh); + CHECK((plan == std::vector{"b/x.wav"})); +} + +// A NEWLY-APPEARED orphan (in fresh, NOT in confirmed) is NEVER swept: it was not shown, +// so it must not be deleted without its own confirm. Would FAIL if plan = fresh. +static void testDeletePlanNeverSweepsUnconfirmed() { + const std::vector confirmed{"b/a.wav"}; + const std::vector fresh{"b/a.wav", "b/new_orphan.wav"}; + const std::vector plan = pruneDeletePlan(confirmed, fresh); + CHECK((plan == std::vector{"b/a.wav"})); + CHECK(!contains(plan, "b/new_orphan.wav")); +} + +// Empty inputs: an empty confirmed (nothing shown) -> empty plan regardless of fresh; an +// empty fresh (everything went stale) -> empty plan (all skipped). +static void testDeletePlanEmptyInputs() { + CHECK(pruneDeletePlan({}, {"b/a.wav"}).empty()); + CHECK(pruneDeletePlan({"b/a.wav"}, {}).empty()); + CHECK(pruneDeletePlan({}, {}).empty()); +} + +// Duplicate spellings in confirmed are de-duplicated in the plan (mirror of pruneOrphans). +static void testDeletePlanDeduplicatesConfirmed() { + const std::vector confirmed{"b/a.wav", "b/a.wav", "b/b.wav"}; + const std::vector fresh{"b/a.wav", "b/b.wav"}; + const std::vector plan = pruneDeletePlan(confirmed, fresh); + CHECK((plan == std::vector{"b/a.wav", "b/b.wav"})); +} + int main() { testNullTestAllReferenced(); testFormulaMixedPopulations(); @@ -346,6 +405,12 @@ int main() { testReportTruncatesListButKeepsExactCountAndSize(); testReportUncappedWhenCapZero(); testReportEmptyOrphanSet(); + testDeletePlanStableEqualsConfirmed(); + testDeletePlanSkipsVanishedFile(); + testDeletePlanSkipsNowReferencedFile(); + testDeletePlanNeverSweepsUnconfirmed(); + testDeletePlanEmptyInputs(); + testDeletePlanDeduplicatesConfirmed(); if (g_fail == 0) std::printf("prune_reconcile_tests: ALL PASS\n"); else std::printf("prune_reconcile_tests: %d FAILURE(S)\n", g_fail); From ef2d3fa846d94286104f4401f003526b2bdff533 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 26 Jul 2026 18:59:30 -0400 Subject: [PATCH 2/2] 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. --- src/bank_panel.cpp | 10 ++++++--- src/persist.cpp | 47 +++++++++++++++++++++++++++++++------------ src/prune_button.h | 9 +++++++-- src/prune_reconcile.h | 10 +++++---- 4 files changed, 54 insertions(+), 22 deletions(-) diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 6275925..30e6d47 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -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 diff --git a/src/persist.cpp b/src/persist.cpp index 7ceb5ab..dbe914e 100644 --- a/src/persist.cpp +++ b/src/persist.cpp @@ -79,6 +79,7 @@ #include #include #include +#include #include // Move-to-trash surface (fork R-C, trash-preferred). On Windows the Recycle Bin is @@ -352,9 +353,13 @@ std::vector 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=, 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 wbuf(static_cast(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 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(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 } diff --git a/src/prune_button.h b/src/prune_button.h index d58eec0..7409884 100644 --- a/src/prune_button.h +++ b/src/prune_button.h @@ -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; }; diff --git a/src/prune_reconcile.h b/src/prune_reconcile.h index 99f7e2e..c65ce3a 100644 --- a/src/prune_reconcile.h +++ b/src/prune_reconcile.h @@ -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 {