浏览代码

Merge branch 'plan-assign-names' of bajdev/mpckit into main

bajdev 14 小时之前
父节点
当前提交
d13842e3eb
共有 2 个文件被更改,包括 27 次插入 和 2 次删除
  1. 4 2
      docs/review-fixes-plan.md
  2. 23 0
      pull-requests/plan-assign-names/add-assign-name-check-to-pr-3.md

+ 4 - 2
docs/review-fixes-plan.md

@@ -86,7 +86,7 @@ Each PR starts from `main` after the previous one is merged. The order matters i
 | --- | --- | --- | --- |
 | 1 | `harden-load` | S1 (load side), S3, M2 | 0.5.2 |
 | 2 | `limit-decompression` | S2 | 0.5.2 |
-| 3 | `safe-export` | S1 (export side), M1, M5, R1 (part) | 0.5.2 |
+| 3 | `safe-export` | S1 (export side and assign), M1, M5, R1 (part) | 0.5.2 |
 | 4 | `release-0-5-2` | Version 0.5.2 | 0.5.2 |
 | 5 | `unbind-paths` | S4 | 0.5.3 |
 | 6 | `atomic-edits` | M3, M4 | 0.5.3 |
@@ -129,12 +129,13 @@ Each PR starts from `main` after the previous one is merged. The order matters i
 
 ### PR 3. Safe export (`safe-export`)
 
-**Why.** Export copies each pool file from its source and writes it into the data folder by name (S1, export side). It follows symlinks, so a link in a shared kit's data folder exports whatever it points to. With overwrite set, `Folder` deletes the old export before moving the new one in; if a rename fails, both are lost, although the comment promises otherwise (M1, R1). The `export` package has the lowest coverage, 74.2%, mostly on these paths (M5).
+**Why.** Export copies each pool file from its source and writes it into the data folder by name (S1, export side). It follows symlinks, so a link in a shared kit's data folder exports whatever it points to. With overwrite set, `Folder` deletes the old export before moving the new one in; if a rename fails, both are lost, although the comment promises otherwise (M1, R1). The `export` package has the lowest coverage, 74.2%, mostly on these paths (M5). Found after PR 1: `AssignSample` names a new pool file after the WAV's own file name, so on Linux or macOS a file called `kick\one.wav` is accepted and exported, and loading that export then fails PR 1's check. On Windows and on the device's card the backslash would also read as a folder separator.
 
 **Tasks**
 
 - [ ] In `collect`, use `os.Lstat` and refuse a source that is a symlink or not a regular file; report it like a missing file.
 - [ ] In `Folder` and `writeZip`, refuse a pool name that is not a plain file name (same rule as PR 1), as a second line of defence.
+- [ ] In `AssignSample`, make the new pool file name pass `PlainFileName` before `uniqueFile`: replace `/`, `\` and control characters with `_`; if the result is empty, `.` or `..`, use `sample.wav`. The sample name (`stem`) follows from the cleaned file name. Do this in one helper in `project` with a comment giving the reason.
 - [ ] Rewrite the overwrite step in `Folder`: rename the old `.xpj` and data folder to temporary names inside the temp folder, rename the new pair into place, then delete the old pair. If the second rename fails, put the old pair back.
 - [ ] If `dest` does not exist, return `export folder does not exist: <dest>` instead of the bare `stat` error.
 - [ ] Correct the `Folder` comment to say what the code now guarantees.
@@ -142,6 +143,7 @@ Each PR starts from `main` after the previous one is merged. The order matters i
 
 **Done when**
 
+- Assigning a WAV named `kick\one.wav` (create it in the test; Linux allows the backslash) gives the pool name `kick_one.wav`, and the exported project loads again without error.
 - Export of a project whose data folder holds a symlink fails and writes nothing.
 - An overwrite that fails leaves the previous export complete.
 - `go test -cover ./internal/export` shows at least 85%.

+ 23 - 0
pull-requests/plan-assign-names/add-assign-name-check-to-pr-3.md

@@ -0,0 +1,23 @@
+# Plan: check new sample file names at assign (PR 3)
+
+Branch: `plan-assign-names` → `main`
+
+## What changed
+
+`docs/review-fixes-plan.md`, PR 3 (`safe-export`) gets one more task and done criterion.
+
+Found while checking PR 1: `AssignSample` names a new pool file after the WAV's own file name. On Linux or macOS a file can be called `kick\one.wav`; MPCkit accepts and exports it, and loading that export fails PR 1's check (`samples[0].path: file name "kick\\one.wav" contains a folder separator`). On Windows and on the device's card the backslash would also read as a folder separator.
+
+- **Task:** in `AssignSample`, clean the new pool file name so it passes `PlainFileName`: `/`, `\` and control characters become `_`, and an empty, `.` or `..` result becomes `sample.wav`.
+- **Done when:** assigning `kick\one.wav` gives `kick_one.wav`, and the exported project loads again.
+- The PR table now lists PR 3 as "S1 (export side and assign)".
+
+No code changes.
+
+## Notes
+
+Merge this before the PR 3 agent starts. It does not affect PR 2.
+
+## Testing
+
+Documentation only. Reproduced the problem with `mpckit-cli` on `main` (`89e2955`): `-pad 'A1=kick\one.wav'` exports, and `-load` of that export fails as above.