# 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 `Set`s 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.