refuse to build a directory a save names, and read the docs back

`save profiles/neu/x.json` used to create profiles/neu and report success, so a
mistyped name became a tree of empty directories that nothing lists, nothing
cleans up and nothing tells the user about. saveTo now requires the directory to
be there and names it when it is not.

The one exception is the per-user profiles/ directory, which is the tool's own and
is created when missing, because a user who has never saved a profile does not have
it and the first `save lava` is exactly the case that needs it. So the creation
moved out of saveTo and into Save, the name form, and the save command branches on
what resolveProfileTarget said about its argument: a name goes to Save, a path to
saveTo. Two directories, two owners, and no caller that can quietly build one.

Two tests hold it: a path whose directory is missing is refused with the
directory's name and leaves nothing behind, and a first save by name into a user
directory that does not exist yet writes the profile.

Also four things the documentation got wrong, found by reading it against the
code rather than the other way round. AGENTS.md said "three shapes" and then
listed four, and "the three lines" where there are two. The README said both
directories are "read from there and written to there" without the exception, and
that profiles are "in the per-user profiles/ directory" as though no save could
put one anywhere else. The table's first column is padded to 39 for the rows this
series touched; the `load` row overflows it, as the hsv: color row already did.

AGENTS.md

@@ -337,16 +337,16 @@ this reason: it now lists the definitions built into the binary as well as the
 ones in the user directory, and a path alone does not say which of the two a line
 is, because a built-in one has a path relative to the build. A command that emits
 structured data and neither honours `--json` nor says why is the bug this rule
-exists for. Three shapes are deliberately
+exists for. Four shapes are deliberately
 not in that list: `effect` with no effect name routes to the effect list,
-`keyboard fetch` writes a file and prints a line about it, and `save` writes a
-file and says which one — the second only since `save` could name a file by path,
-where a bare "saved" would leave the one thing worth reporting unsaid. `delete`
-writes no file and says which one it removed, for the same reason: a command that
-removes something and prints nothing cannot be told apart from one that removed
-something else. The three lines come in one shape, `savedProfileLine` and
-`deletedProfileLine` next to each other in profile.go, so a file the user asked
-for is always named back to them and annotated by describeDataDir.
+`keyboard fetch` writes a file and prints a line about it, and `save` and
+`delete` say which file they wrote or removed — the latter two only since `save`
+could name a file by path, where a bare "saved" would leave the one thing worth
+reporting unsaid, and where a delete that printed nothing could not be told apart
+from one that removed something else. The two lines come in one shape,
+`savedProfileLine` and `deletedProfileLine` next to each other in profile.go, so a
+file the user asked for is always named back to them and annotated by
+describeDataDir.
 
 `keyboard info` does not open the board, so it must not report the board's
 channels or an effect list from one: it reports the name and whether this tool has

README.md

