mpckit/pull-requests/go-readability/go-readability.md

Go readability

Branch: go-readability → main

PR 10 of the review fixes plan (docs/review-fixes-plan.md), a readability PR: no behaviour change, no version change. It was done after PR 11; the two touch different files.

What changed

Four commits, one per area:

1. Service.state split into named helpers (R3, R4).

   • The helpers: state() in internal/app/app.go used p, v, q, f, m and st across a 58-line function. It now builds the State from four helpers:
     • limits();
     • missingSources(proj), the pool files export could not copy;
     • padState(proj, slot, missing);
     • sequenceStates(proj).
       

   They use names such as proj, view and seq.

   • The rename: Service.edit becomes apply.

2. Clearer names and notes in xpj and project (R4, R6).

   • Renames, with the comments that mention them:

   | Was | Now | What it does | | --- | --- | --- | | xpj.wrap | wrapErr | Prefixes JSON errors with xpj:. | | project.wrap | fromFile | Builds a Project from a decoded file. | | project.edit (type) | setter | Applies several Sets and keeps the first error. |

   Together with apply, no two things share a name any more.

   • uniqueFile: it kept taken file names and sample names in one map, told apart by a "\x00" suffix. Now it uses two maps, takenFiles and takenNames, and its comment says both kinds of name are kept unique.
   • The BPM check: !(bpm >= MinBPM && bpm <= MaxBPM) now has a comment saying it is written that way so NaN, for which every comparison is false, is refused.

3. main() reads as a list of steps (R5).

   • main() in main.go goes from 96 lines to 12. It calls four new functions, each keeping the comment that was there:
     • newApp: the application options and the panic handler;
     • newWindow: the hidden window, sized and shown once the screens are known;
     • guardClose: the unsaved-changes guard;
     • handleFileDrops: the file-drop handler.
   • The steps run in the same order as before.

4. The CLI's flags each have a named parse function (R5).

   • Parsing: init() in cmd/mpckit-cli/main.go was 104 lines of closures. Each flag now has its own function: parsePadOp, parseTuneOp, parseMuteOp, parseClearOp, parseCopyOp, parseMoveOp, parseSwapOp, parseSeqOp and parseRmseqOp. init() registers them from a nine-row table.
   • main(): it was 62 lines, over the plan's 60. It now hands opening the project to openProject and exporting to write.
   • Unchanged: flag names, usage text, error messages and output.

Longest functions in main.go, cmd/mpckit-cli/main.go and internal/app/app.go:

	Longest	Next
Before (main)	CLI init 104 lines	main() 96, CLI main 62, state 58
Now	28 lines: padState, parseSeqOp and the CLI main	newApp and openProject, 25 each

Notes

• Name choices. I used the plan's names: wrapErr, fromFile, apply and setter.
  • New in the CLI: a function type for one flag's change to the project, which I named opFunc, beside the existing op and opFlag. I avoided calling it edit, which this PR removes as a name elsewhere.
  • The two CLI helpers: openProject and write.
• Two small changes in how the CLI's parse functions are written, with the same result:
  • -clear and -rmseq used to return a function together with the error. They now return nil on error. flag never stored the function when there was an error, so nothing a user sees changes.
  • -tune's long out-of-range message is now split across two lines of source; the text is the same.
• Not changed: what each function does, the order of main()'s steps, and the tests. load_test.go changes only for the fromFile rename.

Testing

On the server, every command with GOTOOLCHAIN=go1.27.4:

• go test ./... passes. No test was removed or loosened; one renamed call and comment in load_test.go.
• go vet ./... (also with GOOS=windows) and staticcheck ./... are clean.
• govulncheck ./...: "No vulnerabilities found."
• In frontend: npm ci, npm run check (0 errors, 0 warnings), npm run build and npm run format:check pass. The frontend is unchanged.
• wails3 build builds and the app starts under Xvfb.

"No behaviour change", checked against main before the first commit:

• Exports are byte-identical. I ran go run ./cmd/mpckit-cli -load testdata/device/DemoFile.xpj -out <dir> on main and after the last commit. cmp finds the two DemoFile.xpj identical, and diff -r finds the data folders identical.
• go run ./cmd/mpckit-cli -h prints exactly the same text.
• 22 CLI runs give the same results. Two runs use every flag (one exports to a folder, one to a zip). The rest are 15 bad flag values, an existing export with and without -force, a stray argument, both -out and -zip, and no -name. For each run I compared stdout, stderr, exit code, the exported files, and the zip's entries and their contents (the zip itself stores its write time).
• The State the window gets is byte-identical. A scratch test, not committed, wrote the full State JSON after loading DemoFile, ten edits, two refused edits, loading a copy with a missing WAV, and the refused export.
• No bound method changed: TestBoundMethods passes untouched, and frontend/bindings did not change.

On the laptop (PowerShell, in the repo). This changes the window set-up in main.go, so watch the window size, the close prompt and file drop in particular:

1. git fetch; git checkout go-readability
2. go test ./...: all ok.
3. go run ./cmd/mpckit-cli -h: the same usage text as before.
4. wails3 dev. The window opens at the usual size, centred, with nothing scrolling.
5. About: open About and check the version and links. Close it with Escape.
6. Pads tab: load testdata\device\DemoFile.xpj.
   • Click pads to hear them. The pad panel shows names and files.
   • Change a tune and a mute group.
   • Drop a WAV onto an empty pad: it is assigned. Drop a non-WAV file: "Only WAV files can be assigned to a pad." shows.
   • Move/Copy to pad: the picker opens. Drag A1 onto A5: the notes warning shows; Cancel.
7. Sequences tab:
   • Select Intro and Sequence 02.
   • Change a name and a BPM.
   • Set Sequence 02 to 1 bar: the "Shorten…" dialog shows; Cancel.
   • Add a sequence in B5, then remove it: the "Remove…" dialog shows.
8. Export:
   • Export to a new folder.
   • Export there again and Cancel at "Replace existing files?".
   • Export a zip.
9. Missing file: load a copy of DemoFile with one WAV removed. The missing badge and the footer warning show, and export is disabled.
10. New and close:
    • New, with unsaved changes: the confirm dialog shows.
    • Make a change, then close the window: "Close without exporting?" shows. Cancel keeps the app open; confirming closes it.