Procházet zdrojové kódy

Rename samples named after Windows devices

bajdev chose to also rename a new sample whose name Windows reserves for
a device: CON.wav becomes CON_.wav. ValidName and poolFileName now share
one list, reservedOnWindows. Updates the plan's Decisions table and the
PR record.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude před 13 hodinami
rodič
revize
08bd7ce725

+ 1 - 1
docs/review-fixes-plan.md

@@ -15,7 +15,7 @@ These were settled with the user on 2026-11-12. Agents follow them and do not re
 | CI (S6) | Out of scope. No CI files, no `task check`. |
 | PR records (R8) | Keep the current layout. R8 is dropped. |
 | Linked WAVs (PR 3) | A WAV the user picks that is a link is followed at assign: the pool file is named after the link, and its source is the file the link points to. Export refuses any source that is a link or sits in a linked folder. Decided 2026-11-12. |
-| New sample names (PR 3) | Besides `/`, `\` and control characters, the characters the device's FAT card forbids (`: * ? " < > \|`) also become `_`. Decided 2026-11-12. |
+| New sample names (PR 3) | Besides `/`, `\` and control characters, the characters the device's FAT card forbids (`: * ? " < > \|`) also become `_`, and a name Windows reserves for a device (`CON.wav`, `aux.wav`, `com1.wav`) gets `_` added (`CON_.wav`). Decided 2026-11-12 and 2026-11-13. |
 
 Open: none. An agent that meets a choice this plan does not settle stops and asks bajdev in the PR record's Notes, instead of guessing.
 

+ 13 - 4
internal/project/pads.go

