From cd11cdc27c0454351928a5107f34271a6bf6ac27 Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Thu, 9 Jul 2026 14:26:41 +0200 Subject: [PATCH] refactor(editor): extract the keys / piano-roll model to src/keys.js (R2, step 8b) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit src/main.js 20,127 -> 19,963 — under 20k for the first time. keys.js holds which view a part opens in (viewFor, isKeysMode, isKeysArr, _rollReadOnly) and the persisted per-part preference behind it, the roll's MIDI<->y geometry (midiToY, yToMidi, pianoLaneCount, noteToMidi/midiToNote/midiToString/midiToFret, isBlackKey, midiToFreq, PIANO_OCTAVE_COLORS, KEYS_PATTERN), and the sounding-pitch context that renders a fretted part read-only in the roll (_rollPitchCtx, _rollMidiForNote). _rollLockNotice stays in main.js — it calls setStatus. main.js's whole diff is the deletions plus the import block. Graph stays acyclic: keys -> {geometry, lanes, notes, state, theory}. LIVE BINDINGS, not a container. PIANO_LANE_H and pianoRange are reassigned per arrangement, but their SOLE writer — updatePianoRange — moved with them, so they are `export let` and importers read them live and cannot write them. That is the step-5 rule (geometry's lane metrics), not the step-4 one (lanes.js's LC, whose writers had to stay in draw()/onMouseMove()). The question is always "can the writer move?", not "is it reassigned?". Tests: four more suites off the slicer path. - keyboard_gutter, keyboard_gutter_dblclick: @pure:midi-freq is gone; both import midiToFreq / _inKeyboardGutterPure. dblclick still brace-extracts onDblClick, which stays in main.js. - rename_part: was regex-lifting `const KEYS_PATTERN = ...` out of the source; now imports it and injects it into the @pure:rename-arr sandbox. - view_switcher: the real rework. It used to re-evaluate _viewPrefs/_viewPrefsSave/ viewFor/isKeysArr/isKeysMode/_rollReadOnly/updatePianoRange from SOURCE inside a sandbox, against a fabricated `S` and an injected localStorage stub. It now drives the real module against the real S, installs the stub on globalThis (module code resolves `localStorage` at call time), bounces S.filename to bust _viewPrefs' per-song memo between cases, and reads pianoRange back through the NAMESPACE — a destructured copy would go stale, so that read is what proves the live binding actually updates for importers. Verified: node --test 86/86, pytest 248/248. main.js diff mechanically checked to be deletions + the import block. No assignment to PIANO_LANE_H/pianoRange survives in main.js (it would now throw); the only writers are the four lines inside updatePianoRange. No unused import; _viewForPure is the one test-only export. FIFTH headless harness, written for this step: no existing harness opened the roll (they all use a 6-string guitar chart). It discriminates the two views by what they paint — a fretted part draws bold-12px lane labels, a roll draws one keyboard-gutter fillRect per semitone of pianoRange. On Arcturus: Lead → 6 lane labels / 0 gutter rows; Keys → 0 labels / 60 rows; Lead forced into the roll via the view pref → 60 rows, i.e. SOUNDING pitch (the wire packing string*24+fret would span ~140 rows); switching back restores the lanes. The C-row octave labels are not usable as a signal — drawPianoLabels only draws them when PIANO_LANE_H >= 7, and a wide range puts it at 4-6px. All four existing harnesses green. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 19 ++ src/keys.js | 206 +++++++++++++++++ src/main.js | 216 +++--------------- ...utter.test.js => keyboard_gutter.test.mjs} | 14 +- ...t.js => keyboard_gutter_dblclick.test.mjs} | 17 +- ...name_part.test.js => rename_part.test.mjs} | 21 +- tests/view_switcher.test.mjs | 128 +++++------ 7 files changed, 336 insertions(+), 285 deletions(-) create mode 100644 src/keys.js rename tests/{keyboard_gutter.test.js => keyboard_gutter.test.mjs} (94%) rename tests/{keyboard_gutter_dblclick.test.js => keyboard_gutter_dblclick.test.mjs} (89%) rename tests/{rename_part.test.js => rename_part.test.mjs} (94%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82314395..e36d3691 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **ES-module migration, step 8b — the keys / piano-roll model (R2).** + `src/keys.js` (`main.js` 20,127 → 19,963, under 20k): which view a part opens in + (`viewFor`, `isKeysMode`, `isKeysArr`, `_rollReadOnly`) and the persisted + per-part preference behind it, plus the roll's own MIDI⇄y geometry + (`midiToY`/`yToMidi`, `pianoLaneCount`, `noteToMidi` and friends, `midiToFreq`, + `PIANO_OCTAVE_COLORS`, `KEYS_PATTERN`) and the sounding-pitch context that lets a + fretted part render read-only in the roll (`_rollPitchCtx`, `_rollMidiForNote`). + `_rollLockNotice` stays behind — it calls `setStatus`. + `PIANO_LANE_H` and `pianoRange` follow the step-5 rule rather than step-4's: they + are reassigned, but their sole writer `updatePianoRange` moved with them, so they + are live `export let` bindings — no container, no rename, and importers get a + `TypeError` if they try to write. + Four more suites leave the slicer path. `view_switcher` is the notable one: it + used to re-evaluate `_viewPrefs`/`viewFor`/`isKeysArr`/`updatePianoRange` from + source inside a sandbox with a fabricated `S` and a stub `localStorage`. It now + drives the real module against the real `S`, installs the stub on `globalThis`, + and reads `pianoRange` back through the namespace — which is what proves the live + binding updates for importers. + - **ES-module migration, step 8a — the open-string pitch model (R2).** The string→pitch half of the lane model joins `src/lanes.js`: `_GUITAR_OPEN_MIDI` / `_BASS_OPEN_MIDI` (module-private), `_openMidiForArr`, and the diff --git a/src/keys.js b/src/keys.js new file mode 100644 index 00000000..7b952e94 --- /dev/null +++ b/src/keys.js @@ -0,0 +1,206 @@ +/* Slopsmith Arrangement Editor — keys / piano-roll model. + * + * Which view a part opens in (fretted lanes vs piano roll), the persisted + * per-part preference behind that, and the roll's own MIDI⇄y geometry. Reads + * `S`, the lane model and the canvas geometry; the only DOM it touches is + * `localStorage`, inside `_viewPrefs`/`_viewPrefsSave`. + * + * `PIANO_LANE_H` and `pianoRange` are `export let`, not consts: they are + * re-derived per arrangement. Their sole writer, `updatePianoRange`, lives here, + * so importers read them as live bindings and none of them can write — the same + * shape geometry.js uses for its lane metrics, and the reason neither needs a + * container like lanes.js's `LC`. + */ + +import { WAVEFORM_H } from './geometry.js'; +import { _openMidiForArr, _soundingPitchPure, _stringCountFor } from './lanes.js'; +import { notes } from './notes.js'; +import { S } from './state.js'; +import { PIANO_NOTE_NAMES } from './theory.js'; + +// ── Piano roll constants ──────────────────────────────────────────── +export const PIANO_OCTAVE_COLORS = [ + '#ff4466', '#ff8844', '#ffcc33', '#66dd55', '#44ccaa', + '#44aaff', '#7766ff', '#cc55ff', '#ff55aa', '#aaaaaa', +]; +export let PIANO_LANE_H = 10; // pixels per MIDI semitone +export let pianoRange = { lo: 36, hi: 96 }; // MIDI range, updated per arrangement +// Names that should open in keys (piano-roll) editor mode. Arrangements +// named "Piano", "Keyboard", or "Synth" render as piano-roll charts rather +// than 6-string guitar charts. +export const KEYS_PATTERN = /^(keys|piano|keyboard|synth)/i; + +// Per-part editing-view choice (V2/V9 of EDITOR-VIEW-MODALITY-DESIGN): +// 'string' (fretted lanes) or 'piano' (the roll). Keys-DATA arrangements +// are piano-locked — their wire packing (string*24+fret) has no string +// semantics for the lane view to show. Fretted parts default to 'string' +// and may opt into the roll per part. The choice is EDITOR state +// (localStorage per song), never pack data. +export function _partViewKeyPure(arr) { + if (!arr) return ''; + const id = arr.id; + return (id !== undefined && id !== null && String(id) !== '') + ? String(id) + : (arr.name || ''); +} +export function _viewForPure(arrName, storedMode) { + if (KEYS_PATTERN.test(arrName || '')) return 'piano'; + return storedMode === 'piano' ? 'piano' : 'string'; +} + +// Per-song view prefs, cached so draw-path predicates never parse +// localStorage per frame. Keyed song filename → { partKey: 'piano' }. +let _viewPrefCache = null; +let _viewPrefFor = null; +export function _viewPrefs() { + const key = 'editorViewPref:' + (S.filename || ''); + if (_viewPrefFor === key && _viewPrefCache) return _viewPrefCache; + _viewPrefFor = key; + _viewPrefCache = {}; + // Unsaved songs don't read a bare slot every unsaved song would share. + if (!S.filename) return _viewPrefCache; + try { + const raw = localStorage.getItem(key); + if (raw) { + const o = JSON.parse(raw); + if (o && typeof o === 'object' && !Array.isArray(o)) _viewPrefCache = o; + } + } catch (_) { /* ignore */ } + return _viewPrefCache; +} +export function _viewPrefsSave() { + if (!S.filename) return; + try { + const key = 'editorViewPref:' + S.filename; + if (Object.keys(_viewPrefCache || {}).length) { + localStorage.setItem(key, JSON.stringify(_viewPrefCache)); + } else { + localStorage.removeItem(key); + } + } catch (_) { /* ignore */ } +} +export function viewFor(arr) { + return _viewForPure(arr && arr.name, _viewPrefs()[_partViewKeyPure(arr)]); +} + +// Keys-DATA predicate: the active arrangement's wire packing is pitch +// (string*24 + fret) — a keys/piano/synth part. DATA-SEMANTICS sites key +// off this: string-move helpers, chord-sibling grouping, anchors, MIDI +// record. Split from isKeysMode() when the view became a per-part choice. +export function isKeysArr() { + if (!S.arrangements.length) return false; + const arr = S.arrangements[S.currentArr]; + return !!(arr && KEYS_PATTERN.test(arr.name || '')); +} + +// Piano SURFACE predicate: the piano-roll view is active for the current +// part — keys data (always), or a fretted part opted into the roll. Draw +// geometry, hit-testing, and viewport paths key off this. The name is +// historical; every legacy call site that meant "the piano surface is +// showing" keeps working unchanged. +export function isKeysMode() { + if (!S.arrangements.length) return false; + return viewFor(S.arrangements[S.currentArr]) === 'piano'; +} + +// Read-only roll: a FRETTED part shown in the piano roll (V4). Editing +// stays locked until the suggest-position write path exists — the roll +// must never write string/fret by silent guess. +export function _rollReadOnly() { return isKeysMode() && !isKeysArr(); } + +// Sounding-pitch context for showing a FRETTED part in the roll — hoisted +// once per draw/hit-test pass, never per note. Null for keys parts (their +// packing IS the roll pitch). +export function _rollPitchCtx() { + if (!S.arrangements.length) return null; + const arr = S.arrangements[S.currentArr]; + if (!arr || KEYS_PATTERN.test(arr.name || '')) return null; + const laneCount = _stringCountFor(arr); + const tuning = (Array.isArray(arr.tuning) ? arr.tuning : []).slice(0, laneCount); + while (tuning.length < laneCount) tuning.push(0); + return { + openMidi: _openMidiForArr(arr, laneCount), + tuning, + capo: Number(arr.capo) || 0, + }; +} + +// Roll Y-axis MIDI for one note: keys packing, or sounding pitch (D4: +// openMidi + tuning + capo + fret) for fretted. Null = unresolvable — +// callers skip the note rather than paint it at a wrong pitch. +export function _rollMidiForNote(n, rctx) { + if (!rctx) return noteToMidi(n.string, n.fret); + return _soundingPitchPure(rctx.openMidi, rctx.tuning, rctx.capo, n.string, n.fret); +} + +export function pianoLaneCount() { return pianoRange.hi - pianoRange.lo + 1; } + +export function midiToNote(midi) { return PIANO_NOTE_NAMES[midi % 12] + (Math.floor(midi / 12) - 1); } +export function isBlackKey(midi) { const pc = midi % 12; return pc===1||pc===3||pc===6||pc===8||pc===10; } +// Equal-tempered frequency (Hz) of a MIDI note: A4 (69) = 440. Used by the +// keyboard-gutter audition (click a key → hear its pitch). Returns 0 for a +// non-finite input so a caller never schedules a NaN-frequency oscillator. +export function midiToFreq(midi) { + const m = Number(midi); + if (!Number.isFinite(m)) return 0; + return 440 * Math.pow(2, (m - 69) / 12); +} + +// Is (x, y) inside the piano keyboard gutter — the LABEL_W-wide column beside +// the roll's pitch lanes? Used to route a click to pitch audition instead of +// the note-edit pipeline. Half-open on every edge so it never overlaps the +// note area (x >= labelW) or the waveform/beat strips above/below. +export function _inKeyboardGutterPure(x, y, labelW, waveformTop, laneBottom) { + return x >= 0 && x < labelW && y >= waveformTop && y < laneBottom; +} + +export function noteToMidi(string, fret) { return string * 24 + fret; } +export function midiToString(midi) { return Math.floor(midi / 24); } +export function midiToFret(midi) { return midi % 24; } + +// Piano roll Y: higher MIDI = higher on screen (lower Y) +export function midiToY(midi) { return WAVEFORM_H + (pianoRange.hi - midi) * PIANO_LANE_H; } +export function yToMidi(y) { + const m = pianoRange.hi - Math.floor((y - WAVEFORM_H) / PIANO_LANE_H); + return Math.max(pianoRange.lo, Math.min(pianoRange.hi, m)); +} + +// expandOnly=true preserves any wider current range (used during in-place +// edits so adding a low note doesn't collapse the viewport and lose +// previously-clickable upper lanes). Load/import/arrangement-switch call +// without it so the viewport snaps cleanly to the new arrangement. +export function updatePianoRange(expandOnly = false) { + const nn = notes(); + // Fretted parts fit the range to SOUNDING pitch; keys keep the packing + // (noteToMidi encodes up to string=5, fret=23 → max 143, matching the + // drag-clamp ceiling). Unresolvable fretted notes are skipped. + const rctx = typeof _rollPitchCtx === 'function' ? _rollPitchCtx() : null; + let lo = 143, hi = 0; + for (const n of nn) { + const m = _rollMidiForNote(n, rctx); + if (m === null) continue; + if (m < lo) lo = m; + if (m > hi) hi = m; + } + if (lo > hi) { + // Empty arrangement: expose the full 88-key range so any starting + // pitch is clickable. Lanes are deliberately thin (~4px) to keep the + // viewport within ~352px — once a note is added the range snaps to + // the actual note range and lanes return to normal height. + pianoRange = { lo: 21, hi: 108, _fromEmpty: true }; + PIANO_LANE_H = 4; + return; + } + // Expand to octave boundaries with padding; ceiling matches drag-clamp max of 143. + let nlo = Math.max(0, Math.floor(lo / 12) * 12 - 6); + let nhi = Math.min(143, Math.ceil((hi + 1) / 12) * 12 + 5); + if (expandOnly && pianoRange && !pianoRange._fromEmpty) { + nlo = Math.min(nlo, pianoRange.lo); + nhi = Math.max(nhi, pianoRange.hi); + } + pianoRange = { lo: nlo, hi: nhi }; + // Adjust lane height to fill available space nicely. Allow down to 4px + // so wide note ranges (many octaves) remain visible without overflowing + // the canvas wrapper. + PIANO_LANE_H = Math.max(4, Math.min(14, 350 / (nhi - nlo + 1))); +} diff --git a/src/main.js b/src/main.js index d1c9addf..aa76bc34 100644 --- a/src/main.js +++ b/src/main.js @@ -45,6 +45,32 @@ import { xToTime, yToStr, } from './geometry.js'; +import { + KEYS_PATTERN, + PIANO_LANE_H, + PIANO_OCTAVE_COLORS, + _inKeyboardGutterPure, + _partViewKeyPure, + _rollMidiForNote, + _rollPitchCtx, + _rollReadOnly, + _viewPrefs, + _viewPrefsSave, + isBlackKey, + isKeysArr, + isKeysMode, + midiToFreq, + midiToFret, + midiToNote, + midiToString, + midiToY, + noteToMidi, + pianoLaneCount, + pianoRange, + updatePianoRange, + viewFor, + yToMidi, +} from './keys.js'; import { BEND_INTENTS, FRET_FINGER_OPTIONS, @@ -87,18 +113,6 @@ const MIN_NOTE_W = 18; const NOTE_PAD = 3; const DPR = window.devicePixelRatio || 1; -// ── Piano roll constants ──────────────────────────────────────────── -const PIANO_OCTAVE_COLORS = [ - '#ff4466', '#ff8844', '#ffcc33', '#66dd55', '#44ccaa', - '#44aaff', '#7766ff', '#cc55ff', '#ff55aa', '#aaaaaa', -]; -let PIANO_LANE_H = 10; // pixels per MIDI semitone -let pianoRange = { lo: 36, hi: 96 }; // MIDI range, updated per arrangement -// Names that should open in keys (piano-roll) editor mode. Arrangements -// named "Piano", "Keyboard", or "Synth" render as piano-roll charts rather -// than 6-string guitar charts. -const KEYS_PATTERN = /^(keys|piano|keyboard|synth)/i; - // ════════════════════════════════════════════════════════════════════ // State // ════════════════════════════════════════════════════════════════════ @@ -629,189 +643,11 @@ function _loopStripOnMouseUp() { draw(); return true; } -/* @pure:view-pref:start */ -// Per-part editing-view choice (V2/V9 of EDITOR-VIEW-MODALITY-DESIGN): -// 'string' (fretted lanes) or 'piano' (the roll). Keys-DATA arrangements -// are piano-locked — their wire packing (string*24+fret) has no string -// semantics for the lane view to show. Fretted parts default to 'string' -// and may opt into the roll per part. The choice is EDITOR state -// (localStorage per song), never pack data. -function _partViewKeyPure(arr) { - if (!arr) return ''; - const id = arr.id; - return (id !== undefined && id !== null && String(id) !== '') - ? String(id) - : (arr.name || ''); -} -function _viewForPure(arrName, storedMode) { - if (KEYS_PATTERN.test(arrName || '')) return 'piano'; - return storedMode === 'piano' ? 'piano' : 'string'; -} -/* @pure:view-pref:end */ - -// Per-song view prefs, cached so draw-path predicates never parse -// localStorage per frame. Keyed song filename → { partKey: 'piano' }. -let _viewPrefCache = null; -let _viewPrefFor = null; -function _viewPrefs() { - const key = 'editorViewPref:' + (S.filename || ''); - if (_viewPrefFor === key && _viewPrefCache) return _viewPrefCache; - _viewPrefFor = key; - _viewPrefCache = {}; - // Unsaved songs don't read a bare slot every unsaved song would share. - if (!S.filename) return _viewPrefCache; - try { - const raw = localStorage.getItem(key); - if (raw) { - const o = JSON.parse(raw); - if (o && typeof o === 'object' && !Array.isArray(o)) _viewPrefCache = o; - } - } catch (_) { /* ignore */ } - return _viewPrefCache; -} -function _viewPrefsSave() { - if (!S.filename) return; - try { - const key = 'editorViewPref:' + S.filename; - if (Object.keys(_viewPrefCache || {}).length) { - localStorage.setItem(key, JSON.stringify(_viewPrefCache)); - } else { - localStorage.removeItem(key); - } - } catch (_) { /* ignore */ } -} -function viewFor(arr) { - return _viewForPure(arr && arr.name, _viewPrefs()[_partViewKeyPure(arr)]); -} - -// Keys-DATA predicate: the active arrangement's wire packing is pitch -// (string*24 + fret) — a keys/piano/synth part. DATA-SEMANTICS sites key -// off this: string-move helpers, chord-sibling grouping, anchors, MIDI -// record. Split from isKeysMode() when the view became a per-part choice. -function isKeysArr() { - if (!S.arrangements.length) return false; - const arr = S.arrangements[S.currentArr]; - return !!(arr && KEYS_PATTERN.test(arr.name || '')); -} - -// Piano SURFACE predicate: the piano-roll view is active for the current -// part — keys data (always), or a fretted part opted into the roll. Draw -// geometry, hit-testing, and viewport paths key off this. The name is -// historical; every legacy call site that meant "the piano surface is -// showing" keeps working unchanged. -function isKeysMode() { - if (!S.arrangements.length) return false; - return viewFor(S.arrangements[S.currentArr]) === 'piano'; -} - -// Read-only roll: a FRETTED part shown in the piano roll (V4). Editing -// stays locked until the suggest-position write path exists — the roll -// must never write string/fret by silent guess. -function _rollReadOnly() { return isKeysMode() && !isKeysArr(); } function _rollLockNotice() { setStatus('Piano roll is read-only for fretted parts — Shift+↑/↓ cycles same-pitch positions; switch to String view to edit (suggest-position editing is coming)'); } -// Sounding-pitch context for showing a FRETTED part in the roll — hoisted -// once per draw/hit-test pass, never per note. Null for keys parts (their -// packing IS the roll pitch). -function _rollPitchCtx() { - if (!S.arrangements.length) return null; - const arr = S.arrangements[S.currentArr]; - if (!arr || KEYS_PATTERN.test(arr.name || '')) return null; - const laneCount = _stringCountFor(arr); - const tuning = (Array.isArray(arr.tuning) ? arr.tuning : []).slice(0, laneCount); - while (tuning.length < laneCount) tuning.push(0); - return { - openMidi: _openMidiForArr(arr, laneCount), - tuning, - capo: Number(arr.capo) || 0, - }; -} - -// Roll Y-axis MIDI for one note: keys packing, or sounding pitch (D4: -// openMidi + tuning + capo + fret) for fretted. Null = unresolvable — -// callers skip the note rather than paint it at a wrong pitch. -function _rollMidiForNote(n, rctx) { - if (!rctx) return noteToMidi(n.string, n.fret); - return _soundingPitchPure(rctx.openMidi, rctx.tuning, rctx.capo, n.string, n.fret); -} - -function pianoLaneCount() { return pianoRange.hi - pianoRange.lo + 1; } - -function midiToNote(midi) { return PIANO_NOTE_NAMES[midi % 12] + (Math.floor(midi / 12) - 1); } -function isBlackKey(midi) { const pc = midi % 12; return pc===1||pc===3||pc===6||pc===8||pc===10; } -/* @pure:midi-freq:start */ -// Equal-tempered frequency (Hz) of a MIDI note: A4 (69) = 440. Used by the -// keyboard-gutter audition (click a key → hear its pitch). Returns 0 for a -// non-finite input so a caller never schedules a NaN-frequency oscillator. -function midiToFreq(midi) { - const m = Number(midi); - if (!Number.isFinite(m)) return 0; - return 440 * Math.pow(2, (m - 69) / 12); -} - -// Is (x, y) inside the piano keyboard gutter — the LABEL_W-wide column beside -// the roll's pitch lanes? Used to route a click to pitch audition instead of -// the note-edit pipeline. Half-open on every edge so it never overlaps the -// note area (x >= labelW) or the waveform/beat strips above/below. -function _inKeyboardGutterPure(x, y, labelW, waveformTop, laneBottom) { - return x >= 0 && x < labelW && y >= waveformTop && y < laneBottom; -} -/* @pure:midi-freq:end */ - -function noteToMidi(string, fret) { return string * 24 + fret; } -function midiToString(midi) { return Math.floor(midi / 24); } -function midiToFret(midi) { return midi % 24; } - -// Piano roll Y: higher MIDI = higher on screen (lower Y) -function midiToY(midi) { return WAVEFORM_H + (pianoRange.hi - midi) * PIANO_LANE_H; } -function yToMidi(y) { - const m = pianoRange.hi - Math.floor((y - WAVEFORM_H) / PIANO_LANE_H); - return Math.max(pianoRange.lo, Math.min(pianoRange.hi, m)); -} - -// expandOnly=true preserves any wider current range (used during in-place -// edits so adding a low note doesn't collapse the viewport and lose -// previously-clickable upper lanes). Load/import/arrangement-switch call -// without it so the viewport snaps cleanly to the new arrangement. -function updatePianoRange(expandOnly = false) { - const nn = notes(); - // Fretted parts fit the range to SOUNDING pitch; keys keep the packing - // (noteToMidi encodes up to string=5, fret=23 → max 143, matching the - // drag-clamp ceiling). Unresolvable fretted notes are skipped. - const rctx = typeof _rollPitchCtx === 'function' ? _rollPitchCtx() : null; - let lo = 143, hi = 0; - for (const n of nn) { - const m = _rollMidiForNote(n, rctx); - if (m === null) continue; - if (m < lo) lo = m; - if (m > hi) hi = m; - } - if (lo > hi) { - // Empty arrangement: expose the full 88-key range so any starting - // pitch is clickable. Lanes are deliberately thin (~4px) to keep the - // viewport within ~352px — once a note is added the range snaps to - // the actual note range and lanes return to normal height. - pianoRange = { lo: 21, hi: 108, _fromEmpty: true }; - PIANO_LANE_H = 4; - return; - } - // Expand to octave boundaries with padding; ceiling matches drag-clamp max of 143. - let nlo = Math.max(0, Math.floor(lo / 12) * 12 - 6); - let nhi = Math.min(143, Math.ceil((hi + 1) / 12) * 12 + 5); - if (expandOnly && pianoRange && !pianoRange._fromEmpty) { - nlo = Math.min(nlo, pianoRange.lo); - nhi = Math.max(nhi, pianoRange.hi); - } - pianoRange = { lo: nlo, hi: nhi }; - // Adjust lane height to fill available space nicely. Allow down to 4px - // so wide note ranges (many octaves) remain visible without overflowing - // the canvas wrapper. - PIANO_LANE_H = Math.max(4, Math.min(14, 350 / (nhi - nlo + 1))); -} - // Onset-snap tolerance (seconds): how close a note's placement must be to a // detected transient to snap onto it instead of the grid. ~70 ms spans the // grid-vs-attack gap on real recordings without hijacking clearly off-onset diff --git a/tests/keyboard_gutter.test.js b/tests/keyboard_gutter.test.mjs similarity index 94% rename from tests/keyboard_gutter.test.js rename to tests/keyboard_gutter.test.mjs index ef61abf5..65c9cf58 100644 --- a/tests/keyboard_gutter.test.js +++ b/tests/keyboard_gutter.test.mjs @@ -1,4 +1,3 @@ -'use strict'; /* * Tests for the piano-roll keyboard gutter (DAW 4.1): * midiToFreq (equal-tempered pitch), _inKeyboardGutterPure (the click hit @@ -7,13 +6,13 @@ * * All fail on main — none of these exist there. * - * Run: node tests/keyboard_gutter.test.js + * Run: node tests/keyboard_gutter.test.mjs */ -const fs = require('fs'); -const path = require('path'); -const assert = require('assert'); +import assert from 'node:assert'; +import fs from 'node:fs'; +import { _inKeyboardGutterPure, midiToFreq } from '../src/keys.js'; -const src = fs.readFileSync(path.join(__dirname, '..', 'src', 'main.js'), 'utf8'); +const src = fs.readFileSync(new URL('../src/main.js', import.meta.url), 'utf8'); function extractBlock(name) { const re = new RegExp('/\\* @pure:' + name + ':start \\*/[\\s\\S]*?/\\* @pure:' + name + ':end \\*/'); @@ -39,8 +38,7 @@ function t(name, fn) { catch (e) { failed++; console.error(' FAIL ' + name + '\n ' + (e && e.message)); } } -const P = new Function('"use strict";' + extractBlock('midi-freq') - + '\nreturn { midiToFreq, _inKeyboardGutterPure };')(); +const P = { midiToFreq, _inKeyboardGutterPure }; // ── midiToFreq ─────────────────────────────────────────────────────── diff --git a/tests/keyboard_gutter_dblclick.test.js b/tests/keyboard_gutter_dblclick.test.mjs similarity index 89% rename from tests/keyboard_gutter_dblclick.test.js rename to tests/keyboard_gutter_dblclick.test.mjs index dc4ba66d..c171a21b 100644 --- a/tests/keyboard_gutter_dblclick.test.js +++ b/tests/keyboard_gutter_dblclick.test.mjs @@ -1,4 +1,3 @@ -'use strict'; /* * Regression: a DOUBLE-click in the piano-roll keyboard gutter must NOT open * the Add Note dialog. The gutter is audition-only (single-click plays the @@ -12,13 +11,13 @@ * called for a gutter double-click, but IS called for a click in the note area. * Fails on pre-fix src/main.js (showAddNote fires in the gutter). * - * Run: node tests/keyboard_gutter_dblclick.test.js + * Run: node tests/keyboard_gutter_dblclick.test.mjs */ -const fs = require('fs'); -const path = require('path'); -const assert = require('assert'); +import assert from 'node:assert'; +import fs from 'node:fs'; +import { _inKeyboardGutterPure } from '../src/keys.js'; -const src = fs.readFileSync(path.join(__dirname, '..', 'src', 'main.js'), 'utf8'); +const src = fs.readFileSync(new URL('../src/main.js', import.meta.url), 'utf8'); function extractFn(name) { const start = src.indexOf('function ' + name); @@ -38,10 +37,8 @@ function extractBlock(name) { return m[0]; } -// Real _inKeyboardGutterPure straight from the source. -const { _inKeyboardGutterPure } = new Function( - '"use strict";' + extractBlock('midi-freq') + '\nreturn { _inKeyboardGutterPure };' -)(); +// _inKeyboardGutterPure is a real import from src/keys.js; `onDblClick` still +// lives in src/main.js and is still brace-extracted. // Geometry matching src/main.js defaults. const LABEL_W = 52, WAVEFORM_H = 70, PIANO_LANE_H = 10; diff --git a/tests/rename_part.test.js b/tests/rename_part.test.mjs similarity index 94% rename from tests/rename_part.test.js rename to tests/rename_part.test.mjs index 153fce7a..aba0c4b1 100644 --- a/tests/rename_part.test.js +++ b/tests/rename_part.test.mjs @@ -1,4 +1,3 @@ -'use strict'; /* * Tests for the undoable part rename (@pure:rename-arr block + the real * RenameArrangementCmd): renames are display-label edits, undoable, and @@ -9,13 +8,13 @@ * `type`/unknown keys across saves). These fail on main, where none of * this exists. * - * Run: node tests/rename_part.test.js + * Run: node tests/rename_part.test.mjs */ -const fs = require('fs'); -const path = require('path'); -const assert = require('assert'); +import assert from 'node:assert'; +import fs from 'node:fs'; +import { KEYS_PATTERN } from '../src/keys.js'; -const src = fs.readFileSync(path.join(__dirname, '..', 'src', 'main.js'), 'utf8'); +const src = fs.readFileSync(new URL('../src/main.js', import.meta.url), 'utf8'); function extractBlock(name) { const re = new RegExp( @@ -38,8 +37,7 @@ function extractClass(name) { } throw new Error(`unbalanced braces extracting ${name}`); } -const KEYS_PATTERN_SRC = (src.match(/const KEYS_PATTERN = [^\n]+\n/) || [null])[0]; -assert.ok(KEYS_PATTERN_SRC, 'KEYS_PATTERN must exist'); + let pass = 0, fail = 0; function t(name, fn) { @@ -49,10 +47,11 @@ function t(name, fn) { // ── Pure: kind inference + rename guard ────────────────────────────── -const P = new Function( - '"use strict";' + KEYS_PATTERN_SRC + extractBlock('rename-arr') +// KEYS_PATTERN is a real import now; @pure:rename-arr is still in src/main.js. +const P = new Function('KEYS_PATTERN', + '"use strict";' + extractBlock('rename-arr') + '\nreturn { _arrKindPure, _arrSaveKindPure, _renameGuardPure };' -)(); +)(KEYS_PATTERN); t('kind inference mirrors the layout rules: keys > drums > bass > guitar', () => { assert.strictEqual(P._arrKindPure('Piano'), 'keys'); diff --git a/tests/view_switcher.test.mjs b/tests/view_switcher.test.mjs index 6cb6f346..cfba1768 100644 --- a/tests/view_switcher.test.mjs +++ b/tests/view_switcher.test.mjs @@ -1,7 +1,8 @@ /* * Tests for the per-part view switcher + universal read-first piano roll - * (@pure:view-pref block, the isKeysMode/isKeysArr split, _rollMidiForNote, - * the sounding-pitch piano range, and the EditHistory read-only-roll gate). + * (src/keys.js: view-pref resolution, the isKeysMode/isKeysArr split, + * _rollMidiForNote, the sounding-pitch piano range; plus the EditHistory + * read-only-roll gate, which is still sliced out of src/main.js). * * The view became a per-part CHOICE instead of a function of the part's * name: fretted parts may opt into the piano roll (read-first — rendering @@ -12,8 +13,15 @@ */ import assert from 'node:assert'; import fs from 'node:fs'; -import { _soundingPitchPure } from '../src/lanes.js'; +import * as keys from '../src/keys.js'; +import { + _partViewKeyPure, _rollMidiForNote, _viewForPure, isKeysArr, isKeysMode, + noteToMidi, updatePianoRange, viewFor, +} from '../src/keys.js'; +import { LC } from '../src/lanes.js'; +import { S } from '../src/state.js'; +// Only @pure:edit-history is still sliced — it lives in src/main.js. const src = fs.readFileSync(new URL('../src/main.js', import.meta.url), 'utf8'); function extractBlock(name) { @@ -26,19 +34,6 @@ function extractBlock(name) { } return m[0]; } -function extractFn(name) { - const start = src.indexOf('function ' + name); - assert.ok(start >= 0, `function ${name} must exist`); - const open = src.indexOf('{', start); - let depth = 0; - for (let i = open; i < src.length; i++) { - if (src[i] === '{') depth++; - else if (src[i] === '}' && --depth === 0) return src.slice(start, i + 1); - } - throw new Error(`unbalanced braces extracting ${name}`); -} -const KEYS_PATTERN_SRC = (src.match(/const KEYS_PATTERN = [^\n]+\n/) || [null])[0]; -assert.ok(KEYS_PATTERN_SRC, 'KEYS_PATTERN must exist'); let pass = 0, fail = 0; function t(name, fn) { @@ -48,10 +43,7 @@ function t(name, fn) { // ── Pure: view resolution + pref keying ────────────────────────────── -const P = new Function( - '"use strict";' + KEYS_PATTERN_SRC + extractBlock('view-pref') - + '\nreturn { _partViewKeyPure, _viewForPure };' -)(); +const P = { _partViewKeyPure, _viewForPure }; t('keys-named parts are piano-locked regardless of any stored pref', () => { assert.strictEqual(P._viewForPure('Piano', undefined), 'piano'); @@ -75,27 +67,30 @@ t('pref key prefers a stable id over the display name', () => { // ── Stateful: prefs + predicates over stub localStorage ────────────── -function makeViewEnv(S, seed = {}) { +// keys.js reads the real `S` and the ambient `localStorage`. Install a stub on +// globalThis (module code resolves it at call time) and seed the real S, instead +// of re-evaluating the module's source inside a sandbox. +function makeViewEnv(seedS, seed = {}) { const map = new Map(Object.entries(seed)); - const ls = { + globalThis.localStorage = { getItem: k => (map.has(k) ? map.get(k) : null), setItem: (k, v) => { map.set(k, String(v)); }, removeItem: k => { map.delete(k); }, - map, }; - const envSrc = '"use strict";' - + KEYS_PATTERN_SRC + extractBlock('view-pref') - + '\nlet _viewPrefCache = null;\nlet _viewPrefFor = null;\n' - + extractFn('_viewPrefs') + '\n' + extractFn('_viewPrefsSave') + '\n' - + extractFn('viewFor') + '\n' + extractFn('isKeysArr') + '\n' - + extractFn('isKeysMode') + '\n' + extractFn('_rollReadOnly') + '\n' - + 'return { viewFor, isKeysArr, isKeysMode, _rollReadOnly, _viewPrefs, _viewPrefsSave };'; - return { env: new Function('S', 'localStorage', envSrc)(S, ls), ls, S }; + Object.assign(S, seedS); + LC.active = false; + // `_viewPrefs` memoizes per `S.filename`; bounce the key so a fresh seed is + // actually read rather than served from the previous test's cache. + const real = S.filename; + S.filename = '\u0000reset'; + keys._viewPrefs(); + S.filename = real; + return { env: keys, ls: { map }, S }; } t('per-song pref round-trip: override honored, other songs unaffected', () => { - const S = { filename: 'song.sloppak', currentArr: 0, arrangements: [{ id: 'lead', name: 'Lead' }] }; - const { env, ls } = makeViewEnv(S, { + const seed = { filename: 'song.sloppak', currentArr: 0, arrangements: [{ id: 'lead', name: 'Lead' }] }; + const { env, ls, S } = makeViewEnv(seed, { 'editorViewPref:song.sloppak': '{"lead":"piano"}', 'editorViewPref:other.sloppak': '{"lead":"piano"}', }); @@ -110,8 +105,8 @@ t('per-song pref round-trip: override honored, other songs unaffected', () => { }); t('id-keyed pref survives a display rename; keys parts never become read-only', () => { - const S = { filename: 's.sloppak', currentArr: 0, arrangements: [{ id: 'a1', name: 'Lead' }] }; - const { env } = makeViewEnv(S, { 'editorViewPref:s.sloppak': '{"a1":"piano"}' }); + const seed = { filename: 's.sloppak', currentArr: 0, arrangements: [{ id: 'a1', name: 'Lead' }] }; + const { env, S } = makeViewEnv(seed, { 'editorViewPref:s.sloppak': '{"a1":"piano"}' }); S.arrangements[0].name = 'Renamed Solo'; assert.strictEqual(env.viewFor(S.arrangements[0]), 'piano', 'stable id keeps the pref'); S.arrangements[0].name = 'Piano'; // becomes a keys part by name @@ -121,20 +116,14 @@ t('id-keyed pref survives a display rename; keys parts never become read-only', }); t('junk stored JSON never breaks resolution', () => { - const S = { filename: 'x.sloppak', currentArr: 0, arrangements: [{ id: 'a', name: 'Lead' }] }; - const { env } = makeViewEnv(S, { 'editorViewPref:x.sloppak': '[not json' }); + const seed = { filename: 'x.sloppak', currentArr: 0, arrangements: [{ id: 'a', name: 'Lead' }] }; + const { env, S } = makeViewEnv(seed, { 'editorViewPref:x.sloppak': '[not json' }); assert.strictEqual(env.viewFor(S.arrangements[0]), 'string'); }); // ── Sounding-pitch mapping in the roll ─────────────────────────────── -// `_soundingPitchPure` moved to src/lanes.js — inject the REAL one; noteToMidi -// and _rollMidiForNote are still in src/main.js, so they are still sliced. -const M = new Function('_soundingPitchPure', - '"use strict";' - + '\n' + extractFn('noteToMidi') + '\n' + extractFn('_rollMidiForNote') - + '\nreturn { _rollMidiForNote, noteToMidi };' -)(_soundingPitchPure); +const M = { _rollMidiForNote, noteToMidi }; const GUITAR_CTX = { openMidi: [40, 45, 50, 55, 59, 64], tuning: [0, 0, 0, 0, 0, 0], @@ -148,28 +137,35 @@ t('_rollMidiForNote: keys packing without a ctx, sounding pitch with one', () => }); t('piano range fits FRETTED parts to sounding pitch, not the wire packing', () => { - const rangeSrc = '"use strict";' - + extractFn('noteToMidi') + '\n' + extractFn('_rollMidiForNote') + '\n' - + 'let pianoRange = { lo: 36, hi: 96 };\nlet PIANO_LANE_H = 10;\n' - + extractFn('updatePianoRange') + '\n' - + 'return (notesArr, rctx) => {' - + ' globalThis.__vsNotes = notesArr; globalThis.__vsCtx = rctx;' - + ' updatePianoRange();' - + ' return pianoRange;' - + '};'; - const run = new Function( - 'notes', '_rollPitchCtx', '_soundingPitchPure', - rangeSrc - )(() => globalThis.__vsNotes, () => globalThis.__vsCtx, _soundingPitchPure); - // Open low E (sounding 40) + high-e fret 0 (sounding 64), no capo. - const r = run( - [{ string: 0, fret: 0 }, { string: 5, fret: 0 }], - { openMidi: [40, 45, 50, 55, 59, 64], tuning: [0, 0, 0, 0, 0, 0], capo: 0 }); - assert.strictEqual(r.lo, Math.max(0, Math.floor(40 / 12) * 12 - 6), 'floor from sounding 40'); - assert.strictEqual(r.hi, Math.min(143, Math.ceil(65 / 12) * 12 + 5), 'ceiling from sounding 64'); - // The packing would have put string 5 fret 0 at 120 — assert we did NOT. - assert.ok(r.hi < 100, 'range must come from sounding pitch, not string*24+fret'); - delete globalThis.__vsNotes; delete globalThis.__vsCtx; + // updatePianoRange is the SOLE writer of `pianoRange` / `PIANO_LANE_H`; both + // are exported `let`s. Read them back through the namespace to prove the live + // binding updates for importers (a destructured copy would go stale). + S.arrangements = [{ + name: 'Lead', tuning: [0, 0, 0, 0, 0, 0], chords: [], + // Open low E (sounding 40) and open high e (sounding 64), no capo. + notes: [{ string: 0, fret: 0 }, { string: 5, fret: 0 }], + }]; + S.currentArr = 0; + LC.active = false; + updatePianoRange(); + + assert.strictEqual(keys.pianoRange.lo, Math.max(0, Math.floor(40 / 12) * 12 - 6), + 'floor from sounding 40'); + assert.strictEqual(keys.pianoRange.hi, Math.min(143, Math.ceil(65 / 12) * 12 + 5), + 'ceiling from sounding 64'); + // The wire packing would have put string 5 fret 0 at 5*24+0 = 120. + assert.ok(keys.pianoRange.hi < 100, 'range comes from sounding pitch, not string*24+fret'); + assert.ok(keys.PIANO_LANE_H > 0, 'lane height re-derived alongside the range'); +}); + +t('a KEYS part uses the wire packing, not sounding pitch', () => { + S.arrangements = [{ name: 'Keys', tuning: [], chords: [], notes: [{ string: 2, fret: 5 }] }]; + S.currentArr = 0; + LC.active = false; + updatePianoRange(); + const midi = noteToMidi(2, 5); // 53 + assert.ok(keys.pianoRange.lo <= midi && midi <= keys.pianoRange.hi, + 'the packed pitch sits inside the derived range'); }); // ── The read-only-roll gate in EditHistory ───────────────────────────