@@ -48,7 +48,8 @@ qmk-rgb-tool keyboard definitions   # every definition in use, and where each is
 ```
 
 **Where those files live.** Both directories are under the platform's per-user
-configuration directory, and each is read from there and written to there:
+configuration directory, and a name in either is read from there and written to
+there:
 
 | Platform | Directory |
 |---|---|
@@ -75,6 +76,16 @@ the per-user directory and nothing else. `delete` is name-only, so a file outsid
 the per-user directory is read and written and never removed; `rm` is the tool for
 that, and `delete` says which file it removed.
 
+A `save` writes the file and creates no directory for it, so a name in a
+directory that is not there is refused with the directory's name. The per-user
+`profiles/` directory is the one exception and is created when missing, because it
+is this tool's own and the first `save` is the case that needs it:
+
+```bash
+$ qmk-rgb-tool save profiles/typo/x.json
+Error: profile directory profiles/typo does not exist; create it first, or save a name
+```
+
 The Impact 80's definition is additionally built into the binary and consulted
 last, so an installed tool has it; see
 [Effect Names Are Per Board](#effect-names-are-per-board).
@@ -835,7 +846,7 @@ board", and that file is gone, so the field would have been unanswerable:
 | `qmk-rgb-tool --definition <path>`  | Read effect names from this VIA definition file instead of the one in the data directory; applies to every command that resolves names, and a file for another board is refused |
 | `qmk-rgb-tool --json`               | Print JSON instead of text, for `keyboard info`, `info`, `list`, the effect list and `keyboard definitions` |
 | `qmk-rgb-tool -v`, `--version`       | Print the version                          |
-| `qmk-rgb-tool save [name\|file]`      | Save the current state of **every** channel as `<name>.json` in the per-user `profiles/` (name lowercased, non-`[a-z0-9-_]` mapped to `-`, a leading `-` prefixed with `unnamed-`); without a name it writes `default`. An argument ending in `.json` is a path, and that file is written instead, named after the file's own name. It reports the file it wrote, and annotates a path in the per-user directory as such |
+| `qmk-rgb-tool save [name\|file]`      | Save the current state of **every** channel as `<name>.json` in the per-user `profiles/` (name lowercased, non-`[a-z0-9-_]` mapped to `-`, a leading `-` prefixed with `unnamed-`); without a name it writes `default`. An argument ending in `.json` is a path, and that file is written instead, named after the file's own name, in a directory that has to exist. It reports the file it wrote, and annotates a path in the per-user directory as such |
 | `qmk-rgb-tool load <name\|file> [zone]` | Load and apply a profile, by name from the per-user `profiles/` or from a path when the argument ends in `.json`; without a zone it applies the profile to every channel it names, with one it applies it to the named channels only |
 | `qmk-rgb-tool list`                   | List saved profiles                        |
 | `qmk-rgb-tool delete [name]`          | Delete a saved profile; without a name it deletes `default`. A name only, so an argument ending in `.json` names the profile without the dots rather than a file to remove. It reports the file it removed |
@@ -875,12 +886,11 @@ the data directory, and applies to every command that resolves effect names.
 ## Profiles
 
 Profiles store RGB state (effect, brightness, speed, color) per channel as JSON
-files, one per profile, in the per-user `profiles/` directory. They are written
-there and read there, so `save <name>` and `load <name>` always agree, and a
+files, one per profile. A name lives in the per-user `profiles/` directory and is
+written and read there, so `save <name>` and `load <name>` always agree, and a
 checkout's own `profiles/` directory is not searched for a name — you reach it by
 naming the file, with `load profiles/lava.json` or `save profiles/lava.json`. A
-profile also records the
-keyboard it was saved from, because the effect names in
+profile also records the keyboard it was saved from, because the effect names in
 it are that board's:
 
 ```json

cmd/qmk-rgb-tool/profile.go

@@ -132,21 +132,34 @@ func profileFilePath(name string) string {
 }
 
 // Save writes the profile into the per-user directory under its own name, which is
-// what every caller that has a name and no path means.
+// what every caller that has a name and no path means. It creates that directory,
+// which is the one directory a save may create: it is this tool's own, and a user
+// who has never saved a profile does not have it, so the first `save lava` is
+// exactly the case that needs it.
 func (p *Profile) Save() error {
+	if _, err := ensureDataDir(profilesPath()); err != nil {
+		return fmt.Errorf("create profiles directory: %w", err)
+	}
 	return p.saveTo(profileFilePath(p.Name))
 }
 