@@ -193,10 +193,12 @@ func (p *Project) inPool(file string) bool {
 
 // poolFileName turns the name of a WAV the user picked into a pool file
 // name that passes PlainFileName on every system and is legal on the
-// device's FAT card. Linux and macOS allow names such as "kick\one.wav" or
-// "a:b.wav"; exported as they are, the backslash reads as a folder on
-// Windows and the card, the colon cannot be written to the card, and Load
-// refuses the project. Each such character becomes "_".
+// device's FAT card. Linux and macOS allow names such as "kick\one.wav",
+// "a:b.wav" or "con.wav"; exported as they are, the backslash reads as a
+// folder on Windows and the card, the colon cannot be written to the card,
+// Windows cannot create a file named after a device, and Load refuses the
+// project. Each such character becomes "_", and a device name gets "_"
+// added ("con.wav" becomes "con_.wav").
 func poolFileName(name string) string {
 	clean := strings.Map(func(r rune) rune {
 		if unicode.IsControl(r) || strings.ContainsRune(`/\:*?"<>|`, r) {
@@ -207,6 +209,13 @@ func poolFileName(name string) string {
 	if clean == "" || clean == "." || clean == ".." {
 		return "sample.wav"
 	}
+	if reservedOnWindows(clean) {
+		base, ext, _ := strings.Cut(clean, ".")
+		clean = base + "_"
+		if ext != "" {
+			clean += "." + ext
+		}
+	}
 	return clean
 }
 

+ 11 - 5
internal/project/project.go

@@ -296,17 +296,23 @@ func ValidName(name string) error {
 	if strings.HasSuffix(name, ".") || strings.HasSuffix(name, " ") {
 		return errors.New("name cannot end with a dot or a space")
 	}
-	base := strings.ToUpper(name)
-	if i := strings.IndexByte(base, '.'); i >= 0 {
-		base = base[:i]
+	if reservedOnWindows(name) {
+		return fmt.Errorf("%q is a reserved name on Windows", name)
 	}
+	return nil
+}
+
+// reservedOnWindows reports whether Windows treats name as a device, such as
+// "CON" or "com1.wav": the part before the first dot is what counts.
+func reservedOnWindows(name string) bool {
+	base, _, _ := strings.Cut(strings.ToUpper(name), ".")
 	switch base {
 	case "CON", "PRN", "AUX", "NUL",
 		"COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9",
 		"LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9":
-		return fmt.Errorf("%q is a reserved name on Windows", name)
+		return true
 	}
-	return nil
+	return false
 }
 
 // Slot helpers. A slot is 0-127: bank x 16 + pad - 1.

+ 8 - 2
internal/project/project_test.go

@@ -581,13 +581,19 @@ func TestPoolFileName(t *testing.T) {
 		"":                "sample.wav",
 		".":               "sample.wav",
 		"..":              "sample.wav",
+		"CON.wav":         "CON_.wav",
+		"aux.wav":         "aux_.wav",
+		"com1.loop.wav":   "com1_.loop.wav",
+		"Lpt9":            "Lpt9_",
+		"console.wav":     "console.wav",
+		"com10.wav":       "com10.wav",
 	} {
 		got := poolFileName(in)
 		if got != want {
 			t.Errorf("poolFileName(%q) = %q, want %q", in, got, want)
 		}
-		if err := PlainFileName(got); err != nil {
-			t.Errorf("poolFileName(%q) = %q: %v", in, got, err)
+		if err := PlainFileName(got); err != nil || reservedOnWindows(got) {
+			t.Errorf("poolFileName(%q) = %q: not a safe name", in, got)
 		}
 	}
 }

+ 3 - 2
pull-requests/safe-export/safe-export-and-clean-sample-names.md

@@ -15,7 +15,7 @@ PR 3 of the review fixes plan (`docs/review-fixes-plan.md`). Part of 0.5.2; the
 
 **New sample names are cleaned at assign (S1, assign).**
 - A new pool file name is passed through `poolFileName` in `project` before the 001, 002 suffixes are added. A comment there gives the reason.
-- The helper replaces `/`, `\`, control characters, and the characters the device's FAT card forbids (`: * ? " < > |`) with `_`. A name that comes out empty, `.` or `..` becomes `sample.wav`. The sample name follows from the cleaned file name.
+- The helper replaces `/`, `\`, control characters, and the characters the device's FAT card forbids (`: * ? " < > |`) with `_`. A name Windows reserves for a device (`CON`, `PRN`, `AUX`, `NUL`, `COM1`–`COM9`, `LPT1`–`LPT9`, in any case and with any extension) gets `_` after that part, so `CON.wav` becomes `CON_.wav`. A name that comes out empty, `.` or `..` becomes `sample.wav`. The sample name follows from the cleaned file name.
 - So `kick\one.wav`, which Linux and macOS allow, becomes `kick_one.wav`, and its export loads again.
 - Load still accepts existing names; only new assignments change.
 
@@ -55,7 +55,8 @@ Against the old code, every new export test fails.
   - Samples you assign are unaffected, since their paths are resolved at assign.
 - **Making a rename fail.** The plan suggested a directory where the `.xpj` should go. With the new overwrite, that directory is simply moved aside and replaced, so it no longer fails. The tests instead swap the package's `rename` (normally `os.Rename`) for one that fails on chosen calls. This reaches every step on every system.
 - **The source shown for a linked WAV.** It is the file the link points to, not the link.
-- **Not covered:** a WAV named like a Windows device (`CON.wav`, `aux.wav`) is still taken as it is. It can be written on Linux and macOS, but not on Windows. Say if you want it renamed too.
+- **Windows device names (bajdev, 2026-11-13).** These are renamed too. `ValidName` and `poolFileName` now share one list, `reservedOnWindows`.
+- **What the user sees.** Renaming is silent, like the 001 suffixes added for duplicate names. The pad shows the new name (`kick_one`), so the change is visible but not explained. Only names that could not be exported as they are change, and none of them can be created on Windows anyway.
 - **On Windows, the link tests skip** unless developer mode is on, because creating links needs it. `TestBackslashNameExportsAndLoads` also skips there, because Windows file names cannot hold `\`.
 
 ## Testing