Cut shell/actions, bank_ops, app comment bloat ~48% (comments only, zero code change)

src/shell/actions/bank_actions.cpp

@@ -1,23 +1,17 @@
-// bank_actions.cpp — the multi-bank bindable action family (Phase B3; Q-W4 split of
-// actions.cpp). See bank_actions.h.
+// bank_actions.cpp — see bank_actions.h.
 //
-// Q-W4 dedupe / Q-W6 seam: each mutating handler is a THIN UX SKIN — text prompts
-// (promptBankName), name resolution, and console feedback — over the promptless
-// bankOp* inner verbs homed in shell/bank_ops (model op + persistBankOp, one bank op
-// = one Ctrl-Z), driven against this family's registered session. The book's rules
-// (pool privileges, collapse-by-hash, active-fallback-to-pool) all live in
-// bank_book; these handlers only drive the verbs and react to the boolean.
+// Each mutating handler is a thin UX skin — text prompts, name resolution, console
+// feedback — over the promptless bankOp* verbs in shell/bank_ops (model op +
+// persistBankOp, one bank op = one Ctrl-Z). Pool privileges / collapse-by-hash /
+// active-fallback-to-pool live in bank_book; handlers only drive the verbs.
 //
-// REFERENCE-INVALIDATION GUARDRAIL (B2 review): book().activeIndex() / bank()->index
-// return a reference INTO the book's internal vector, which a create/delete can
-// reallocate. No handler here caches a BankModel& (or a Bank*) across a structural
-// mutation — each resolves ids to strings up front and re-resolves after any
-// create/delete. Move/copy pass ids (not references) straight to the verbs.
+// REFERENCE-INVALIDATION GUARDRAIL: book().activeIndex() / bank()->index return a
+// reference INTO the book's internal vector, which a create/delete can reallocate.
+// No handler caches a BankModel&/Bank* across a structural mutation — ids are
+// resolved to strings up front and re-resolved after any create/delete.
 //
-// Compiled into the reaper_reasampler MODULE. Includes reaper_plugin_functions.h
-// WITHOUT REAPERAPI_IMPLEMENT — main.cpp owns the API pointers (CLAUDE.md §contract).
-// The action ids are minted from FOREVER-STABLE strings; user keybindings key off
-// them, so they must never change after ship.
+// main.cpp owns the API pointers; this TU gets them extern. Action ids are minted
+// from FOREVER-STABLE strings — never change one after ship.
 
 #include "shell/actions/bank_actions.h"
 
