Merge panel-undo pass: panel gestures get R-B undo points

src/actions.cpp

@@ -476,22 +476,6 @@ bool persistBook() { return g_session->saveToActiveProject(); }
 // rejected op (duplicate name, un-deletable pool, etc.) returns before reaching here,
 // so no dangling/empty undo point is ever opened for a rejected op.
 //
-// UNSAVED-PROJECT GUARDRAIL: on an unsaved / no-active project persistBook() no-ops
-// (nothing is written to ext state). We must still CLOSE the block we opened, but with
-// an EMPTY label and a zero flag so REAPER DISCARDS the point instead of recording a
-// no-effect undo entry — mirroring view.cpp's empty-plan close. The in-session model
-// change stands and persists on the user's next save; it just earns no undo point until
-// there is a project to persist into (undo of an unsaved bank op has nothing to roll
-// back to anyway). The Begin/End must still be balanced, hence the close-either-way.
-void persistBankOp(const char* label) {
-    Undo_BeginBlock2(nullptr);
-    const bool persisted = persistBook();
-    if (persisted)
-        Undo_EndBlock2(nullptr, label, UNDO_STATE_MISCCFG);
-    else
-        Undo_EndBlock2(nullptr, "", 0);  // no ext-state write -> discard the empty point
-}
-
 // Prompts the user for a single line of text via REAPER's stock input dialog.
 // GetUserInputs(title, num_inputs=1, captions_csv, retvals_csv, sz) -> false on
 // cancel (SDK ~3808). `initial` pre-fills the field. Returns false (leaving `out`
@@ -827,6 +811,39 @@ void doBankRemoveSelected() {
 
 }  // namespace
 
+// Persists a completed bank-index verb as a SINGLE batched REAPER undo point (R-B) —
+// one bank op = one Ctrl-Z. Declared in actions.h so bank_panel.cpp can call it
+// without duplicating the undo logic.
+//
+// WHY THIS WRAPS AND persistBook() DOES NOT: a bank verb mutates ONLY our project
+// ext-state (SetProjExtState under "reasampler"), which REAPER's undo system captures
+// iff UNDO_STATE_MISCCFG is set in the Undo_EndBlock2 flags — the SDK documents
+// MISCCFG as covering "extensions!" project ext-state (reaper_plugin.h ~1544, ~1199).
+// We pass exactly UNDO_STATE_MISCCFG (not -1 / UNDO_STATE_ALL as the item-move family
+// does): a bank verb touches no tracks, FX, items, or envelopes, so snapshotting them
+// would be both heavier and semantically wrong. persistBook() (= SetProjExtState) runs
+// INSIDE the block so the post-mutation ext-state is the block's "after" image.
+//
+// NO-OP GUARDRAIL: callers invoke this ONLY after the model mutation succeeded — a
+// rejected op (duplicate name, un-deletable pool, etc.) returns before reaching here,
+// so no dangling/empty undo point is ever opened for a rejected op.
+//
+// UNSAVED-PROJECT GUARDRAIL: on an unsaved / no-active project persistBook() no-ops
+// (nothing is written to ext state). We must still CLOSE the block we opened, but with
+// an EMPTY label and a zero flag so REAPER DISCARDS the point instead of recording a
+// no-effect undo entry — mirroring view.cpp's empty-plan close. The in-session model
+// change stands and persists on the user's next save; it just earns no undo point until
+// there is a project to persist into (undo of an unsaved bank op has nothing to roll
+// back to anyway). The Begin/End must still be balanced, hence the close-either-way.
+void persistBankOp(const char* label) {
+    Undo_BeginBlock2(nullptr);
+    const bool persisted = persistBook();
+    if (persisted)
+        Undo_EndBlock2(nullptr, label, UNDO_STATE_MISCCFG);
+    else
+        Undo_EndBlock2(nullptr, "", 0);  // no ext-state write -> discard the empty point
+}
+
 void bankRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session) {
     g_session = session;  // shared with the Design View family; same live session
 

src/actions.h

@@ -66,4 +66,13 @@ bool bankHandleCommand(int command);
 // rec==nullptr (before g_session is torn down).
 void bankUnregisterActions(reaper_plugin_info_t* rec);
 
+// 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
+// and the block is closed with an empty label + zero flag (REAPER discards it). Callers
+// must invoke this ONLY after a successful/effective mutation — rejected ops (duplicate
+// name, un-deletable pool, etc.) must return before reaching here so no empty undo
+// point is ever opened for a no-op. Defined in actions.cpp alongside persistBook().
+void persistBankOp(const char* label);
+
 }  // namespace reasampler

src/bank_panel.cpp

@@ -46,6 +46,7 @@
 #include <unordered_map>
 #include <vector>
 
+#include "actions.h"    // persistBankOp — shared undo-block wrapper (R-B panel path)
 #include "bank_book.h"
 #include "bank_grid.h"
 #include "bank_model.h"
