fix(editor): resnap works with Snap off and snaps BOTH edges to the subdivision guidelines

CHANGELOG.md

@@ -34,6 +34,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   on load, through plain saves, Save As, and first builds alike. Sliding the
   shift back to zero removes it from the pack again, so unshifted songs stay
   byte-identical to before.
+- **Inspector technique edits are undoable now.** Toggling a technique flag
+- **Resnap selection now works with Snap toggled off — and snaps both edges.**
+  "Resnap selection to grid" (Edit menu / its shortcut) honoured the live Snap
+  toggle, so with snapping off it silently moved nothing — which read as the
+  feature not existing at all ("I miss a way to snap the selected notes to the
+  grid"). An explicit quantize now always snaps, using whatever **subdivision
+  you have selected** — the same guidelines the grid draws, like the piano-roll
+  grid in a DAW. And it snaps **both edges**: note starts quantise to the
+  nearest guideline, and a sustained note's end edge follows — never collapsing
+  onto its start (it keeps at least one subdivision), while zero-length chips
+  are never inflated. One undoable step, and the status line now always tells
+  you what happened, including "already on the grid."
 - **Inspector technique edits are undoable now.** Toggling a technique flag
   (Palm Mute, Hammer-On, Tap, …) or setting a bend/slide value from the
   inspector panel used to mutate the note in place with no undo — so Ctrl+Z

src/input.js

@@ -18,7 +18,7 @@ import { hitNote } from './hit-test.js';
 import { _renderInspector, _selectedChordContext } from './inspector.js';
 import { PIANO_LANE_H, _rollLockNotice, _rollReadOnly, isKeysArr, isKeysMode, midiToFret, midiToString, pianoLaneCount, yToMidi } from './keys.js';
 import { lanes, _stringCountFor } from './lanes.js';
-import { _editorClampScrollX, _loopNudgeEdge, snapTime } from './loop.js';
+import { _editorClampScrollX, _loopNudgeEdge, snapGuidelineAfter, snapTime } from './loop.js';
 import { _recState } from './midi-record.js';
 import { getMousePos } from './mouse.js';
 import { _resizeSustainsForDeltaPure, notes } from './notes.js';
@@ -502,22 +502,83 @@ function _editorNudgeSelectionTime(dir) {
     return true;
 }
 
+/* @pure:resnap-edges:start */
+// Both EDGES of every note snap to the current-subdivision guidelines — the
+// Logic piano-roll model: the start edge quantises to its nearest guideline,
+// and a SUSTAINED note's end edge does too, except it never collapses onto
+// the start (it keeps at least one subdivision — `afterFn` supplies the first
+// guideline strictly after the new start). A zero-sustain chip stays a chip:
+// length is authored intent, never inflated by a quantize.
+export function _resnapEdgesPure(oldTimes, oldSustains, snapFn, afterFn) {
+    const newTimes = oldTimes.map(t => snapFn(t));
+    const newSustains = oldSustains.map((sus, i) => {
+        if (!(sus > 0)) return sus || 0;
+        const end = snapFn(oldTimes[i] + sus);
+        const bounded = end > newTimes[i] + 1e-9 ? end : afterFn(newTimes[i]);
+        // afterFn is the identity when there is no usable grid (fewer than two
+        // beats, or the snap value is off) — and in ONSET mode snapFn still
+        // snaps, so a short note's two edges can land on the same onset with no
+        // guideline to push the end past it. A quantize must never eat a
+        // sustained note: with no positive bound to offer, keep the length.
+        return bounded > newTimes[i] + 1e-9 ? bounded - newTimes[i] : sus;
+    });
+    return { newTimes, newSustains };
+}
+/* @pure:resnap-edges:end */
+
+// One undoable step for the two halves of an edge resnap — starts move,
+// sustained ends re-length. Exec in order, rollback in reverse; gating
+// follows the MoveNoteCmd half (same as the verb always had).
+class ResnapEdgesCmd {
+    constructor(move, resize) { this.move = move; this.resize = resize; }
+    exec() { this.move.exec(); if (this.resize) this.resize.exec(); }
+    rollback() { if (this.resize) this.resize.rollback(); this.move.rollback(); }
+}
+
 function _editorResnapSelection() {
     const idxs = _editorCurrentNoteIndices();
     if (!idxs.length) { setStatus('Select notes first'); return false; }
+    // The verb has always been refused on a read-only roll (its MoveNoteCmd half
+    // isn't pitchPreserving, so the history gate rejects it). Say so HERE: the
+    // gate's refusal is silent to us, and the success line below would otherwise
+    // overwrite the lock notice and claim notes moved when none did.
+    if (_rollReadOnly()) { _rollLockNotice(); return true; }
     const nn = notes();
     const oldTimes = idxs.map(i => nn[i].time);
-    const newTimes = oldTimes.map(t => snapTime(t));
-    for (let i = 0; i < idxs.length; i++) nn[idxs[i]].time = oldTimes[i];
+    const oldSustains = idxs.map(i => Number(nn[i].sustain) || 0);
+    // FORCED snap: this is the explicit quantize verb, so it snaps to the
+    // guidelines (or onsets, in Onset mode) even while the live Snap toggle
+    // is OFF — with the toggle honoured it silently moved nothing, which read
+    // as "there's no way to snap notes to the grid" (a real tester report).
+    const { newTimes, newSustains } = _resnapEdgesPure(
+        oldTimes, oldSustains, (t) => snapTime(t, true), (t) => snapGuidelineAfter(t, true));
     const dtimes = newTimes.map((t, i) => t - oldTimes[i]);
+    const touched = idxs.filter((_, i) => Math.abs(dtimes[i]) > 1e-9
+        || Math.abs(newSustains[i] - oldSustains[i]) > 1e-9).length;
+    if (!touched) {
+        // Say so — a silent no-op is indistinguishable from a missing feature.
+        setStatus(`Selection already on the grid (${idxs.length} note${idxs.length === 1 ? '' : 's'} checked).`);
+        return true;
+    }
     const dstrings = idxs.map(() => 0);
-    S.history.exec(new MoveNoteCmd(idxs, dtimes, dstrings, null));
+    const susIdx = [], susVals = [];
+    for (let i = 0; i < idxs.length; i++) {
+        if (Math.abs(newSustains[i] - oldSustains[i]) > 1e-9) {
+            susIdx.push(idxs[i]);
+            susVals.push(newSustains[i]);
+        }
+    }
+    S.history.exec(new ResnapEdgesCmd(
+        new MoveNoteCmd(idxs, dtimes, dstrings, null),
+        susIdx.length ? new ResizeSustainGroupCmd(susIdx, susVals) : null));
     // Repeated resnapping is the canonical "fighting the grid" signal (charrette
     // §3.2): the notes keep landing off the beat, so the GRID may be the thing
     // that's wrong — nudge toward the Tempo tools.
     _signpostNote('gridFight');
     host.draw();
     host.updateStatus();
+    setStatus(`Snapped ${touched} of ${idxs.length} note${idxs.length === 1 ? '' : 's'} to the `
+        + `${S.snapMode === 'onset' ? 'detected onsets' : 'grid'} (both edges).`);
     return true;
 }
 

src/loop.js

@@ -382,18 +382,24 @@ export function editorToggleLoopRegion() { return _setLoopRegionEnabled(!S.loopE
 // placements; beyond it, snapTime falls back to grid snap.
 export const ONSET_SNAP_TOL = 0.07;
 
-export function snapTime(t) {
+// `force` bypasses the Snap ON/OFF toggle (the snap VALUE and mode still
+// apply): the toggle governs live interactive placement, but an EXPLICIT
+// quantize verb ("Resnap selection to grid") must snap regardless — with the
+// toggle off it silently returned every time unchanged, which read as the
+// command not existing at all. Default off: every interactive caller keeps
+// honouring the toggle exactly as before.
+export function snapTime(t, force = false) {
     // Onset-snap mode (charrette §1.6): when snapping is on and the target is
     // 'onset', prefer the nearest detected audio transient within ONSET_SNAP_TOL
     // — the bridge between musical time and audio-attack time for transcription
     // (no warp; just snap placement to the onset time). Falls back to grid snap
     // when no onset is near, or none is computed, so placement stays sensible.
-    if (S.snapEnabled && S.snapMode === 'onset') {
+    if ((S.snapEnabled || force) && S.snapMode === 'onset') {
         const onsets = (typeof _ensureOnsetsShifted === 'function') ? _ensureOnsetsShifted() : null;
         const near = _nearestOnsetTimePure(onsets, t, ONSET_SNAP_TOL);
         if (near !== null) return near;
     }
-    const sv = _editorEffectiveSnapValuePure(S.snapEnabled, SNAP_VALUES[S.snapIdx]);
+    const sv = _editorEffectiveSnapValuePure(S.snapEnabled || force, SNAP_VALUES[S.snapIdx]);
     if (!sv || S.beats.length < 2) return t;
     // Snap in the beat domain, then convert back (charrette §1.1):
     // snap = timeOf(round(beatOf(t)·subs)/subs). Sharing the converter makes the
@@ -406,6 +412,19 @@ export function snapTime(t) {
     return timeOf(S.beats, _swingQuantizeBeatPure(beatOf(S.beats, t), subs, S.swingPct));
 }
 
+// The first grid guideline strictly AFTER `t` at the current subdivision —
+// the minimum length an END-edge snap may leave a sustained note (the Logic
+// piano-roll rule: edges snap to the guidelines, but a sustained note never
+// collapses to zero; it keeps at least one subdivision). Same `force`
+// semantics as snapTime. Identity when snapping is unavailable.
+export function snapGuidelineAfter(t, force = false) {
+    const sv = _editorEffectiveSnapValuePure(S.snapEnabled || force, SNAP_VALUES[S.snapIdx]);
+    if (!sv || S.beats.length < 2) return t;
+    const subs = _editorSnapSubdivisionsPure(sv);
+    const q = Math.floor(beatOf(S.beats, t) * subs + 1e-6) + 1;
+    return timeOf(S.beats, q / subs);
+}
+
 /* @pure:group-time-delta:start */
 // The time delta to apply to a WHOLE dragged selection. Snapping each note's
 // absolute time independently (`snapFn(origTime + dtRaw)` per note) quantises

tests/resnap_explicit.test.mjs

@@ -0,0 +1,108 @@
+/*
+ * Explicit resnap works with the Snap toggle OFF (tester report: "I miss a
+ * way to snap the selected notes to the grid" — the command existed, but
+ * snapTime honoured the live-placement toggle, so with Snap off the explicit
+ * quantize verb silently moved nothing and read as a missing feature).
+ *
+ * Pinned: snapTime's new `force` arg bypasses the ON/OFF toggle while the
+ * snap VALUE still applies; the default (no force) keeps honouring the
+ * toggle bit-exactly, so every interactive caller is unchanged.
+ *
+ * The force cases fail on main (the arg is ignored there → identity).
+ * Run: node tests/resnap_explicit.test.mjs
+ */
+import assert from 'node:assert';
+
+globalThis.document = globalThis.document || {
+    getElementById: () => null, addEventListener: () => {}, activeElement: null,
+};
+globalThis.localStorage = globalThis.localStorage || { getItem: () => null, setItem: () => {} };
+globalThis.window = globalThis.window || globalThis;
+
+const { snapGuidelineAfter, snapTime } = await import('../src/loop.js');
+const { _resnapEdgesPure } = await import('../src/input.js');
+const { S } = await import('../src/state.js');
+
+let pass = 0, fail = 0;
+function t(name, fn) {
+    try { fn(); pass++; console.log('  ok   ' + name); }
+    catch (e) { fail++; console.error('  FAIL ' + name + ': ' + e.message); }
+}
+
+// A steady 120 BPM 4/4 grid: beats every 0.5s, downbeats every 2s.
+const beats = [];
+for (let m = 0; m < 4; m++) for (let b = 0; b < 4; b++) {
+    beats.push({ time: (m * 4 + b) * 0.5, measure: b === 0 ? m + 1 : 0 });
+}
+Object.assign(S, { beats, snapIdx: 0, snapMode: 'grid', swingPct: 50 });
+
+t('snap OFF: interactive snapTime stays an identity (toggle honoured, unchanged)', () => {
+    S.snapEnabled = false;
+    assert.strictEqual(snapTime(1.13), 1.13);
+});
+
+t('snap OFF + force: the explicit quantize still lands on the grid', () => {
+    S.snapEnabled = false;
+    assert.strictEqual(snapTime(1.13, true), 1.0, 'quantised to the nearest guideline');
+    assert.strictEqual(snapTime(1.38, true), 1.5);
+});
+
+t('snap ON: force and no-force agree (force only widens, never changes)', () => {
+    S.snapEnabled = true;
+    assert.strictEqual(snapTime(1.13), snapTime(1.13, true));
+    assert.strictEqual(snapTime(1.13), 1.0);
+});
+
+t('force in onset mode with no audio falls back to the grid, not identity', () => {
+    S.snapEnabled = false;
+    S.snapMode = 'onset';
+    assert.strictEqual(snapTime(1.13, true), 1.0, 'no onsets under node → grid fallback');
+    S.snapMode = 'grid';
+});
+
+// ── the guideline-after helper (the end-edge minimum) ────────────────
+
+t('snapGuidelineAfter returns the first guideline strictly after t', () => {
+    S.snapEnabled = false;
+    assert.strictEqual(snapGuidelineAfter(1.0, true), 1.5, 'ON a line → the next one');
+    assert.strictEqual(snapGuidelineAfter(1.13, true), 1.5, 'between lines → the next one');
+    assert.strictEqual(snapGuidelineAfter(1.0), 1.0, 'unforced with snap off → identity, like snapTime');
+});
+
+// ── both edges (the Logic piano-roll model) ──────────────────────────
+
+t('both edges land on guidelines: start quantises, the end edge follows', () => {
+    // start 1.13 → 1.0; end 1.63 → 1.5 → sustain 0.5 (both edges on lines).
+    const r = _resnapEdgesPure([1.13], [0.5], (x) => snapTime(x, true), (x) => snapGuidelineAfter(x, true));
+    assert.strictEqual(r.newTimes[0], 1.0);
+    assert.strictEqual(r.newSustains[0], 0.5);
+});
+
+t('a short sustained note never collapses — it keeps one subdivision', () => {
+    // start 1.13 → 1.0; end 1.23 quantises to 1.0 = the start line → the
+    // guard takes the NEXT guideline instead: end 1.5, sustain 0.5.
+    const r = _resnapEdgesPure([1.13], [0.1], (x) => snapTime(x, true), (x) => snapGuidelineAfter(x, true));
+    assert.strictEqual(r.newTimes[0], 1.0);
+    assert.strictEqual(r.newSustains[0], 0.5, 'one subdivision, never zero');
+});
+
+t('a zero-sustain chip stays a chip — length is authored intent', () => {
+    const r = _resnapEdgesPure([1.13], [0], (x) => snapTime(x, true), (x) => snapGuidelineAfter(x, true));
+    assert.strictEqual(r.newTimes[0], 1.0);
+    assert.strictEqual(r.newSustains[0], 0, 'never inflated by a quantize');
+});
+
+t('no usable grid: a sustained note keeps its length, never collapses to a chip', () => {
+    // The hole the end-edge guard has to plug: in ONSET mode snapFn still snaps
+    // even with no beat grid, so both edges of a short note can land on the same
+    // onset — and afterFn (grid-based) has no guideline to offer, so it returns
+    // the identity. A quantize must not silently eat the sustain.
+    const bothToSameOnset = () => 1.0;      // snapFn: every edge → the one onset
+    const noGuideline = (x) => x;           // afterFn: identity (no grid)
+    const r = _resnapEdgesPure([1.13], [0.1], bothToSameOnset, noGuideline);
+    assert.strictEqual(r.newTimes[0], 1.0);
+    assert.strictEqual(r.newSustains[0], 0.1, 'original length kept, not zeroed');
+});
+
+console.log(`\n${pass} passed, ${fail} failed`);
+process.exit(fail ? 1 : 0);