-// saveTo writes the profile to one file and creates the directory it is in. It
-// resolves nothing: the caller decides where the file goes, so that the rule that
-// decides is the only one there is.
+// saveTo writes the profile to one file, in a directory that has to be there
+// already. A caller that names a path names a directory the tool did not create,
+// and building it turns `save profiles/neu/x.json` with a typo in it into a tree of
+// empty ones that nothing lists and nothing cleans up — reported, meanwhile, as a
+// save. It resolves nothing either: the caller decides where the file goes, so
+// that the rule that decides is the only one there is.
 func (p *Profile) saveTo(path string) error {
 	if p.Name == "" {
 		return fmt.Errorf("profile name is required")
 	}
 
-	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
-		return fmt.Errorf("create profiles directory: %w", err)
+	dir := filepath.Dir(path)
+	if _, err := os.Stat(dir); err != nil {
+		if os.IsNotExist(err) {
+			return fmt.Errorf("profile directory %s does not exist; create it first, or save a name", dir)
+		}
+		return fmt.Errorf("profile directory %s: %w", dir, err)
 	}
 
 	data, err := json.MarshalIndent(p, "", "  ")
@@ -271,9 +284,10 @@ func applyProfileToProfile(proto rgbProtocol, channels []via.Channel, display ma
 // loadProfileFromDevice reads RGB state from device and saves it. Every channel
 // is recorded, so the profile a `load` applies later is the whole keyboard and
 // not the part of it that happened to be named. Warnings go to warn, which is the
-// command's stderr. The file is written where path says and the profile is named
-// after name, so a `save` of a file writes that file and names the profile for it.
-func loadProfileFromDevice(path, name string, warn io.Writer) error {
+// command's stderr. Where the file goes is the caller's decision: isPath is what
+// resolveProfileTarget said about the argument, and a name is the only form that
+// brings a directory with it.
+func loadProfileFromDevice(path, name string, isPath bool, warn io.Writer) error {
 	proto, target, channels, err := openTarget("")
 	if err != nil {
 		return err
@@ -303,7 +317,10 @@ func loadProfileFromDevice(path, name string, warn io.Writer) error {
 	if err := applyProfileToProfile(proto, channels, target.Display, catalog, p); err != nil {
 		return err
 	}
-	return p.saveTo(path)
+	if isPath {
+		return p.saveTo(path)
+	}
+	return p.Save()
 }
 
 // savedProfileLine is what a save says it did. The path is annotated the way every
@@ -329,8 +346,9 @@ func NewProfileSaveCmd() *cobra.Command {
 		Short: "Save current RGB state to a profile",
 		Long: "Read the current RGB settings from every channel of the keyboard and save\n" +
 			"them as a JSON profile. A name is written as <name>.json in the per-user\n" +
-			"profiles/ directory; an argument ending in .json is a path, and that file is\n" +
-			"written where it says. It reports the file it wrote.\n" +
+			"profiles/ directory, which is created if it is not there yet; an argument\n" +
+			"ending in .json is a path, and that file is written where it says, in a\n" +
+			"directory that has to exist already. It reports the file it wrote.\n" +
 			"\n" +
 			"A profile is always complete. Recording only some of the channels would let a\n" +
 			"later `load` apply them and leave the rest as they were, which reads as a\n" +
@@ -341,8 +359,8 @@ func NewProfileSaveCmd() *cobra.Command {
 			if len(args) > 0 {
 				arg = args[0]
 			}
-			path, name, _ := resolveProfileTarget(arg)
-			if err := loadProfileFromDevice(path, name, cmd.ErrOrStderr()); err != nil {
+			path, name, isPath := resolveProfileTarget(arg)
+			if err := loadProfileFromDevice(path, name, isPath, cmd.ErrOrStderr()); err != nil {
 				return err
 			}
 			fmt.Fprint(cmd.OutOrStdout(), savedProfileLine(name, path))

cmd/qmk-rgb-tool/profile_path_test.go

@@ -140,6 +140,9 @@ func TestSaveProfileToWritesThePathItWasGiven(t *testing.T) {
 	t.Cleanup(stubUserConfigDir(t, userDir))
 	t.Cleanup(func() { profilesDirOverride = "" })
 	here := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(here, "profiles"), 0o755); err != nil {
+		t.Fatal(err)
+	}
 	t.Chdir(here)
 
 	path, name, isFile := resolveProfileTarget("profiles/new.json")
@@ -171,6 +174,54 @@ func TestSaveProfileToWritesThePathItWasGiven(t *testing.T) {
 	}
 }
 
+// A save writes a file the user named and creates no directory for it. Building
+// `profiles/neu` for `save profiles/neu/x.json` turns a mistyped name into a tree
+// of empty ones that nothing ever lists and nothing ever cleans up, and the save
+// reports success.
+func TestSaveRefusesToCreateADirectoryForAPath(t *testing.T) {
+	userDir := t.TempDir()
+	t.Cleanup(stubUserConfigDir(t, userDir))
+	t.Cleanup(func() { profilesDirOverride = "" })
+	here := t.TempDir()
+	t.Chdir(here)
+
+	path, name, isFile := resolveProfileTarget("profiles/neu/x.json")
+	if !isFile {
+		t.Fatal(`resolveProfileTarget("profiles/neu/x.json") isPath = false, want true`)
+	}
+	err := (&Profile{Name: name, Version: 1}).saveTo(path)
+	if err == nil {
+		t.Fatal("saveTo into a directory that does not exist = nil error, want one")
+	}
+	if !strings.Contains(err.Error(), filepath.Join("profiles", "neu")) {
+		t.Errorf("saveTo = %q, want it to name the directory that is missing", err)
+	}
+	if _, err := os.Stat(filepath.Join(here, "profiles")); err == nil {
+		t.Error("saveTo created the directory, which is what the save has to stop doing")
+	}
+}
+
+// The per-user directory is the one a save may create, because it is the tool's
+// own and a user who has never saved a profile does not have it. Without this a
+// first `save lava` would fail on the very directory it is meant to fill.
+func TestSaveByNameCreatesTheUserDirectory(t *testing.T) {
+	userDir := t.TempDir()
+	t.Cleanup(stubUserConfigDir(t, userDir))
+	t.Cleanup(func() { profilesDirOverride = "" })
+	t.Chdir(t.TempDir())
+
+	dir := filepath.Join(userDir, "qmk-rgb-tool", "profiles")
+	if _, err := os.Stat(dir); err == nil {
+		t.Fatalf("%s exists before the save, want nothing there", dir)
+	}
+	if err := (&Profile{Name: "lava", Version: 1}).Save(); err != nil {
+		t.Fatalf("Save() with no profiles directory yet: %v", err)
+	}
+	if _, err := os.Stat(filepath.Join(dir, "lava.json")); err != nil {
+		t.Errorf("Save() did not write the profile: %v", err)
+	}
+}
+
 // A delete says which file it removed, and saying so must not make it resolve a
 // path: `delete` is name-only, so `delete lava.json` names no file, it names the
 // profile `lava-json`. A same-named file in the working directory is the thing
@@ -252,7 +303,9 @@ func TestSavedProfileLineNamesTheFileItWrote(t *testing.T) {
 }
 
 // A save by name goes to the user directory, which is the point of the name form:
-// the argument is a name, so it is the directory that decides where it goes.
+// the argument is a name, so it is the directory that decides where it goes. It is
+// Save rather than saveTo that does it, because the directory is the tool's own to
+// make and only the name form is allowed to.
 func TestSaveByNameGoesToTheUserDirectory(t *testing.T) {
 	userDir := t.TempDir()
 	t.Cleanup(stubUserConfigDir(t, userDir))
@@ -264,14 +317,17 @@ func TestSaveByNameGoesToTheUserDirectory(t *testing.T) {
 	if isFile {
 		t.Fatal(`resolveProfileTarget("lava") isPath = true, want false`)
 	}
-	if err := (&Profile{Name: name, Version: 1}).saveTo(path); err != nil {
-		t.Fatalf("saveTo(%q): %v", path, err)
+	if want := profileFilePath(name); path != want {
+		t.Errorf("resolveProfileTarget(lava) path = %q, want %q", path, want)
+	}
+	if err := (&Profile{Name: name, Version: 1}).Save(); err != nil {
+		t.Fatalf("Save(): %v", err)
 	}
 
 	if _, err := os.Stat(filepath.Join(userDir, "qmk-rgb-tool", "profiles", "lava.json")); err != nil {
-		t.Fatalf("saveTo(%q) did not write into the user's directory: %v", path, err)
+		t.Fatalf("Save() did not write into the user's directory: %v", err)
 	}
 	if _, err := os.Stat(filepath.Join(here, "lava.json")); err == nil {
-		t.Error("saveTo wrote into the working directory, which a name does not ask for")
+		t.Error("Save wrote into the working directory, which a name does not ask for")
 	}
 }