@@ -1179,14 +1180,11 @@ bool regionAt(int x, int y, Region& out) {
 
 // --- Bank management ops (id-keyed; drive the B1 model + persist) --------------
 //
-// Each op mutates g_session.book() then persists via saveToActiveProject(). After a
+// Each op mutates g_session.book() then persists via persistBankOp(). After a
 // STRUCTURAL mutation (create/delete/evacuate) any Bank*/BankIndex& is invalid — we
-// resolve fresh, pass ids, and let the next refreshFingerprint repaint. persistBook
+// resolve fresh, pass ids, and let the next refreshFingerprint repaint. On an
-// no-ops on an unsaved project (matches the capture/B3 quiet-persist idiom).
+// unsaved project the empty-close discard in persistBankOp ensures no stale state
-
+// survives (matches the capture/B3 quiet-persist idiom).
-void persistBook() {
-    if (g_panel.session) g_panel.session->saveToActiveProject();
-}
 
 // REAPER's stock single-line input (comma-safe via the \x1f return separator, as B3).
 bool promptText(const char* title, const char* caption, const std::string& initial,
@@ -1224,7 +1222,7 @@ void doCreateBank() {
     }
     g_panel.shownBankId = id;             // show the freshly-created bank
     g_panel.focusedRegion = Region::Banks;
-    persistBook();
+    persistBankOp("ReaSampler: create bank");
     invalidatePanel();
 }
 
@@ -1240,7 +1238,7 @@ void doRenameBank(const std::string& bankId) {
                        "ReaSampler: rename bank", 0);
         return;
     }
-    persistBook();
+    persistBankOp("ReaSampler: rename bank");
     invalidatePanel();
 }
 
@@ -1273,7 +1271,7 @@ void doDeleteBank(const std::string& bankId) {
         // r == 6 (Yes) falls through to a plain delete (drops members).
     }
     if (!book()->deleteBank(bankId)) return;
-    persistBook();
+    persistBankOp("ReaSampler: delete bank");
     // shownBankId is reconciled by the next fingerprint pass. If no named banks remain,
     // nudge focus to the pool so the selection has a valid home.
     if (namedBanks().empty()) g_panel.focusedRegion = Region::Pool;
@@ -1285,30 +1283,51 @@ void doEvacuateBank(const std::string& bankId) {
     const Bank* bk = book()->bank(bankId);
     if (!bk || bk->isPool()) return;
     if (!book()->evacuate(bankId)) return;
-    persistBook();
+    persistBankOp("ReaSampler: evacuate bank");
     invalidatePanel();
 }
 
 void doActivateBank(const std::string& bankId) {
     if (!book()) return;
     if (!book()->setActiveBank(bankId)) return;  // rejects an unknown id
-    persistBook();
+    persistBankOp("ReaSampler: activate bank");
     invalidatePanel();
 }
 
 // Move or copy `sampleIds` from `srcBankId` to `destBankId` (index-only). Both pass
 // ids straight to the model op (no BankIndex& cached across the loop's mutations).
+//
+// NO-OP GUARDRAIL — VERB-AWARE (matches the action layer's doBankTransferSelected):
+//   * MOVE collapse: the source entry WAS removed (bank_book removes unconditionally
+//     before the dest add collapses on hash), so the index DID mutate — counts.
+//   * COPY collapse: the source is left intact AND the dest already held the hash,
+//     so NOTHING changed — a true index no-op. Must NOT open an undo point.
+// Hence: copy counts only real gains (Copied); move counts gains OR collapses.
 void transferSamples(const std::vector<std::string>& sampleIds,
                      const std::string& srcBankId, const std::string& destBankId,
                      bool copy) {
     if (!book()) return;
     if (sampleIds.empty() || srcBankId == destBankId) return;
     if (!book()->bank(srcBankId) || !book()->bank(destBankId)) return;
+    int ok = 0, collapsed = 0;
     for (const std::string& sid : sampleIds) {
-        if (copy) book()->copySample(sid, srcBankId, destBankId);
+        const TransferResult r =
-        else      book()->moveSample(sid, srcBankId, destBankId);
+            copy ? book()->copySample(sid, srcBankId, destBankId)
+                 : book()->moveSample(sid, srcBankId, destBankId);
+        switch (r) {
+            case TransferResult::Moved:
+            case TransferResult::Copied:        ++ok; break;
+            case TransferResult::Collapsed:     ++collapsed; break;
+            case TransferResult::RejectedUnknownBank:
+            case TransferResult::RejectedSampleAbsent:
+            case TransferResult::RejectedSameBank: break;
         }
-    persistBook();
+    }
+    const bool mutated = copy ? (ok > 0) : (ok > 0 || collapsed > 0);
+    if (!mutated) return;  // nothing changed — no persist, no undo point
+
+    const char* label = copy ? "ReaSampler: copy sample(s)" : "ReaSampler: move sample(s)";
+    persistBankOp(label);
     // The selection indexed into the source; after a move those indices are stale, so
     // clear it (the fingerprint pass will also clear, but do it now for immediacy).
     g_panel.selection = Selection{};
@@ -1357,7 +1376,7 @@ void removeSamples(const std::vector<std::string>& sampleIds,
             ++removed;
     if (removed == 0) return;  // nothing changed — no persist, no undo point
 
-    persistBook();
+    persistBankOp("ReaSampler: remove sample(s)");
     // The selection indexed into the source; after a remove those indices are stale, so
     // clear it (the fingerprint pass will also clear, but do it now for immediacy).
     g_panel.selection = Selection{};