daniel/reasampler
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ux: drop last-reference confirm from sample remove; silent remove via undo (R-B)

Browse Source
This commit is contained in:
Daniel Harvey
2026-07-26 17:27:05 -04:00
parent 6199a4868d
commit ff4eb26eab
5 changed files with 30 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files
COMPLETED.md
+4 -5
View File
@@ -947,9 +947,8 @@ timeline item).
stays a latent seam-only parameter, not shipped).
- [x] `bank_panel` remove affordance on the current selection (reuse M5 selection
model, as move/copy do).
- [x] Confirm-on-last-reference guardrail: remove that orphans a file (no other
bank references it) confirms, naming the orphaned-until-prune consequence;
remove of a still-referenced sample does not confirm.
- [x] Silent remove: no confirm dialog; recoverability via batched REAPER undo
(R-B) — one Ctrl-Z restores the index entry; files are never deleted by remove.
- [x] Tests: remove drops the target entry; same-hash entry in another bank
survives; remove-from-pool allowed; last-reference remove leaves an orphan (file
untouched); non-destructive (no file/timeline mutation).
@@ -958,8 +957,8 @@ timeline item).
- **R-A — remove scope.** Settled: **this-bank**. Removes the entry from the bank in
view only; the `scope: this-bank | all-banks` seam stays in the action signature
but all-banks is a latent parameter, not a surfaced verb.
- `hashReferencedElsewhere` cross-bank reference query added to `bank_book` to
support the confirm-on-last-reference guardrail without coupling to the panel.
- `hashReferencedElsewhere` cross-bank reference query in `bank_book` retained as
a tested model API for Phase R prune; the remove shells no longer call it.
- Both `bank_panel` context menu ("Remove selected sample(s)") and Delete key
affordance wired; panel gesture is also one batched undo point (R-B applies).
CONTEXT.md
+5 -6
View File
@@ -797,12 +797,11 @@ model capability.
## Guardrails
- **Confirm on last-reference remove; don't confirm otherwise.** A remove that
drops the *last* index reference to a file orphans it (until prune) — confirm
that case, naming the consequence ("…its file remains on disk until pruned"). A
remove of a sample still referenced by another bank is cheap and re-derivable
(re-copy it back) and needs no confirmation. The confirmation is *earned by
actual orphan risk*, not fired on every remove.
- **Removes are silent — no confirm dialog.** Recoverability is provided by the
batched REAPER undo (R-B): one Ctrl-Z restores the index entry, whether or not
the sample was a last reference. Files are never deleted by remove
(orphaned-until-prune is unchanged). `hashReferencedElsewhere` is a tested
model API retained for Phase R prune; it has no shell caller in the remove path.
- **Undo (fork R-B, SETTLED 2026-07-24 — batched REAPER undo points, Phase-B-wide).**
Bank/index mutations integrate into REAPER's undo system as **batched undo points**
(`Undo_BeginBlock` / `Undo_EndBlock`): the related index mutations of one bank
docs/product/removal-and-prune.md
+8 -9
View File
@@ -95,14 +95,13 @@ the bytes survive on disk until an explicit prune. So the recovery story is: an
accidental remove loses the *index entry*, not the audio. But there are two
sharpnesses to guard:
- **Last-reference remove is the orphan-maker.** Removing a sample that exists in
only one bank orphans its file (until prune). This is the same footgun as
non-empty delete-bank, and it deserves the same treatment: **confirm when the
remove drops the last index reference** ("Remove 'kick_03'? It is in no other
bank — its file will remain on disk until pruned."). A remove of a sample that
still lives in another bank is cheap and reversible-in-spirit (re-copy it back)
and need not confirm. This makes the confirmation *earned* by actual risk rather
than fired on every remove.
- **Removes are silent — no confirm dialog.** Recoverability is provided by the
batched REAPER undo (R-B): one Ctrl-Z restores the index entry. A last-reference
remove does orphan the file (no other bank holds it), but the file stays on disk
until an explicit prune — remove never deletes bytes. The undo path makes the
confirm unnecessary; the file-safety guarantee (orphaned-until-prune) remains
unchanged. `hashReferencedElsewhere` is a tested model API retained for Phase R
prune; it has no shell caller in the remove path.
- **Undo.** REAPER's own undo stack does not natively cover ext-state index
mutations, so this is a Phase-B-wide decision (fork R-B, **settled**): bank/index
mutations integrate into REAPER's undo system as **batched undo points**
@@ -361,4 +360,4 @@ settled spec (explicitly out of scope). A periodic/background sweep remains reje
| Deletes bytes? | no | no | **yes (only op that does)** |
| Produces orphans? | yes (last-ref) | yes (non-empty) | — (it *reclaims* them) |
| Reversible? | Ctrl-Z (batched undo, R-B) / re-capture | Ctrl-Z (batched undo, R-B) / re-create | **no in-app** (recoverable via OS trash, R-C) |
| Guardrail | confirm on last-ref | confirm on non-empty | dry-run + manifest confirm |
| Guardrail | silent (Ctrl-Z restores; files never deleted by remove) | confirm on non-empty | dry-run + manifest confirm |
src/actions.cpp
+6 -35
View File
@@ -758,12 +758,10 @@ void doBankTransferSelected(bool copy) {
// SCOPE (fork R-A): this-bank only — the sole surfaced verb. The RemoveScope::AllBanks
// seam stays latent in the model; nothing here reaches for it.
//
// CONFIRM-ON-LAST-REFERENCE (guardrail): a remove that would orphan a file (no OTHER
// bank references its content hash after the remove) earns a confirm; a remove of a
// still-referenced sample does not. BATCH UX: for a multi-select we compute the
// last-reference set BEFORE mutating (removal changes the reference graph), then fire a
// SINGLE confirm summarizing the N that would orphan — not one dialog per sample. If
// none would orphan, no confirm fires at all (the confirm is earned by actual risk).
// SILENT REMOVE: removes proceed without a confirm dialog. Recoverability is provided
// by the batched REAPER undo (R-B) — one Ctrl-Z restores the index entry. Files are
// never deleted by remove (orphaned-until-prune is unchanged). hashReferencedElsewhere
// is a tested model API retained for Phase R prune; it has no shell caller here.
void doBankRemoveSelected() {
const std::vector<std::string> selected = bankPanelSelectedSampleIds();
if (selected.empty()) {
@@ -772,41 +770,14 @@ void doBankRemoveSelected() {
}
const std::string srcId = bankPanelSelectedSourceBankId();
BankBook& book = g_session->book();
const Bank* src = book.bank(srcId);
if (src == nullptr) {
if (book.bank(srcId) == nullptr) {
ShowConsoleMsg("ReaSampler: the selection's bank no longer exists.\n");
return;
}
// Count the samples whose file this remove would orphan — computed on the CURRENT
// (pre-mutation) reference graph so a same-hash sibling in another bank counts as a
// surviving reference. Resolve by id against the live source index (ids, not cached
// refs); an id no longer present is skipped (it removes to a no-op below).
int orphanCount = 0;
for (const std::string& sampleId : selected) {
const Sample* s = src->index.query(sampleId);
if (s == nullptr) continue; // already gone; not a last-reference orphan
if (!book.hashReferencedElsewhere(s->contentHash, srcId)) ++orphanCount;
}
if (orphanCount > 0) {
const std::string msg =
std::to_string(orphanCount) +
(orphanCount == 1 ? " selected sample is" : " selected samples are") +
" in no other bank.\n\nRemoving " +
(orphanCount == 1 ? "it" : "them") +
" drops the index entry only -- the file stays on disk until you prune "
"(it is never deleted by remove).\n\nRemove anyway?";
const int r = ShowMessageBox(msg.c_str(),
"ReaSampler: remove last-reference sample(s)", 4);
if (r != 6) return; // 6 == YES; anything else cancels (SDK ~6544)
}
// Perform the removes (this-bank scope). Pass ids by value — no BankIndex& is cached
// across the loop's mutations. Count real drops so the no-op guardrail can skip the
// undo point when nothing was removed (every id was already absent). The per-outcome
// console summary was dropped (m11 chatter policy); only the "did anything change?"
// signal the undo guardrail needs is retained.
// undo point when nothing was removed (every id was already absent).
int removed = 0;
for (const std::string& sampleId : selected) {
if (book.removeSample(sampleId, srcId, RemoveScope::ThisBank) ==
src/bank_panel.cpp
+7 -31
View File
@@ -1365,38 +1365,14 @@ void transferSamples(const std::vector<std::string>& sampleIds,
// Remove `sampleIds` from `srcBankId` (index-only, this-bank scope). Non-destructive to
// the file: a last-reference remove leaves the file on disk, orphaned until Phase R
// prune — remove NEVER deletes bytes (the manifest is untouched). Confirm-on-last-
// reference guardrail: a single confirm summarizing the N whose files this would orphan
// (computed on the PRE-mutation reference graph), fired only when at least one would
// orphan. Ids passed by value — no BankIndex& cached across the loop's mutations.
// prune — remove NEVER deletes bytes (the manifest is untouched). Removes are silent
// (no confirm dialog); recoverability is provided by the batched REAPER undo (R-B) —
// one Ctrl-Z restores the index entry. Ids passed by value — no BankIndex& cached
// across the loop's mutations.
void removeSamples(const std::vector<std::string>& sampleIds,
const std::string& srcBankId) {
if (!book() || sampleIds.empty()) return;
const Bank* src = book()->bank(srcBankId);
if (!src) return;
// Count files this remove would orphan — computed BEFORE mutating, so a same-hash
// sibling in another bank counts as a surviving reference.
int orphanCount = 0;
for (const std::string& sid : sampleIds) {
const Sample* s = src->index.query(sid);
if (!s) continue; // already gone; not a last-reference orphan
if (!book()->hashReferencedElsewhere(s->contentHash, srcBankId)) ++orphanCount;
}
if (orphanCount > 0) {
const std::string msg =
std::to_string(orphanCount) +
(orphanCount == 1 ? " selected sample is" : " selected samples are") +
" in no other bank.\n\nRemoving " +
(orphanCount == 1 ? "it" : "them") +
" drops the index entry only -- the file stays on disk until you prune "
"(it is never deleted by remove).\n\nRemove anyway?";
// 4 == MB_YESNO. 6=Yes (SDK); anything else cancels.
const int r = ShowMessageBox(msg.c_str(),
"ReaSampler: remove last-reference sample(s)", 4);
if (r != 6) return;
}
if (!book()->bank(srcBankId)) return;
int removed = 0;
for (const std::string& sid : sampleIds)
@@ -1771,8 +1747,8 @@ bool handleKey(int vk) {
stopAudition();
return true;
case VK_DELETE: {
// Remove the focused-region selection (B5). Same confirm-on-last-reference
// path the context-menu "Remove" uses; a no-op when nothing is selected.
// Remove the focused-region selection (B5). Silent; a no-op when nothing
// is selected.
const std::vector<std::string> sel = focusedSelectionIds();
if (sel.empty()) return false; // nothing selected — let the key fall through
removeSamples(sel, bankIdForRegion(g_panel.focusedRegion));