# All-or-nothing edits Branch: `atomic-edits` → `main` PR 6 of the review fixes plan (`docs/review-fixes-plan.md`), part of the 0.5.3 series; the version does not change in this PR. ## What changed **Each edit checks first, then changes the project (M3).** An edit that fails now leaves the project exactly as it was, and only an edit that succeeds marks it as having unsaved changes. - **`AssignSample`** works out everything before touching the project: which pool entry to reuse, which entries replacing the pad frees, the sample limit, the new file name, and the new pad, built in full from the blank pad. Only then does it replace the pad and add the pool entry. None of those last steps can fail. - The name of a sample freed by the replacement can still be reused, as before: replacing `kick.wav` with a different `kick.wav` keeps the name instead of making it `kick001.wav`. A new `freedBy` helper works this out, and also handles a file used by two layers of the same pad. - **`SetTune`** checks the pad's first layer is an object before writing. Before, if it was not, the project was marked changed anyway. - **`SetMuteGroup`** marks the project changed only if the write worked. - **`SetSequence`** looks up and checks every clip before writing anything. Before, if a clip had no `value`, the new name, BPM and length were already half written when it failed. The length write is split into `lengthClips` (check) and `writeLength` (write); `setLength`, used by `AddSequence`, uses both. - **`AddSequence` and `RemoveSequence`** set `dirty` after the change rather than before. `RemoveSequence` picks the next current sequence before removing anything. **Project fields are written only by `project` (M4).** - `Dirty` and `Sources` are now the unexported `dirty` and `sources`. - New methods replace them: `Dirty()` reads the flag, `MarkExported()` clears it after an export, and `Source(file)` returns a pool file's source and whether one is recorded. - `app` and `export` use these. The CLI and tools did not use either field. **Tests.** - **`TestFailedEditsChangeNothing`:** each of these edits fails on a loaded DemoFile and leaves the encoded project, the sources and `Dirty()` unchanged: - assigning a missing file; - assigning a file that is not a WAV; - tuning a pad whose layer is not an object; - setting the mute group of a pad that is not an object; - editing a sequence with a broken clip; - a bad slot; - assigning a 129th sample. Against the old ordering, the tune, mute-group and sequence cases fail. - **`TestAssignReplacesBrokenPad`:** the plan's example, a pad whose `layersv` is empty. `AssignSample` does not fail on it: it starts from the blank pad, so the broken pad is replaced whole. - **`TestAssignReusesFreedName`:** the freed-name behaviour above. - **Export tests:** the two that set `Sources` directly now rename a pool entry in the tree, or call the name check and the zip writer directly. ## Notes - **The plan's example does not fail (see the test above).** `AssignSample` replaces the pad with the blank pad before writing, so a pad with no layers is simply replaced. The failures that can actually happen are now all covered. - **The second-line name check in export.** Only `project` can record a source now, so `Folder` and `Zip` can no longer be handed a bad pool name from outside, and their check is reached only if `project` itself has a bug. The test calls the check and the zip writer directly. `internal/export` coverage is 85.9%, still above PR 3's 85%. - **One test still writes a field from outside `project`:** `export_test.go` sets `Name` to `a/b` to test the export name check, since `SetName` refuses such a name. No non-test code outside `project` writes a project field. `Name` and `SourceDir` stay exported, as the plan only names `Dirty` and `Sources`. - **Nothing changes in the files MPCkit writes.** The golden tests (MpckitT1, T3, Demo02) and the round-trip tests pass unchanged. ## Testing **On the server, every command with `GOTOOLCHAIN=go1.27.4`:** - `go test ./...` passes, including the golden and round-trip tests. - `go vet ./...` (also with `GOOS=windows`) and `staticcheck ./...` are clean. - `govulncheck ./...`: "No vulnerabilities found." - `npm install`, `npm run check` (0 errors, 0 warnings) and `npm run build` pass. The lockfile was not committed. The bindings did not change. - `wails3 build` builds, and the app starts under Xvfb. **On the laptop (PowerShell, in the repo):** 1. `git fetch; git checkout atomic-edits` 2. `go test ./...`: all `ok`. 3. `wails3 dev`: load `testdata\device\DemoFile.xpj`. The footer shows no unsaved changes. 4. Assign a file that is not a WAV to pad A1, for example by renaming a text file to `.wav`. An error shows, A1 keeps its sample, and the footer still shows no unsaved changes. 5. Assign a real WAV to A1: it replaces the sample, and the footer shows unsaved changes. 6. Change a tune, a mute group, and a sequence's name, BPM and length. Each works as before. Export to folder, then load the export: the changes are there.