@@ -42,11 +36,9 @@ namespace reasampler {
 
 namespace {
 
-// FOREVER-STABLE multi-bank action-id SUFFIXES (Phase V, V4). The channel family prefix is
-// prepended at register via channelCommandId (as with the Design View family) — stable
-// rebuilds the shipped id, beta the isolated one. NEVER change a shipped suffix.
-// Each suffix + the stable prefix must byte-match the pre-V4 shipped literal exactly
-// (e.g. "BANK_REMOVE_SELECTED" -> "CEREBELLUM_REASAMPLER_BANK_REMOVE_SELECTED").
+// FOREVER-STABLE action-id SUFFIXES: the channel prefix is prepended at register
+// (channelCommandId); NEVER change a shipped suffix — user keybindings key off the
+// composed id.
 constexpr const char* kIdBankCreate       = "BANK_CREATE";
 constexpr const char* kIdBankRename       = "BANK_RENAME";
 constexpr const char* kIdBankDelete       = "BANK_DELETE";
@@ -58,14 +50,10 @@ constexpr const char* kIdBankCopySel      = "BANK_COPY_SELECTED";
 constexpr const char* kIdBankRemoveSel    = "BANK_REMOVE_SELECTED";
 constexpr const char* kIdBankPoolFull     = "BANK_POOL_FULLHEIGHT";
 constexpr const char* kIdBankBanksFull    = "BANK_BANKS_FULLHEIGHT";
-// Phase R (Reclaim), R2: the FOREVER-STABLE "Prune bank folder" id. Registered NOW so
-// in-DAW dry-run verification is possible; R2 behaviour is REPORT-ONLY (no deletion),
-// and R3 extends the confirm-and-delete step behind this SAME id — never a throwaway id.
 constexpr const char* kIdBankPruneFolder  = "BANK_PRUNE_FOLDER";
 
-// The live session the actions read (name resolution, member counts, prune) and
-// pass to the bankOp* verbs by reference (bankHandleCommand guards it non-null
-// before any handler runs). Set once by bankRegisterActions; not owned here.
+// Not owned here; set once by bankRegisterActions. bankHandleCommand guards it
+// non-null before any handler runs.
 ReaSamplerSession* g_session = nullptr;
 
 int g_cmdBankCreate = 0;
@@ -96,25 +84,17 @@ gaccel_register_t g_accelBankPoolFull{};
 gaccel_register_t g_accelBankBanksFull{};
 gaccel_register_t g_accelBankPruneFolder{};
 
-// Resolves a user-typed bank reference (a display name) to a bank id, scanning the
-// book's banks in ordinal order. Exact match on displayName; "Pool" resolves the pool.
-// Returns "" when no bank carries that name. Kept in the action layer (not the model)
-// — it is UI name-resolution, not a model rule. First-match is unambiguous BY
-// CONSTRUCTION: the model enforces unique display names (trimmed + case-insensitive),
-// so at most one bank can carry a given name — no duplicate can shadow another here.
+// Resolves a user-typed display name to a bank id ("" if none matches). UI name
+// resolution, not a model rule — kept here rather than the model. Unambiguous by
+// construction: the model enforces unique display names.
 std::string bankIdByDisplayName(const std::string& name) {
     for (const Bank& b : g_session->book().banks())
         if (b.displayName == name) return b.id;
     return {};
 }
 
-// -- Action bodies (thin UX skins over the bankOp* verbs) -------------------
-
-// Create a named bank: prompt for a display name; the verb mints a stable GUID id,
-// creates it in the model, persists. The new bank is NOT auto-activated (create and
-// activate are distinct acts — mirrors capture/placement separation). The model
-// rejects a duplicate display name (trimmed + case-insensitive, incl. "Pool"); the
-// create then fails and the user is told the name is taken.
+// The new bank is NOT auto-activated (create and activate are distinct acts,
+// mirroring capture/placement separation).
 void doBankCreate() {
     std::string name;
     if (!promptBankName("ReaSampler: create bank", "Bank name:", "", name)) return;
@@ -126,9 +106,8 @@ void doBankCreate() {
     }
 }
 
-// Rename a bank: prompt for which bank (by current display name) and the new name.
-// The pool is un-renamable (the model rejects it). Two prompts keep the bindable form
-// self-contained; the panel renames in place on a tab.
+// Two prompts (which bank, then the new name) keep this bindable form
+// self-contained; the panel renames in place on a tab instead.
 void doBankRename() {
     std::string which;
     if (!promptBankName("ReaSampler: rename bank", "Bank to rename (current name):", "",
@@ -142,18 +121,13 @@ void doBankRename() {
     std::string newName;
     if (!promptBankName("ReaSampler: rename bank", "New name:", which, newName)) return;
     if (!bankOpRename(*g_session, id, newName)) {
-        // The verb rejects the pool (un-renamable) or a name already used by another
-        // bank (unique display names, trimmed + case-insensitive).
         ShowConsoleMsg("ReaSampler: cannot rename that bank (the pool is un-renamable, "
                        "or another bank already uses that name).\n");
     }
 }
 
-// Delete a named bank. Bindable safe-form of the confirm-on-non-empty guardrail:
-// prompt for the bank; if it holds members, a YESNO ShowMessageBox names evacuate as
-// the alternative before dropping them (a plain delete orphans those members' files
-// until prune — CONTEXT.md §delete). An empty bank deletes with no prompt. The richer
-// panel confirm (naming evacuate inline, with a one-click evacuate) lives in the panel.
+// If the bank holds members, confirm first (a plain delete orphans those members'
+// files until prune); an empty bank deletes with no prompt.
 void doBankDelete() {
     std::string which;
     if (!promptBankName("ReaSampler: delete bank", "Bank to delete:", "", which)) return;
@@ -162,15 +136,13 @@ void doBankDelete() {
         ShowConsoleMsg(("ReaSampler: no bank named \"" + which + "\".\n").c_str());
         return;
     }
-    // Pool early-out: the pool is un-deletable (the model rejects it). Catch it here,
-    // BEFORE the non-empty confirm, so typing "Pool" never shows a misleading
-    // "delete anyway?" prompt for an operation the model will refuse regardless.
+    // Catch the pool BEFORE the non-empty confirm, so typing "Pool" never shows a
+    // misleading "delete anyway?" for an operation the model will refuse regardless.
     if (id == kPoolBankId) {
         ShowConsoleMsg("ReaSampler: the pool cannot be deleted.\n");
         return;
     }
-    // Read member count BEFORE deleting (the Bank* is invalidated by the delete; we do
-    // not cache it — resolve size to an int up front).
+    // Read member count before deleting — the Bank* is invalidated by the delete.
     const Bank* b = g_session->book().bank(id);
     if (!b) return;  // race-safe: id resolved above but re-check
     const std::size_t members = b->index.size();
@@ -184,16 +156,14 @@ void doBankDelete() {
         const int r = ShowMessageBox(msg.c_str(), "ReaSampler: delete non-empty bank", 4);
         if (r != 6) return;  // 6 == YES; anything else cancels (SDK ~6544)
     }
-    // S9: bump only when the deleted bank held samples — dropping them changes what a live
-    // instance referencing one could play. Deleting an EMPTY bank is purely organizational.
+    // Bump only when the deleted bank held samples — dropping them changes what a
+    // live instance referencing one could play.
     if (!bankOpDelete(*g_session, id, /*bumpGeneration=*/members > 0)) {
         ShowConsoleMsg("ReaSampler: cannot delete that bank (the pool is un-deletable).\n");
     }
 }
 
-// Evacuate a named bank: move every member back to the pool (index-only, collapse by
-// hash), leaving the bank empty. The pool is un-evacuable (the verb rejects it). The
-// intended "keep the samples" companion to delete.
+// The "keep the samples" companion to delete: moves every member back to the pool.
 void doBankEvacuate() {
     std::string which;
     if (!promptBankName("ReaSampler: evacuate bank", "Bank to evacuate to the pool:", "",
@@ -210,10 +180,8 @@ void doBankEvacuate() {
     }
 }
 
-// Cycle the active bank forward in ordinal order (pool -> named -> ... -> pool),
-// via the pure nextBankId helper. Activating a bank changes the CAPTURE TARGET (the
-// next capture lands in the newly-active bank — B2's book().activeIndex() seam) and
-// never touches the timeline. The verb persists so the active id travels with the .rpp.
+// Cycles the active bank (pool -> named -> ... -> pool). Activating changes the
+// CAPTURE TARGET only — never touches the timeline.
 void doBankActivateNext() {
     std::vector<std::string> ids;
     ids.reserve(g_session->book().size());
@@ -223,20 +191,13 @@ void doBankActivateNext() {
     bankOpActivate(*g_session, target);
 }
 
-// Activate the pool directly (the common "back to the default target" jump). Bindable
-// direct-by-id form; a general activate-bank-by-name/menu is a panel affordance.
 void doBankActivatePool() {
     bankOpActivate(*g_session, kPoolBankId);
 }
 
-// Move or copy the panel's selected samples into a named destination bank (prompted
-// by display name). The SOURCE is the bank the selection lives in — the focused
-// region's displayed bank (bankPanelSelectedSourceBankId), which under B4's vertical
-// split is NOT necessarily the active/capture-target bank (active ≠ shown). Both are
-// index-only (files never relocate); the verb owns the verb-aware no-op guardrail and
-// destination collapse-by-hash. The panel's "move to bank" menu drives the same verb
-// with a menu-chosen destination — this bindable form is the same operation with a
-// text-prompt destination.
+// SOURCE is the bank the selection lives in (bankPanelSelectedSourceBankId), which is
+// NOT necessarily the active/capture-target bank — the vertical split can show a
+// different bank than the one active for capture.
 void doBankTransferSelected(bool copy) {
     const std::vector<std::string> selected = bankPanelSelectedSampleIds();
     if (selected.empty()) {
@@ -253,7 +214,6 @@ void doBankTransferSelected(bool copy) {
         ShowConsoleMsg(("ReaSampler: no bank named \"" + destName + "\".\n").c_str());
         return;
     }
-    // Source = the bank the selection lives in (the focused region's displayed bank).
     const std::string srcId = bankPanelSelectedSourceBankId();
     if (srcId == destId) {
         ShowConsoleMsg("ReaSampler: source and destination are the same bank.\n");
@@ -262,10 +222,8 @@ void doBankTransferSelected(bool copy) {
     bankOpTransfer(*g_session, selected, srcId, destId, copy);
 }
 
-// Remove the panel's selected samples from the SOURCE bank (the focused region's
-// displayed bank — same source as move/copy). Index-only and non-destructive to the
-// file (orphaned until Phase R prune); silent, with the batched undo as recovery —
-// see bankOpRemove for the full contract.
+// Index-only and non-destructive to the file (orphaned until prune); silent, with
+// the batched undo as recovery.
 void doBankRemoveSelected() {
     const std::vector<std::string> selected = bankPanelSelectedSampleIds();
     if (selected.empty()) {
@@ -307,8 +265,6 @@ void bankRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session)
                                        "toggle pool full-height");
     g_cmdBankBanksFull = registerAction(rec, kIdBankBanksFull, g_accelBankBanksFull,
                                         "toggle banks full-height");
-    // Phase R, R2: the "Prune bank folder" action (report-only in this wave; R3 extends
-    // the confirm-and-delete step behind this SAME forever-stable id).
     g_cmdBankPruneFolder = registerAction(rec, kIdBankPruneFolder, g_accelBankPruneFolder,
                                           "prune bank folder");
 }
@@ -335,8 +291,8 @@ bool bankHandleCommand(int command) {
 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).
+    // Reverse of registration order; each '-command_id' re-presents the same
+    // interned id (channelIdFor).
     rec->Register("-gaccel", (void*)&g_accelBankPruneFolder);
     rec->Register("-command_id", (void*)channelIdFor(kIdBankPruneFolder));
     rec->Register("-gaccel", (void*)&g_accelBankBanksFull);