Quellcode durchsuchen

take the zone as an argument, and let it name several channels

--zone and --all are gone. A flag on the root is a flag every help lists,
so `keyboard info` and `list` were told about a channel neither can address
and ignored it; a positional argument is spelled only where it is read. The
zone is a parameter of prepareTarget and openTarget rather than a package
variable, so no command can reach the wrong one by accident.

The argument is one name or several, comma separated, and `all` is every
channel the keyboard reports. The channels come back in channel order
whatever the order they were written in, because a script that reads
`side,logo` and one that reads `logo,side` are asking for the same thing.
A name twice is one channel, spaces around the commas are ignored, and a
channel the keyboard does not have is refused by name, every missing one of
a list, so nothing is written on the way to a failure.

The argument count is what refuses a write without a target, and it refuses
it before the keyboard is opened. That is why the message can name the
vocabulary: a command cannot list the channels the keyboard has without
asking it, so the fixed subsystem names are all the error ever needed.
requireSelection and the two flags' mutual exclusion go with it.

`effect <zone>` lists and `effect <zone> <name>` sets, so the count is what
tells them apart and `--list` has nothing left to do. A zone named and an
effect that channel does not have is refused, `all` included: `effect all`
asks for every channel, so a name only one of them has is a request the tool
cannot carry out rather than a reason to write the others and report it as
done. `load` still warns and skips, because a whole profile is being
applied there. `load` takes a name and at most a zone, and `save` writes
every channel: a partial profile is applied partially by a later load, which
reads as a zone the profile had nothing to say about.

The zone completion now consults the built-in definitions as well as the
data directory. `go install` creates no data directory, so a shell offered
a board only the subsystem names, which are not the names a renamed channel
answers to.

Also restores a writer test set on the shipped `keyboard info` command to
nil rather than to os.Stdout: that left the command with a writer of its
own, which a root's SetOut can no longer override, and the help went to the
terminal instead of the buffer a later test believed it had captured.
Paul-Dieter Klumpp vor 1 Woche
Ursprung
Commit
5c322358c3
36 geänderte Dateien mit 1377 neuen und 423 gelöschten Zeilen
  1. 29 16
      .claude/skills/qmk-rgb/SKILL.md
  2. 45 19
      AGENTS.md
  3. 127 80
      README.md
  4. 2 2
      cmd/qmk-rgb-tool/agents_doc_test.go
  5. 86 18
      cmd/qmk-rgb-tool/arity_test.go
  6. 8 9
      cmd/qmk-rgb-tool/brightness.go
  7. 5 11
      cmd/qmk-rgb-tool/brightness_summary_test.go
  8. 1 1
      cmd/qmk-rgb-tool/brightness_verify_test.go
  9. 131 18
      cmd/qmk-rgb-tool/channels_test.go
  10. 9 10
      cmd/qmk-rgb-tool/color.go
  11. 22 25
      cmd/qmk-rgb-tool/color_verify_test.go
  12. 38 12
      cmd/qmk-rgb-tool/completion.go
  13. 112 4
      cmd/qmk-rgb-tool/completion_test.go
  14. 3 1
      cmd/qmk-rgb-tool/definition.go
  15. 1 1
      cmd/qmk-rgb-tool/definition_test.go
  16. 6 6
      cmd/qmk-rgb-tool/disable.go
  17. 26 30
      cmd/qmk-rgb-tool/effect.go
  18. 5 4
      cmd/qmk-rgb-tool/effect_list_test.go
  19. 61 28
      cmd/qmk-rgb-tool/effect_test.go
  20. 4 6
      cmd/qmk-rgb-tool/effect_verify_test.go
  21. 8 7
      cmd/qmk-rgb-tool/enable.go
  22. 39 18
      cmd/qmk-rgb-tool/flags_test.go
  23. 11 5
      cmd/qmk-rgb-tool/info.go
  24. 33 3
      cmd/qmk-rgb-tool/keyboard_info_test.go
  25. 66 8
      cmd/qmk-rgb-tool/main.go
  26. 34 23
      cmd/qmk-rgb-tool/profile.go
  27. 1 1
      cmd/qmk-rgb-tool/profile_load_test.go
  28. 19 11
      cmd/qmk-rgb-tool/rgb.go
  29. 53 19
      cmd/qmk-rgb-tool/rgb_test.go
  30. 8 9
      cmd/qmk-rgb-tool/speed.go
  31. 6 10
      cmd/qmk-rgb-tool/speed_verify_test.go
  32. 4 2
      cmd/qmk-rgb-tool/target_seam_test.go
  33. 304 0
      cmd/qmk-rgb-tool/zone_selection_test.go
  34. 67 3
      cmd/qmk-rgb-tool/zones.go
  35. 2 2
      internal/rgb/catalog.go
  36. 1 1
      internal/rgb/catalog_test.go

+ 29 - 16
.claude/skills/qmk-rgb/SKILL.md

@@ -23,8 +23,8 @@ The tool is the single source of truth. Always start by fetching its output:
 
 ```bash
 qmk-rgb-tool --help                           # Command structure
-qmk-rgb-tool effect --list                    # Effects per zone, one per line
-qmk-rgb-tool --json effect --list             # The same, as JSON for parsing
+qmk-rgb-tool effect all                       # Effects per zone, one per line
+qmk-rgb-tool --json effect all                # The same, as JSON for parsing
 qmk-rgb-tool keyboard info                    # Connected keyboards
 qmk-rgb-tool info                             # Current RGB state
 qmk-rgb-tool keyboard definitions            # Which definition files are in use, and where each is from
@@ -35,7 +35,7 @@ Present the raw output to the user. Never duplicate documentation — the tool o
 ### 2. If the tool is not built or no keyboard is connected
 
 These are two different problems and need different commands. `qmk-rgb-tool
-effect --list` opens the keyboard, so it fails when none is connected; use
+effect all` opens the keyboard, so it fails when none is connected; use
 `keyboard info`, which does not.
 
 If the tool is not built:
@@ -49,8 +49,11 @@ If no keyboard is connected:
 - Identify the channel: `backlight`, `rgblight`, `rgb_matrix`, `audio` or
   `led_matrix`, or the name this keyboard's definition gives it — on the Impact
   80 that is `logo`, `Backlight` or `side`, and the name matches ignoring case
-- Ask for confirmation if the action will affect every channel
-- Run the command: `qmk-rgb-tool effect breathing --zone rgb_matrix`
+- The zone is the **first argument** of every command that addresses a channel;
+  every such command requires one. Several channels are written comma separated
+  (`logo,side`), and `all` means every channel the keyboard reports
+- Ask for confirmation before using `all`, which changes every channel
+- Run the command: `qmk-rgb-tool effect rgb_matrix breathing`
 - Return the output
 
 ### 4. Profiles
@@ -61,10 +64,11 @@ and written to, so a `save` is a `load` away. A checkout's own `profiles/`
 directory is not used. They persist RGB state across reboots.
 
 ```bash
-qmk-rgb-tool list       # List saved profiles
-qmk-rgb-tool save name  # Save current state
-qmk-rgb-tool load name  # Apply a profile
-qmk-rgb-tool delete name # Remove a profile
+qmk-rgb-tool list            # List saved profiles
+qmk-rgb-tool save name       # Save the state of every channel
+qmk-rgb-tool load name       # Apply a profile to every channel it names
+qmk-rgb-tool load name side  # Apply it to one channel or a list of them only
+qmk-rgb-tool delete name     # Remove a profile
 ```
 
 ## Setting up a keyboard whose effect names are missing
@@ -97,10 +101,19 @@ rebuild.
 
 ## Zone behavior
 
-`--zone` names a VIA lighting channel, and the tool asks the keyboard which
-channels it has. Without `--zone`, commands target every channel it reports, in
-channel order. With `--zone`, commands target exactly that channel. `--zone` is
-not remembered between runs, so repeat it on every invocation.
+A command that writes lighting names its target as its first argument: one
+channel, several comma separated, or `all` for every channel the keyboard reports.
+The channels are written in channel order however the names were spelled, so a
+script sees one order for every way of writing the same selection. Naming no zone
+is refused by the argument count before the keyboard is even opened, and a channel
+the keyboard does not have is refused by name. `info`, `effect` without a name and
+`save` only read, so a zone is optional there; `load` takes a profile name and at
+most a zone.
+
+An effect a named channel does not have fails rather than being skipped, `all`
+included — so `effect all` with a name only one channel has is refused rather
+than writing the others. `load` is the exception: it warns on stderr and skips,
+because a whole profile is being applied.
 
 Effect names come from a VIA definition file, read at runtime from the per-user
 `definitions/` directory; the Impact 80's is also built into the binary so an
@@ -112,9 +125,9 @@ the tool's: the Impact 80's file writes `fixed wave`,
 reports each channel as written while accepting the tool's spellings too.
 
 On the Impact 80 the `rgblight` and `audio` channels carry fewer effects (0–6)
-than `rgb_matrix` (0–45, and the board takes a 46th the file does not name). Always check `qmk-rgb-tool effect --list` to see what is
+than `rgb_matrix` (0–45, and the board takes a 46th the file does not name). Always check `qmk-rgb-tool effect all` to see what is
 available per channel, and note that a keyboard with no catalog has no effect
-names at all — `effect <index>` is the way to set one there. The list prints the
+names at all — `effect <zone> <index>` is the way to set one there. The list prints the
 file the names came from as its last line.
 
 ## Values the keyboard changes
@@ -129,7 +142,7 @@ These commands read the value back. When a zone differs, the output names what
 was actually applied:
 
 ```
-$ qmk-rgb-tool brightness 200
+$ qmk-rgb-tool brightness all 200
 Brightness logo 160 backlight 255 side 160 (requested 200)
 ```
 

+ 45 - 19
AGENTS.md

@@ -19,7 +19,7 @@ to go stale. So the split is:
 - `.claude/skills/qmk-rgb/SKILL.md` — the live-query workflow for the tool.
 
 For the effect catalog specifically, read it from no document at all. Run
-`qmk-rgb-tool effect --list` and use the output. Logo and Side accept fewer
+`qmk-rgb-tool effect all` and use the output. Logo and Side accept fewer
 effects than Backlight, and only the tool knows what the current firmware
 supports.
 
@@ -64,10 +64,10 @@ per-user directory.
 
 `keyboard info` does not open the board, so it must not report the board's
 channels: it does not know them, and printing the ones a definition names would
-claim more than it knows. `info` and `effect --list` open the board and report
+claim more than it knows. `info` and the effect list open the board and report
 them.
 
-No command validates `--zone` or `--device` before the command that needs it
+No command validates the zone or `--device` before the command that needs it
 does. A root-level pre-run would make `keyboard info`, `list`, `delete` and
 `completion` demand a keyboard, and the one you run to choose a keyboard cannot
 require that you have chosen one. Where two errors apply, the board wins: a
@@ -81,16 +81,42 @@ otherwise they fail and list the selectable numbers, so a command never targets
 an unintended keyboard. Device numbers are stable for the current session only —
 HID paths are reassigned on reboot and most keyboards report no serial number.
 
-`--zone` takes a VIA lighting channel, named by its QMK subsystem, and a board's
-definition file may name a channel differently. README.md carries the
-vocabulary. It is a persistent flag in cobra's sense only — accepted on the
-root and inherited by subcommands — and nothing is remembered between runs, so
-it must be repeated on every invocation. Without `--zone`, commands target every
-channel the keyboard reports, in channel order; the channels are discovered by
-asking the keyboard, not assumed. With `--zone`, commands target exactly one
-channel. A default command skips a target that does not support an effect and
-warns on stderr; an explicitly named channel that does not support it, and an
-unknown name, fail before the device is opened.
+The zone is a **positional argument**, not a flag, and that is not a style choice.
+It was a flag, on the root, and a flag on the root is a flag every help lists:
+`keyboard info` and `list` were told about a channel neither can address, and
+ignored it. An argument is spelled only where it is read, so there is nothing to
+ignore. `zoneArgs` in `cmd/qmk-rgb-tool/main.go` is the one place an argument
+count is declared, and it prints the usage line on a wrong count, because cobra's
+own message does not say what the missing argument should have been.
+
+The zone takes a VIA lighting channel, named by its QMK subsystem, and a board's
+definition file may name a channel differently. README.md carries the vocabulary.
+Several channels are written comma separated and `all` means every channel the
+keyboard reports; `all` next to another name is `all`, since the union is the
+whole keyboard. Both live in `resolveZoneName` in `cmd/qmk-rgb-tool/zones.go`, and
+so does the fact that a list comes back in channel order however it was written —
+one order for every spelling of one selection, which is what a JSON consumer
+parses. The channels themselves are discovered by asking the keyboard, not
+assumed, and a name that resolves to a channel the board does not have is refused
+by name, every missing one of a list, so nothing is written on the way to a
+failure.
+
+A command that writes lighting cannot be called without a zone, and the argument
+count is what keeps it that way: `brightness 160` fails before the keyboard is
+opened. A command that only reads — `info`, `effect` without a name, `save` —
+takes no zone and reports every channel, which is what they are for. `load` takes
+a profile name and at most a zone, and applies the profile to the channels the
+zone names. Write the arity into a new command that writes, or the new command
+will be the one command that writes every channel by default.
+
+The zone is a parameter of `prepareTarget` and `openTarget` rather than a
+package-level variable, which is what makes the rule above enforceable: there is
+no global a test or a command could set by accident, and a command that forgets to
+pass one passes `""`, which means every channel. Only `load` skips a channel that
+does not support an effect, warning on stderr, because there a whole profile is
+being applied. `effect` refuses instead, `all` included: `effect all` asks for
+every channel, so a name only one of them has is a request the tool cannot carry
+out rather than a reason to write the others and report success.
 
 ## Stack
 
@@ -112,7 +138,7 @@ behavior counts too. Before any edit:
 1. **Binary name** — the `go build -o` target is `qmk-rgb-tool`, which is not the module's last path element. It must match every example in README, AGENTS.md, comments, and docs.
 2. **CLI command** — the root command name (e.g. `qmk-rgb-tool`) is the binary name; every usage example, CLI reference table, and doc must use the same string.
 3. **Module path** — the `go.mod` module path is the source of truth for `go install` targets.
-4. **Zone names** — the vocabulary is QMK's subsystem names, so they must be identical in code, docs, and examples: `backlight`, `rgblight`, `rgb_matrix`, `audio`, `led_matrix`. A board's own names (`logo`, `Backlight`, `side` on the Impact 80) are **data** in its definition file, not code and not part of the vocabulary: do not hardcode one into Go, and do not put one in a `--zone` help string. A board's *model* name is the definition's `name`, or the USB product string, or nothing — there is no lookup file left to put one in.
+4. **Zone names** — the vocabulary is QMK's subsystem names, so they must be identical in code, docs, and examples: `backlight`, `rgblight`, `rgb_matrix`, `audio`, `led_matrix`. A board's own names (`logo`, `Backlight`, `side` on the Impact 80) are **data** in its definition file, not code and not part of the vocabulary: do not hardcode one into Go, and do not put one in a zone's help text. A board's *model* name is the definition's `name`, or the USB product string, or nothing — there is no lookup file left to put one in.
 
 Documented behavior must also match the code:
 
@@ -150,7 +176,7 @@ so `effect none` there does nothing to the effect and is reported as the effect
 still running. `disable` is the command that turns a channel off, because it
 also writes brightness 0. Above the top the behaviour is the opposite: the
 backlight channel does not refuse ID 47 or ID 99 but clamps it to the 46 it
-holds, so an `effect <index>` there is reported as the index the keyboard ended
+holds, so an effect index there is reported as the index the keyboard ended
   up with. Read
 the register back; never report the number that was asked for.
 
@@ -236,7 +262,7 @@ measured on VIA's own collection of 2029 definitions rather than assumed:
 - A definition names its channels too, in the sub-menu it puts each one under,
   and those names are what VIA shows, so they are the display name. The QMK
   subsystem name stays accepted as an alternative and matching ignores case, so
-  `--zone backlight` keeps working on a board whose definition writes
+  `brightness backlight 100` keeps working on a board whose definition writes
   `Backlight`. The QMK subsystem name always works, which is why the subsystem is
   what a document should tell a user to type.
 - A fetched file has to be parsed and matched before it is believed. VIA answers
@@ -289,7 +315,7 @@ transcribed per board and cannot be generated from a common source.
 
 ## Output Shape
 
-`keyboard info`, `info`, `list`, `effect --list` and `keyboard definitions` print text,
+`keyboard info`, `info`, `list`, the effect list and `keyboard definitions` print text,
 because that is what a person reads, and JSON only behind the persistent `--json`.
 The field names are the ones the JSON always had, so a consumer that passes the
 flag is unaffected — where a field had to be added, it is added rather than
@@ -299,12 +325,12 @@ ones in the user directory, and a path alone does not say which of the two a lin
 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. Two shapes are deliberately
-not in that list: `effect` with no argument routes to `effect --list`, and
+not in that list: `effect` with no effect name routes to the effect list, and
 `keyboard fetch` writes a file and prints a line about it.
 
 `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
-effect names for that board, and the channels come from `info` and `effect --list`,
+effect names for that board, and the channels come from `info` and the effect list,
 which open it.
 
 ## Code vs Documentation

+ 127 - 80
README.md

@@ -74,7 +74,7 @@ the binary, because that board is not in VIA's collection and so cannot be fetch
 at all; that directory is read only at build time.
 
 Without a definition the keyboard is still fully driven with `brightness`, `speed`,
-`color`, `effect <index>` and `info` — only the effect names are missing, and
+`color`, an effect index and `info` — only the effect names are missing, and
 [Effect Names Are Per Board](#effect-names-are-per-board) says what that costs and
 what the file does and does not tell the tool.
 
@@ -126,7 +126,7 @@ behaviour comes from whichever binary is on `$PATH`. What the shell offers is
 registered per flag and per argument, which is why these work:
 
 ```bash
-$ qmk-rgb-tool effect --zone <TAB>     # the QMK subsystem names and the board's own
+$ qmk-rgb-tool effect <TAB>             # the QMK subsystem names and the board's own
 $ qmk-rgb-tool effect --device <TAB>   # the numbers `keyboard info` prints
 $ qmk-rgb-tool load <TAB>              # the profiles in the per-user profiles/
 ```
@@ -142,47 +142,85 @@ into the prompt, which is worse than an empty list.
 # Discover connected keyboards
 ./qmk-rgb-tool keyboard info
 
-# RGB commands target every channel the keyboard reports, in channel order
-./qmk-rgb-tool effect breathing
-./qmk-rgb-tool effect rainbow_moving_chevron
-./qmk-rgb-tool effect rainbow_moving_chevron --zone backlight
-./qmk-rgb-tool brightness 160
-./qmk-rgb-tool speed 2
-./qmk-rgb-tool color 00ff00
-./qmk-rgb-tool color hsv:85,255,255
-# A raw effect index is zone-specific; ID 17 is Backlight-only
-./qmk-rgb-tool effect 17 --zone backlight
-./qmk-rgb-tool enable
-./qmk-rgb-tool disable
+# A lighting command names its target as its first argument
+./qmk-rgb-tool effect rgb_matrix breathing
+./qmk-rgb-tool effect backlight rainbow_moving_chevron
+./qmk-rgb-tool brightness side 160
+./qmk-rgb-tool speed all 2
+./qmk-rgb-tool color all 00ff00
+./qmk-rgb-tool color backlight hsv:85,255,255
+# Several channels at once, comma separated; a raw effect index is
+# zone-specific, and ID 17 is Backlight-only
+./qmk-rgb-tool brightness logo,side 160
+./qmk-rgb-tool effect backlight 17
+./qmk-rgb-tool enable all
+./qmk-rgb-tool disable logo
 ./qmk-rgb-tool info
 
-# Select exactly one zone
-./qmk-rgb-tool brightness 160 --zone side
+# Reading needs no target: without a name, an effect or an info reads everything
+./qmk-rgb-tool effect all
+./qmk-rgb-tool effect logo
+./qmk-rgb-tool info
 
 # Save and load RGB profiles
 ./qmk-rgb-tool save paul
 ./qmk-rgb-tool load paul
+./qmk-rgb-tool load paul side
 ./qmk-rgb-tool list
 ./qmk-rgb-tool delete paul
 
 # Select a keyboard by the number shown in `keyboard info`
-./qmk-rgb-tool --device 1 enable
+./qmk-rgb-tool --device 1 enable rgb_matrix
 ```
 
-`--zone` accepts a VIA lighting channel: `backlight`, `rgblight`, `rgb_matrix`,
-`audio` or `led_matrix`, and may be given on any subcommand. That name follows
+The zone is a VIA lighting channel: `backlight`, `rgblight`, `rgb_matrix`, `audio`
+or `led_matrix`, and every command that names a channel takes it. That name follows
 from the channel number, so it works on every QMK keyboard. A board's definition
 file may name a channel differently, and that name is accepted too, ignoring
 case — the Impact 80 calls channels 2, 3 and 4 `logo`, `Backlight` and `side`.
 Where a name would be the subsystem name of a different channel the keyboard has,
 the board is refused rather than a command being sent to the wrong channel.
 
-`--zone` is not remembered between runs: repeat it on every invocation. Without
-`--zone`, commands target every channel the keyboard reports, in channel order.
-With `--zone`, commands target exactly the named channel. An unknown name fails
-before the device is opened; an effect a named channel does not have fails
-rather than being skipped, while a default command skips it and prints a
-warning on stderr.
+Several channels are written comma separated, and `all` is every channel the
+keyboard reports: `brightness logo,side 160` reaches exactly those two, and
+`all` next to another name is the same request as `all` alone, since the union of
+the two is the whole keyboard. A name written twice is one channel, spaces around
+the commas are ignored, and the channels are always written in channel order, so a
+script sees one order for every spelling of the same selection.
+
+The zone is an argument rather than a flag because a flag every help listed told
+`keyboard info` and `list` about a channel they cannot address, and they ignored
+it. A command that writes lighting — `effect`, `brightness`, `speed`, `color`,
+`enable`, `disable` — cannot be called without one, and the argument count refuses
+it before the keyboard is opened:
+
+```console
+$ qmk-rgb-tool brightness 160
+Error: brightness takes a zone and a brightness value
+usage: qmk-rgb-tool brightness <zone> <val>
+```
+
+A zone the keyboard does not have is refused by name, and where a list names
+several it names every one that is missing, so nothing is written on the way to a
+failure:
+
+```console
+$ qmk-rgb-tool brightness logo,led_matrix 160
+Error: this keyboard has no led_matrix channel
+```
+
+A command that only reads needs no zone: `info`, `effect` without a name and
+`save` report every channel the keyboard has, which is what they are for. `load`
+applies the zones its profile names, and a second argument narrows that to the
+channel or channels it names.
+
+An unknown name fails before the device is opened. An effect a named channel does
+not have fails rather than being skipped, `all` included — `effect all` asks for
+every channel, so a name only one of them has is a request the tool cannot carry
+out, not a reason to write the others and report it as done. `load` is the one
+place a name is skipped instead, with a warning on stderr, because there a whole
+profile is being applied and a key the board cannot do is worth saying rather
+than refusing over.
 
 The keyboard is asked which channels it has: one read per channel, and a channel
 its firmware does not implement answers as unhandled. The probe covers channels 1
@@ -203,14 +241,14 @@ All output is machine-parseable JSON when applicable. The shapes an agent parses
             "effectId": 2, "brightness": 10, "speed": 0,
             "color": {"hue": 0, "saturation": 255}, "error": "only on a channel that failed"}]}
 
-// effect --list
+// effect all
 {"catalog": "Impact 80",
  "zones": [{"zone": "logo", "channel": 2, "subsystem": "rgblight",
             "effect": "none", "id": 0}]}
 ```
 
 `catalog` carries the name from the definition file, so it is the vendor's own
-rather than a label this tool invented. `effect --list` reports `"catalog": ""` and
+rather than a label this tool invented. `effect all` reports `"catalog": ""` and
 an empty `zones` array for a board that has no definition, and it reads the
 keyboard to learn which channels to list.
 `info` prints the JSON and then exits non-zero if a channel could not be read.
@@ -240,9 +278,9 @@ rather than storing the number.
 `color` accepts three notations for one operation:
 
 ```bash
-qmk-rgb-tool color ff0000            # six digit hex
-qmk-rgb-tool color rgb:ff0000        # the same, written out
-qmk-rgb-tool color hsv:0,255,255     # hue, saturation, value, each 0-255
+qmk-rgb-tool color rgb_matrix ff0000    # six digit hex
+qmk-rgb-tool color all rgb:ff0000            # the same, written out
+qmk-rgb-tool color side hsv:0,255,255   # hue, saturation, value, each 0-255
 ```
 
 `hsv:` takes the numbers the keyboard itself computes in, so a color can be
@@ -259,9 +297,9 @@ Verbatim" for the table.
 Every notation is read back, so the line reports what the keyboard holds:
 
 ```console
-$ qmk-rgb-tool color 00ff00 --zone backlight
+$ qmk-rgb-tool color backlight 00ff00
 Color set to hue 85 sat 255
-$ qmk-rgb-tool color hsv:0,255,200
+$ qmk-rgb-tool color all hsv:0,255,200
 Color logo hue 0 sat 255 brightness 160 backlight hue 0 sat 255 brightness 255 (requested hue 0 sat 255 brightness 200)
 ```
 
@@ -274,7 +312,7 @@ a hex triple.
 
 ## Effect Names Are Per Board
 
-The keyboard holds effect numbers, not names, so `effect <name>` needs a catalog,
+The keyboard holds effect numbers, not names, so `effect <zone> <name>` needs a catalog,
 and the catalog is the board's VIA definition file. **No catalog is hand-written
 into this tool for any board.** A transcribed name list and the vendor's file
 describing the same board are two places to update one name, and that is how the
@@ -297,11 +335,11 @@ channels of the same board. The tool's own spellings resolve alongside them
 `effect` writes the ID and reads the register back, because a board can refuse
 one. All 47 backlight IDs are taken, and so are the `logo` and `side` IDs from 1
 to 6. ID 0 is not: on those two channels the firmware reads it as "lighting
-off" and leaves the mode register where it was, so `effect none --zone logo`
+off" and leaves the mode register where it was, so `effect logo none`
 leaves the previous effect running and says so.
 
 ```
-$ qmk-rgb-tool effect none --zone logo
+$ qmk-rgb-tool effect logo none
 Effect logo "wave" (1) (requested "none" (0))
 ```
 
@@ -315,7 +353,7 @@ above 46 down to it. The definition file names 46 of them, ending at `riverflow`
 so **ID 46 has no name and the tool does not invent one**:
 
 ```
-$ qmk-rgb-tool effect 46 --zone backlight
+$ qmk-rgb-tool effect backlight 46
 Effect set to index 46
 $ qmk-rgb-tool info
 unknown
@@ -336,17 +374,17 @@ Wobkey's own
 ID from its dropdown either. A raw `effect 46` is the only way to reach it.
 
 A keyboard without a catalog is still driven: `brightness`, `speed`, `color`,
-`effect <index>` and `info` all work, because none of them needs a name. Four
+an effect index and `info` all work, because none of them needs a name. Four
 commands need the catalog and say so rather than guessing:
 
 ```
-$ qmk-rgb-tool effect wave
+$ qmk-rgb-tool effect rgb_matrix wave
 Error: no effect names for this keyboard: run `keyboard fetch` for its VIA definition, or set an
-effect by number with `effect <index>`
+effect by number with `effect <zone> <index>`
 
-$ qmk-rgb-tool enable
+$ qmk-rgb-tool enable rgb_matrix
 Error: this keyboard has no effect names, so `enable` cannot choose an effect: run `keyboard fetch` for
-its VIA definition, or set one with `effect <index>`
+its VIA definition, or set one with `effect <zone> <index>`
 
 $ qmk-rgb-tool load paul
 Warning: profile "paul" has no effect names for this keyboard, so nothing applied; run `keyboard fetch` for
@@ -354,7 +392,7 @@ its VIA definition
 
 $ qmk-rgb-tool save paul
 Warning: this keyboard has no effect names, so the profile records effect "unknown" and cannot restore it; run
-`keyboard fetch` for its VIA definition, or set an effect with `effect <index>`
+`keyboard fetch` for its VIA definition, or set an effect with `effect <zone> <index>`
 ```
 
 `enable` needs it because it has to choose an effect to turn a channel on.
@@ -367,9 +405,9 @@ Naming a real effect on a channel that does not have it is refused, and a name
 the board does not have anywhere is reported as unknown:
 
 ```
-$ qmk-rgb-tool effect rainbow_moving_chevron --zone logo
+$ qmk-rgb-tool effect logo rainbow_moving_chevron
 Error: effect rainbow_moving_chevron is not supported on rgblight
-$ qmk-rgb-tool effect nonsense --zone rgb_matrix
+$ qmk-rgb-tool effect rgb_matrix nonsense
 Error: unknown effect: nonsense
 ```
 
@@ -444,13 +482,13 @@ range is 0–255 but the value the keyboard ends up holding is often different.
 applied:
 
 ```console
-$ qmk-rgb-tool brightness 200
+$ qmk-rgb-tool brightness all 200
 Brightness logo 160 backlight 255 side 160 (requested 200)
-$ qmk-rgb-tool brightness 160 --zone logo
+$ qmk-rgb-tool brightness logo 160
 Brightness set to 160
-$ qmk-rgb-tool speed 60
+$ qmk-rgb-tool speed all 60
 Speed logo 4 backlight 60 side 4 (requested 60)
-$ qmk-rgb-tool speed 60 --zone backlight
+$ qmk-rgb-tool speed backlight 60
 Speed set to 60
 ```
 
@@ -486,7 +524,7 @@ failure, but the summary line always states the value that was actually applied.
 - **Effect names from VIA definition files** — the vendor's own names, read at runtime; no name is hand-written, so there is one source per board, and a file the user places overrides the one built in
 - **Compatibility aliases** — `off`, `breathe`, `rainbow`, `rainbow_wave`, `solid`, `static` resolve to correct effect IDs per channel
 - **Reactive & splash effects** — honor `color` and `speed` for key-press illumination
-- **Machine-parseable output** — text by default, JSON behind `--json` for `keyboard info`, `info`, `list`, `effect --list` and `keyboard definitions`
+- **Machine-parseable output** — text by default, JSON behind `--json` for `keyboard info`, `info`, `list`, the effect list and `keyboard definitions`
 - **Agent-friendly** — designed for automation, scripting, and CLI-first workflows
 - **Multiple devices** — `keyboard info` numbers each keyboard; `--device <n>` targets one
 
@@ -503,7 +541,7 @@ it puts each channel under. The QMK subsystem names for the same channels are
 `rgblight`, `rgb_matrix` and `audio`, and both spellings work on this board.
 
 Logo and Side share this complete effect family. Two of these are the tool's own
-spellings: the file writes `fixed wave` and `breathe`, and `effect --list` prints
+spellings: the file writes `fixed wave` and `breathe`, and the effect list prints
 the file's spelling.
 
 | ID | Name | In the file |
@@ -550,7 +588,7 @@ one column carries the whole list:
 
 The names below are the tool's own spellings, which every definition resolves
 alongside the vendor's; the output shows whichever the file in use writes, so
-`effect --list` may print `fixed wave` and `breathe` where this list says
+The effect list may print `fixed wave` and `breathe` where this list says
 `fixed_wave` and `breathing`. Same effect, same ID.
 
 Effects fall into categories that behave differently:
@@ -590,13 +628,13 @@ character as far as lookup is concerned. So on this board all four of these sele
 the same effect:
 
 ```
-qmk-rgb-tool effect breathe   --zone logo     # the file's spelling
-qmk-rgb-tool effect breathing --zone logo     # the tool's spelling
-qmk-rgb-tool effect fixed_wave --zone logo     # the tool's spelling
-qmk-rgb-tool effect "fixed wave" --zone logo   # the file's spelling
+qmk-rgb-tool effect logo breathe          # the file's spelling
+qmk-rgb-tool effect logo breathing        # the tool's spelling
+qmk-rgb-tool effect logo fixed_wave        # the tool's spelling
+qmk-rgb-tool effect logo "fixed wave"     # the file's spelling
 ```
 
-`info` and `effect --list` report the spelling the file uses, so the output
+`info` and the effect list report the spelling the file uses, so the output
 carries `fixed wave` and `breathe` where this document's own prose uses the tool's
 `fixed_wave` and `breathing`. The ID beside the name is the same in both. The file
 is also inconsistent with itself across channels — `breathing` on `backlight`,
@@ -696,8 +734,8 @@ channel at all is listed and then fails every command that opens it with
 A definition also names its channels, and that is where the name comes from —
 there is no other place. They are the names VIA shows, so the tool and VIA call
 a channel the same thing, and a name is matched ignoring case. On a board whose
-definition writes `Backlight`, all of `--zone backlight`, `--zone Backlight` and
-`--zone rgb_matrix` reach the same channel: the label is the name, the subsystem
+definition writes `Backlight`, all of `brightness backlight`, `brightness Backlight` and
+`brightness rgb_matrix` reach the same channel: the label is the name, the subsystem
 stays accepted because it follows from the channel number. `qmk-rgb-tool
 keyboard definitions` prints the label beside the subsystem it stands for, and says
 where each definition came from — `user` for a file in the per-user directory,
@@ -752,36 +790,44 @@ board", and that file is gone, so the field would have been unanswerable:
 | Command                         | Description                              |
 |---------------------------------|------------------------------------------|
 | `qmk-rgb-tool keyboard info`          | Discover connected QMK keyboards          |
-| `qmk-rgb-tool enable`                 | Enable selected lighting zones            |
-| `qmk-rgb-tool disable`                | Disable selected lighting zones           |
-| `qmk-rgb-tool info`                   | Show per-zone RGB state                   |
-| `qmk-rgb-tool effect <name\|index>`   | Set an effect by name, or a raw effect index 0–255, verified by read-back; a board that does not implement the index clamps it to the highest it does |
-| `qmk-rgb-tool effect`                 | With no argument, list every effect per channel |
-| `qmk-rgb-tool effect --list`          | The same list, as a flag                   |
+| `qmk-rgb-tool enable <zone>`           | Enable the named lighting zones            |
+| `qmk-rgb-tool disable <zone>`          | Disable the named lighting zones           |
+| `qmk-rgb-tool info [zone]`             | Show per-zone RGB state; without a zone, every channel |
+| `qmk-rgb-tool effect <zone> <name\|index>` | Set an effect by name, or a raw effect index 0–255, verified by read-back; a board that does not implement the index clamps it to the highest it does |
+| `qmk-rgb-tool effect <zone>`           | With no name, list the effects that zone has; `effect all` lists every channel |
 | `qmk-rgb-tool keyboard fetch`         | Download the VIA definition for the connected keyboard |
 | `qmk-rgb-tool keyboard definitions`   | List every definition in use, from the per-user directory and built into the binary, each marked with which it is |
-| `qmk-rgb-tool brightness <val>`       | Set brightness (0–255) on selected zones, verified by read-back |
-| `qmk-rgb-tool speed <val>`            | Set effect speed (0–255) on selected zones, verified by read-back; on `logo` and `side` only 0, 1 and 4 are reachable |
-| `qmk-rgb-tool color <hex>`            | Set color (e.g. `ff0000`) on selected zones |
-| `qmk-rgb-tool color rgb:<hex>`        | The same hex color, written out |
-| `qmk-rgb-tool color hsv:<h>,<s>,<v>`  | Set hue and saturation (0–255) and write `v` to the brightness of the same zones |
-| `qmk-rgb-tool --zone <channel> ...`   | Target one channel: `backlight`, `rgblight`, `rgb_matrix`, `audio` or `led_matrix`, or the name the board's definition gives it |
+| `qmk-rgb-tool brightness <zone> <val>` | Set brightness (0–255) on the named zones, verified by read-back |
+| `qmk-rgb-tool speed <zone> <val>`     | Set effect speed (0–255) on the named zones, verified by read-back; on `logo` and `side` only 0, 1 and 4 are reachable |
+| `qmk-rgb-tool color <zone> <hex>`     | Set color (e.g. `ff0000`) on the named zones |
+| `qmk-rgb-tool color <zone> rgb:<hex>` | The same hex color, written out |
+| `qmk-rgb-tool color <zone> hsv:<h>,<s>,<v>` | Set hue and saturation (0–255) and write `v` to the brightness of the same zones |
+| `qmk-rgb-tool <command> <zone> ...` | Name the channels: `backlight`, `rgblight`, `rgb_matrix`, `audio` or `led_matrix`, the name the board's definition gives it, several comma separated, or `all` for every channel the keyboard reports. A command that writes lighting cannot be called without one |
 | `qmk-rgb-tool --device <n> ...`      | Target keyboard by number (see `keyboard info`) |
 | `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`, `effect --list` and `keyboard definitions` |
+| `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]`            | Save current RGB state 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` |
-| `qmk-rgb-tool load [name]`            | Load and apply a profile by name from the per-user `profiles/`; without a name it loads `default` |
+| `qmk-rgb-tool save [name]`            | 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` |
+| `qmk-rgb-tool load <name> [zone]`     | Load and apply a profile by name from the per-user `profiles/`; 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` |
 | `qmk-rgb-tool completion <shell>`     | Write an autocompletion script for `bash`, `zsh`, `fish` or `powershell` to stdout |
 
-`enable`, `disable`, `info` and `list` take no arguments and reject a stray
-token. `effect`, `load`, `save` and `delete` accept an optional name.
-`brightness`, `speed` and `color` require exactly one argument. `effect` takes one
-argument too, and accepts either a name or a raw effect ID.
-
-`keyboard info`, `info`, `list`, `keyboard definitions` and `effect --list` print
+`enable` and `disable` take a zone and nothing else. `brightness`, `speed` and
+`color` take a zone and a value. `effect` takes a zone and, optionally, an effect
+name or a raw effect ID: two arguments set it, one lists that zone's effects.
+`info` takes at most a zone, `load` a profile name and at most a zone, `save` and
+`delete` an optional name, and `list` nothing. `list` and the `keyboard`
+subcommands reject a stray token, which is how a mistyped invocation is caught
+before it looks like a command that did its work.
+
+A zone is an argument and not a flag, so a command that cannot address a channel —
+`keyboard info`, `list`, `delete` — has no way to be handed one, and its help says
+nothing about channels. That is the whole reason it is spelled this way: a flag on
+the root is a flag every help lists, and it was ignored rather than refused
+wherever it did not apply.
+
+`keyboard info`, `info`, `list`, `keyboard definitions` and the effect list print
 text, because a person reads them. Pass `--json` for the machine shape, which is
 the same data with the same field names as before:
 
@@ -812,7 +858,8 @@ there are reported per key and skipped — but it says which profile belongs to
 which keyboard. A profile written before this field existed has no board and is
 loaded without complaint. The keys are the names the file was written with, so
 a profile written before a board was renamed reports the key it cannot place and
-skips it. They are the only way to persist RGB
+skips it. `load` applies the zones the profile names, and a second argument
+narrows that to the channels it names. They are the only way to persist RGB
 settings across reboots. The keyboard's VIA protocol does not expose a
 standard command to save RGB state to internal EEPROM, so profiles rely on
 files instead.

+ 2 - 2
cmd/qmk-rgb-tool/agents_doc_test.go

@@ -29,13 +29,13 @@ func TestAgentsDocDoesNotDuplicateTheEffectCatalog(t *testing.T) {
 		"multisplash",
 	} {
 		if strings.Contains(body, name) {
-			t.Errorf("AGENTS.md lists the effect %q; the catalog comes live from `qmk-rgb-tool effect --list`", name)
+			t.Errorf("AGENTS.md lists the effect %q; the catalog comes live from `qmk-rgb-tool effect all`", name)
 		}
 	}
 
 	for _, rangeStmt := range []string{"ID 0–45 family", "ID 0-45 family", "IDs 0–45", "IDs 0-45"} {
 		if strings.Contains(body, rangeStmt) {
-			t.Errorf("AGENTS.md states %q; the per-zone catalog comes from `qmk-rgb-tool effect --list`", rangeStmt)
+			t.Errorf("AGENTS.md states %q; the per-zone catalog comes from `qmk-rgb-tool effect all`", rangeStmt)
 		}
 	}
 }

+ 86 - 18
cmd/qmk-rgb-tool/arity_test.go

@@ -7,16 +7,13 @@ import (
 	"github.com/spf13/cobra"
 )
 
-// `info`, `enable`, `disable` and `list` take nothing. Without a declared
-// arity cobra applies arbitraryArgs, so a stray token yielded exit 0 and the
-// command looked like it had done its work — a caller could believe a
-// lighting change happened when the argument was silently dropped.
+// `list` takes nothing. Without a declared arity cobra applies arbitraryArgs, so
+// a stray token yielded exit 0 and the command looked like it had done its work —
+// a caller could believe a lighting change happened when the argument was silently
+// dropped.
 func TestZeroArgCommandsRejectStrayTokens(t *testing.T) {
 	cmds := map[string]*cobra.Command{
-		"info":    NewInfoCmd(),
-		"enable":  NewEnableCmd(),
-		"disable": NewDisableCmd(),
-		"list":    NewProfileListCmd(),
+		"list": NewProfileListCmd(),
 	}
 
 	for name, cmd := range cmds {
@@ -36,10 +33,7 @@ func TestZeroArgCommandsRejectStrayTokens(t *testing.T) {
 // Rejecting extras must not break the no-argument case.
 func TestZeroArgCommandsAcceptNoArgs(t *testing.T) {
 	cmds := map[string]*cobra.Command{
-		"info":    NewInfoCmd(),
-		"enable":  NewEnableCmd(),
-		"disable": NewDisableCmd(),
-		"list":    NewProfileListCmd(),
+		"list": NewProfileListCmd(),
 	}
 
 	for name, cmd := range cmds {
@@ -51,8 +45,10 @@ func TestZeroArgCommandsAcceptNoArgs(t *testing.T) {
 	}
 }
 
-// Commands that do take an argument must keep accepting exactly one.
-func TestOneArgCommandsKeepTheirArity(t *testing.T) {
+// A command whose arguments are a zone and a value takes exactly those two, and
+// refuses a third: an extra token means the invocation is not the one the user
+// meant, and carrying on with the first two is a guess.
+func TestTwoArgCommandsKeepTheirArity(t *testing.T) {
 	cmds := map[string]*cobra.Command{
 		"brightness": NewBrightnessCmd(),
 		"speed":      NewSpeedCmd(),
@@ -61,12 +57,84 @@ func TestOneArgCommandsKeepTheirArity(t *testing.T) {
 
 	for name, cmd := range cmds {
 		t.Run(name, func(t *testing.T) {
-			if err := cmd.ValidateArgs([]string{"160"}); err != nil {
-				t.Errorf("%s rejected its argument: %v", name, err)
+			if err := cmd.ValidateArgs([]string{"logo", "160"}); err != nil {
+				t.Errorf("%s rejected its arguments: %v", name, err)
+			}
+			if err := cmd.ValidateArgs([]string{"160"}); err == nil {
+				t.Errorf("%s accepted one argument, want a zone and a value", name)
 			}
-			if err := cmd.ValidateArgs([]string{"1", "2"}); err == nil {
-				t.Errorf("%s accepted two arguments, want exactly one", name)
+			if err := cmd.ValidateArgs([]string{"logo", "160", "extra"}); err == nil {
+				t.Errorf("%s accepted three arguments, want exactly two", name)
 			}
 		})
 	}
 }
+
+// A zone is an argument and not a flag, so the count of a command that writes
+// lighting is the thing that keeps it from writing every channel by accident.
+func TestZoneCommandsRejectAMissingZone(t *testing.T) {
+	cmds := map[string]*cobra.Command{
+		"enable":  NewEnableCmd(),
+		"disable": NewDisableCmd(),
+	}
+
+	for name, cmd := range cmds {
+		t.Run(name, func(t *testing.T) {
+			if err := cmd.ValidateArgs([]string{"logo"}); err != nil {
+				t.Errorf("%s rejected a zone: %v", name, err)
+			}
+			err := cmd.ValidateArgs(nil)
+			if err == nil {
+				t.Fatalf("%s accepted no arguments, want the zone to be required", name)
+			}
+			if !strings.Contains(err.Error(), "zone") {
+				t.Errorf("%s error = %q, want it to name the missing zone", name, err)
+			}
+		})
+	}
+}
+
+// `effect` reads with one argument and writes with two, so the count is what tells
+// the two apart, and either count alone has to be accepted.
+func TestEffectTakesAZoneAndOptionallyAName(t *testing.T) {
+	cmd := NewEffectCmd()
+
+	if err := cmd.ValidateArgs([]string{"logo"}); err != nil {
+		t.Errorf("effect rejected a zone on its own: %v", err)
+	}
+	if err := cmd.ValidateArgs([]string{"logo", "wave"}); err != nil {
+		t.Errorf("effect rejected a zone and a name: %v", err)
+	}
+	if err := cmd.ValidateArgs(nil); err == nil {
+		t.Error("effect accepted no arguments, want a zone to be required")
+	}
+	if err := cmd.ValidateArgs([]string{"logo", "wave", "extra"}); err == nil {
+		t.Error("effect accepted three arguments, want at most two")
+	}
+}
+
+// `info` reads every channel when it is given nothing and one zone when it is
+// given one. Both are requests, so neither is a stray token.
+func TestInfoTakesAtMostAZone(t *testing.T) {
+	cmd := NewInfoCmd()
+
+	if err := cmd.ValidateArgs(nil); err != nil {
+		t.Errorf("info rejected no arguments: %v", err)
+	}
+	if err := cmd.ValidateArgs([]string{"logo"}); err != nil {
+		t.Errorf("info rejected a zone: %v", err)
+	}
+	if err := cmd.ValidateArgs([]string{"logo", "side"}); err == nil {
+		t.Error("info accepted two arguments, want one zone or none")
+	}
+}
+
+// A list of zones is one argument, so `info logo,side` is one zone naming two
+// channels and not two arguments.
+func TestInfoTakesAZoneListAsOneArgument(t *testing.T) {
+	cmd := NewInfoCmd()
+
+	if err := cmd.ValidateArgs([]string{"logo,side"}); err != nil {
+		t.Errorf("info rejected a comma separated zone list: %v", err)
+	}
+}

+ 8 - 9
cmd/qmk-rgb-tool/brightness.go

@@ -7,19 +7,18 @@ import (
 )
 
 func NewBrightnessCmd() *cobra.Command {
-	return &cobra.Command{
-		Use:   "brightness <val>",
-		Short: "Set RGB brightness",
-		Long: "Set the RGB brightness value (0-255) and read it back, so a value the\n" +
-			"keyboard clamps or rescales is reported instead of silently applied.",
-		Args: cobra.ExactArgs(1),
+	return withZoneArgs(&cobra.Command{
+		Use:   "brightness <zone> <val>",
+		Short: "Set RGB brightness on a zone",
+		Long: writesLighting("Set the RGB brightness value (0-255) and read it back, so a value the\n" +
+			"keyboard clamps or rescales is reported instead of silently applied."),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			val, err := ParseUint8(args[0])
+			val, err := ParseUint8(args[1])
 			if err != nil {
 				return err
 			}
 
-			proto, target, channels, err := openTarget()
+			proto, target, channels, err := openTarget(args[0])
 			if err != nil {
 				return err
 			}
@@ -40,5 +39,5 @@ func NewBrightnessCmd() *cobra.Command {
 			fmt.Fprintf(cmd.OutOrStdout(), "Brightness set to %d\n", val)
 			return nil
 		},
-	}
+	}, 2, 2, "a zone and a brightness value")
 }

+ 5 - 11
cmd/qmk-rgb-tool/brightness_summary_test.go

@@ -8,20 +8,17 @@ import (
 	"netdome.biz/paul/qmk-rgb/internal/via"
 )
 
-func runBrightness(t *testing.T, applied map[via.Channel]uint8, zoneFlag string, arg string) (stdout, stderr string) {
+func runBrightness(t *testing.T, applied map[via.Channel]uint8, zone string, arg string) (stdout, stderr string) {
 	t.Helper()
 
 	proto := &verifyingProtocol{applied: applied}
-	originalZone := targetZone
-	t.Cleanup(func() { targetZone = originalZone })
-	targetZone = zoneFlag
 	t.Cleanup(impact80Target(t, proto))
 
 	var out, errOut bytes.Buffer
 	cmd := NewBrightnessCmd()
 	cmd.SetOut(&out)
 	cmd.SetErr(&errOut)
-	cmd.SetArgs([]string{arg})
+	cmd.SetArgs([]string{zone, arg})
 
 	if err := cmd.Execute(); err != nil {
 		t.Fatalf("brightness returned error: %v", err)
@@ -49,7 +46,7 @@ func TestBrightnessSummarisesAppliedValuesOnMismatch(t *testing.T) {
 		via.ChannelRgblight:  160,
 		via.ChannelRgbMatrix: 255,
 		via.ChannelAudio:     160,
-	}, "", "200")
+	}, "all", "200")
 
 	for _, want := range []string{"logo 160", "backlight 255", "side 160", "requested 200"} {
 		if !strings.Contains(stdout, want) {
@@ -79,16 +76,13 @@ func TestBrightnessSummarisesASingleMismatchingZone(t *testing.T) {
 // The summary must not invent zones the user did not select.
 func TestBrightnessSummaryNamesOnlySelectedZones(t *testing.T) {
 	proto := &verifyingProtocol{applied: map[via.Channel]uint8{via.ChannelAudio: 160}}
-	originalZone := targetZone
-	t.Cleanup(func() { targetZone = originalZone })
-	targetZone = "side"
-	t.Cleanup(stubOpenTarget(t, proto, impact80Display(), impact80Channels(), 0x36B0, 0x309F))
+	t.Cleanup(impact80Target(t, proto))
 
 	var out bytes.Buffer
 	cmd := NewBrightnessCmd()
 	cmd.SetOut(&out)
 	cmd.SetErr(&out)
-	cmd.SetArgs([]string{"200"})
+	cmd.SetArgs([]string{"side", "200"})
 
 	if err := cmd.Execute(); err != nil {
 		t.Fatalf("brightness returned error: %v", err)

+ 1 - 1
cmd/qmk-rgb-tool/brightness_verify_test.go

@@ -171,7 +171,7 @@ func TestBrightnessCommandStaysQuietWhenApplied(t *testing.T) {
 	cmd := NewBrightnessCmd()
 	cmd.SetOut(&out)
 	cmd.SetErr(&errOut)
-	cmd.SetArgs([]string{"200"})
+	cmd.SetArgs([]string{"logo", "200"})
 
 	if err := cmd.Execute(); err != nil {
 		t.Fatalf("brightness returned error: %v", err)

+ 131 - 18
cmd/qmk-rgb-tool/channels_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"bytes"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -61,6 +62,121 @@ func TestResolveZoneNameReportsAnAmbiguousName(t *testing.T) {
 	}
 }
 
+// A zone is one name or several written comma separated, and the channels come
+// back in channel order however they were written. A script that parses the
+// result sees one order for every spelling of one selection, so the order the
+// user typed in is not a thing to depend on.
+func TestResolveZoneNameReturnsAChannelOrderWhateverTheSpelling(t *testing.T) {
+	display := map[uint16]string{2: "logo", 3: "backlight", 4: "side"}
+	cases := []struct {
+		zone string
+		want []via.Channel
+	}{
+		{"logo", []via.Channel{via.ChannelRgblight}},
+		{"logo,side", []via.Channel{via.ChannelRgblight, via.ChannelAudio}},
+		{"side,logo", []via.Channel{via.ChannelRgblight, via.ChannelAudio}},
+		{" logo , side ", []via.Channel{via.ChannelRgblight, via.ChannelAudio}},
+		{"side,logo,backlight", []via.Channel{via.ChannelRgblight, via.ChannelRgbMatrix, via.ChannelAudio}},
+		// A name twice, and a name that reaches one channel by two of its names,
+		// are one channel each: a double write is not what was asked for.
+		{"logo,logo", []via.Channel{via.ChannelRgblight}},
+		{"logo,rgblight", []via.Channel{via.ChannelRgblight}},
+		// The QMK subsystem name and the definition's label for the same channel.
+		{"backlight", []via.Channel{via.ChannelRgbMatrix}},
+		{"Backlight", []via.Channel{via.ChannelRgbMatrix}},
+		// A board that renames a channel keeps the subsystem name working.
+		{"rgblight", []via.Channel{via.ChannelRgblight}},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.zone, func(t *testing.T) {
+			got, err := resolveZoneName(tc.zone, display, map[uint16][]string{3: {"rgb_matrix"}})
+			if err != nil {
+				t.Fatalf("resolveZoneName(%q) error = %v", tc.zone, err)
+			}
+			if !reflect.DeepEqual(got, tc.want) {
+				t.Errorf("resolveZoneName(%q) = %v, want %v", tc.zone, got, tc.want)
+			}
+		})
+	}
+}
+
+// all is every channel, and naming it beside one more channel is the same
+// request as naming it alone. The nil slice is how "every channel" travels: a
+// command that only read asks for no zone at all and gets the same thing.
+func TestResolveZoneNameTreatsAllAsEveryChannel(t *testing.T) {
+	display := map[uint16]string{2: "logo", 3: "backlight", 4: "side"}
+
+	for _, zone := range []string{"all", "ALL", "All", " all ", "side,all", "all,logo"} {
+		t.Run(zone, func(t *testing.T) {
+			got, err := resolveZoneName(zone, display, nil)
+			if err != nil {
+				t.Fatalf("resolveZoneName(%q) error = %v", zone, err)
+			}
+			if got != nil {
+				t.Errorf("resolveZoneName(%q) = %v, want nil, which means every channel", zone, got)
+			}
+		})
+	}
+}
+
+// A list that names no channel is a typo, and guessing which one was meant writes
+// to a channel nobody asked for.
+func TestResolveZoneNameRefusesAnEmptyNameInAList(t *testing.T) {
+	cases := []struct {
+		zone string
+		want string
+	}{
+		{"logo,", "logo,"},
+		{",logo", ",logo"},
+		{"logo,,side", "logo,,side"},
+		{"logo,   ", "logo,"},
+		{",", ","},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.zone, func(t *testing.T) {
+			_, err := resolveZoneName(tc.zone, map[uint16]string{2: "logo", 4: "side"}, nil)
+			if err == nil {
+				t.Fatalf("resolveZoneName(%q) = nil error, want the empty name refused", tc.zone)
+			}
+			if !strings.Contains(err.Error(), "empty") {
+				t.Errorf("error = %q, want it to say the list names no channel", err)
+			}
+			if !strings.Contains(err.Error(), tc.want) {
+				t.Errorf("error = %q, want it to quote %q", err, tc.want)
+			}
+		})
+	}
+}
+
+// A name the tool cannot place is reported with the list it came from, because
+// "unknown zone" on its own would be a claim about a word the user never wrote.
+func TestResolveZoneNameNamesTheListItCameFrom(t *testing.T) {
+	_, err := resolveZoneName("logo,nope", map[uint16]string{2: "logo"}, nil)
+	if err == nil {
+		t.Fatal("resolveZoneName() = nil error, want the unknown name refused")
+	}
+	for _, want := range []string{`"nope"`, `in "logo,nope"`, zoneAll} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("error = %q, want it to contain %q", err, want)
+		}
+	}
+}
+
+// One ambiguous name refuses the whole list, before the other names are written
+// to. A board that gave the same name to two channels has a name that means both
+// or neither, and resolving it to one of them is a retarget nobody asked for.
+func TestResolveZoneNameRefusesAWholeListOverOneAmbiguousName(t *testing.T) {
+	_, err := resolveZoneName("logo,backlight", map[uint16]string{2: "logo", 3: "backlight", 4: "backlight"}, nil)
+	if err == nil {
+		t.Fatal("resolveZoneName() = nil error, want the ambiguous name refused")
+	}
+	if !strings.Contains(err.Error(), "several channels") {
+		t.Errorf("error = %q, want it to say the name is ambiguous", err)
+	}
+}
+
 func TestDisplayNameConflictsRejectsAShadowedSubsystemName(t *testing.T) {
 	// Channel 1 is present and its subsystem is "backlight", while the file
 	// also calls channel 3 "backlight": two channels, one name.
@@ -119,18 +235,15 @@ func TestPrepareTargetUsesTheSelectedKeyboard(t *testing.T) {
 
 	originalDiscover := discoverAll
 	originalTarget := targetDevice
-	originalZone := targetZone
 	t.Cleanup(func() {
 		discoverAll = originalDiscover
 		targetDevice = originalTarget
-		targetZone = originalZone
 	})
 
 	discoverAll = func() ([]intdevice.Device, error) { return devices, nil }
 	targetDevice = "2"
-	targetZone = "logo"
 
-	got, err := prepareTarget()
+	got, err := prepareTarget("logo")
 	if err != nil {
 		t.Fatalf("prepareTarget() error = %v", err)
 	}
@@ -186,25 +299,29 @@ func TestLoadWarnsForEveryUnresolvableZoneKey(t *testing.T) {
 }
 
 // stubTargetForProfileTest points the profile commands at one protocol, one
-// profile directory and one --zone value, and returns the restore function.
-func stubTargetForProfileTest(t *testing.T, proto rgbProtocol, zoneFlag string, display map[uint16]string) func() {
+// profile directory and one zone, and returns the restore function.
+func stubTargetForProfileTest(t *testing.T, proto rgbProtocol, zone string, display map[uint16]string) func() {
 	t.Helper()
 
 	originalTarget := openTarget
-	originalZone := targetZone
 
-	openTarget = func() (rgbProtocol, targetDeviceData, []via.Channel, error) {
+	openTarget = func(requested string) (rgbProtocol, targetDeviceData, []via.Channel, error) {
 		// The board identity matters: the catalog is selected by VID/PID, and a
 		// profile that names an effect needs one to resolve it against.
 		target := targetDeviceData{
 			Device:  intdevice.Device{VendorID: 0x36B0, ProductID: 0x309F},
 			Display: display,
 		}
-		requested, err := resolveZoneName(zoneFlag, display, nil)
+		// A profile test says which zone the command was given; the seam falls
+		// back to the one the test fixed so the stub works either way.
+		if requested == "" {
+			requested = zone
+		}
+		channels, err := resolveZoneName(requested, display, nil)
 		if err != nil {
 			return nil, target, nil, err
 		}
-		target.Requested = requested
+		target.Requested = channels
 
 		resolved, err := resolveChannels(proto, target)
 		if err != nil {
@@ -212,12 +329,8 @@ func stubTargetForProfileTest(t *testing.T, proto rgbProtocol, zoneFlag string,
 		}
 		return proto, target, resolved, nil
 	}
-	targetZone = zoneFlag
 
-	return func() {
-		openTarget = originalTarget
-		targetZone = originalZone
-	}
+	return func() { openTarget = originalTarget }
 }
 
 // A key that names a channel this keyboard does not have must be reported like
@@ -317,7 +430,7 @@ func stubTargetForUnknownBoard(t *testing.T, proto rgbProtocol, dir string) func
 	originalTarget := openTarget
 	originalDir := profilesDirOverride
 
-	openTarget = func() (rgbProtocol, targetDeviceData, []via.Channel, error) {
+	openTarget = func(string) (rgbProtocol, targetDeviceData, []via.Channel, error) {
 		return proto, targetDeviceData{
 			Device: intdevice.Device{VendorID: 0x6666, ProductID: 0x0001},
 		}, impact80Channels(), nil
@@ -366,8 +479,8 @@ func TestSaveWarnsThatEffectNamesCannotBeRecorded(t *testing.T) {
 
 // A channel's own definition label and the subsystem name it replaces can both
 // match one name, and that is one channel rather than a conflict: the
-// documented `--zone backlight` has to keep working on a board whose definition
-// calls the channel "Backlight".
+// documented `brightness backlight 100` has to keep working on a board whose
+// definition calls the channel "Backlight".
 func TestResolveZoneNameAcceptsTheNameADefinitionReplaced(t *testing.T) {
 	display := map[uint16]string{2: "logo", 3: "Backlight", 4: "side"}
 	alternatives := map[uint16][]string{3: {"backlight"}}

+ 9 - 10
cmd/qmk-rgb-tool/color.go

@@ -9,10 +9,10 @@ import (
 )
 
 func NewColorCmd() *cobra.Command {
-	return &cobra.Command{
-		Use:   "color <hex>|rgb:<hex>|hsv:<h>,<s>,<v>",
-		Short: "Set RGB color",
-		Long: "Set the color of the selected zones from one of three notations:\n" +
+	return withZoneArgs(&cobra.Command{
+		Use:   "color <zone> <hex>|rgb:<hex>|hsv:<h>,<s>,<v>",
+		Short: "Set RGB color on a zone",
+		Long: writesLighting("Set the color of the named zones from one of three notations:\n" +
 			"  ff0000          six digit hex, hue and saturation only\n" +
 			"  rgb:ff0000      the same, written out\n" +
 			"  hsv:0,255,255   hue, saturation and value, each 0-255\n" +
@@ -23,15 +23,14 @@ func NewColorCmd() *cobra.Command {
 			"brightness the keyboard ended up holding is reported.\n" +
 			"\n" +
 			"Every notation is read back, so a color the keyboard rounded or refused\n" +
-			"is reported instead of being claimed as set.",
-		Args: cobra.ExactArgs(1),
+			"is reported instead of being claimed as set."),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			spec, err := intrgb.ParseColorSpec(args[0])
+			spec, err := intrgb.ParseColorSpec(args[1])
 			if err != nil {
 				return err
 			}
 
-			proto, target, channels, err := openTarget()
+			proto, target, channels, err := openTarget(args[0])
 			if err != nil {
 				return err
 			}
@@ -49,13 +48,13 @@ func NewColorCmd() *cobra.Command {
 
 			brightness, err := setBrightnessVerified(proto, channels, target.Display, spec.V)
 			if err != nil {
-				return fmt.Errorf("set brightness from %s: %w", args[0], err)
+				return fmt.Errorf("set brightness from %s: %w", args[1], err)
 			}
 
 			printColorResult(cmd, colors, brightness)
 			return nil
 		},
-	}
+	}, 2, 2, "a zone and a color")
 }
 
 // printColorResult states the color every zone actually holds. The brightness

+ 22 - 25
cmd/qmk-rgb-tool/color_verify_test.go

@@ -138,16 +138,18 @@ func TestSetColorVerifiedPropagatesWriteErrors(t *testing.T) {
 	}
 }
 
-func runColor(t *testing.T, proto *colorProtocol, zoneFlag string, args ...string) (stdout, stderr string, err error) {
+func runColor(t *testing.T, proto *colorProtocol, zone string, args ...string) (stdout, stderr string, err error) {
 	t.Helper()
 
-	t.Cleanup(stubColorTarget(t, proto, zoneFlag))
+	t.Cleanup(stubColorTarget(t, proto, zone))
 
 	var out, errOut bytes.Buffer
 	cmd := NewColorCmd()
 	cmd.SetOut(&out)
 	cmd.SetErr(&errOut)
-	cmd.SetArgs(args)
+	// The zone is the first argument; whatever notation the test follows comes
+	// after it, so the command sees the spelling a user would type.
+	cmd.SetArgs(append([]string{zone}, args...))
 
 	err = cmd.Execute()
 	return out.String(), errOut.String(), err
@@ -182,7 +184,7 @@ func TestColorReportsTheAppliedHueAndSaturation(t *testing.T) {
 func TestColorHexNotationLeavesBrightnessAlone(t *testing.T) {
 	proto := &colorProtocol{appliedColor: allZonesAppliedColor(0, 255)}
 
-	stdout, _, err := runColor(t, proto, "", "rgb:ff0000")
+	stdout, _, err := runColor(t, proto, "all", "rgb:ff0000")
 	if err != nil {
 		t.Fatalf("color returned error: %v", err)
 	}
@@ -202,7 +204,7 @@ func TestColorHSVNotationWritesTheValueAsBrightness(t *testing.T) {
 		appliedBright: map[via.Channel]uint8{via.ChannelRgblight: 200, via.ChannelRgbMatrix: 200, via.ChannelAudio: 200},
 	}
 
-	stdout, _, err := runColor(t, proto, "", "hsv:85,255,200")
+	stdout, _, err := runColor(t, proto, "all", "hsv:85,255,200")
 	if err != nil {
 		t.Fatalf("color returned error: %v", err)
 	}
@@ -227,7 +229,7 @@ func TestColorHSVSummarisesTheBrightnessTheKeyboardApplied(t *testing.T) {
 		appliedBright: map[via.Channel]uint8{via.ChannelRgblight: 160, via.ChannelRgbMatrix: 255, via.ChannelAudio: 160},
 	}
 
-	stdout, _, err := runColor(t, proto, "", "hsv:0,255,200")
+	stdout, _, err := runColor(t, proto, "all", "hsv:0,255,200")
 	if err != nil {
 		t.Fatalf("color returned error: %v", err)
 	}
@@ -256,22 +258,17 @@ func TestColorRejectsAnUnknownNotationWithoutOpeningTheDevice(t *testing.T) {
 
 	opened := false
 	originalTarget := openTarget
-	originalZone := targetZone
-	t.Cleanup(func() {
-		openTarget = originalTarget
-		targetZone = originalZone
-	})
-	openTarget = func() (rgbProtocol, targetDeviceData, []via.Channel, error) {
+	t.Cleanup(func() { openTarget = originalTarget })
+	openTarget = func(string) (rgbProtocol, targetDeviceData, []via.Channel, error) {
 		opened = true
 		return proto, targetDeviceData{}, nil, nil
 	}
-	targetZone = ""
 
 	var out bytes.Buffer
 	cmd := NewColorCmd()
 	cmd.SetOut(&out)
 	cmd.SetErr(&out)
-	cmd.SetArgs([]string{"xyz:1"})
+	cmd.SetArgs([]string{"all", "xyz:1"})
 
 	err := cmd.Execute()
 	if err == nil {
@@ -285,20 +282,24 @@ func TestColorRejectsAnUnknownNotationWithoutOpeningTheDevice(t *testing.T) {
 	}
 }
 
-// stubColorTarget points the color command at one protocol and one --zone value.
-func stubColorTarget(t *testing.T, proto *colorProtocol, zoneFlag string) func() {
+// stubColorTarget points the color command at one protocol, resolving the zone
+// the command was given. A test that passes no zone means every channel, which is
+// the `all` spelling rather than a missing argument.
+func stubColorTarget(t *testing.T, proto *colorProtocol, zone string) func() {
 	t.Helper()
 
 	originalTarget := openTarget
-	originalZone := targetZone
 
-	openTarget = func() (rgbProtocol, targetDeviceData, []via.Channel, error) {
+	openTarget = func(requested string) (rgbProtocol, targetDeviceData, []via.Channel, error) {
 		target := targetDeviceData{Display: impact80Display()}
-		requested, err := resolveZoneName(zoneFlag, impact80Display(), nil)
+		if requested == "" {
+			requested = zone
+		}
+		channels, err := resolveZoneName(requested, impact80Display(), nil)
 		if err != nil {
 			return nil, target, nil, err
 		}
-		target.Requested = requested
+		target.Requested = channels
 
 		resolved, err := resolveChannels(proto, target)
 		if err != nil {
@@ -306,10 +307,6 @@ func stubColorTarget(t *testing.T, proto *colorProtocol, zoneFlag string) func()
 		}
 		return proto, target, resolved, nil
 	}
-	targetZone = zoneFlag
 
-	return func() {
-		openTarget = originalTarget
-		targetZone = originalZone
-	}
+	return func() { openTarget = originalTarget }
 }

+ 38 - 12
cmd/qmk-rgb-tool/completion.go

@@ -6,7 +6,6 @@ import (
 	"strings"
 
 	"github.com/spf13/cobra"
-	intrgb "netdome.biz/paul/qmk-rgb/internal/rgb"
 	"netdome.biz/paul/qmk-rgb/internal/via"
 )
 
@@ -15,24 +14,50 @@ import (
 // to say it. None of these open the keyboard: a shell asks while nothing is
 // plugged in, and an error printed into a prompt is worse than an empty list.
 
-// completeZoneNames offers the QMK subsystem names, which follow from the channel
-// number and work on any board, plus the names the definition files give their
-// channels, which are what a board calls them in VIA.
+// completeZoneNames offers the word `all`, then the QMK subsystem names, which
+// follow from the channel number and work on any board, then the names the
+// definition files give their channels, which are what a board calls them in VIA.
 func completeZoneNames(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-	var names []string
+	names := []string{zoneAll}
 	for _, ch := range via.LightingChannels {
 		names = append(names, ch.Subsystem())
 	}
-	if defs, err := intrgb.LoadDefinitionsDir(definitionsPath()); err == nil {
-		for _, def := range defs {
-			for _, label := range def.Labels {
-				names = append(names, label)
-			}
+	// Every definition the tool knows, not only the ones in the data directory:
+	// the Impact 80's file is built into the binary, so a board that names its
+	// channels logo, Backlight and side offers those names in a shell that has
+	// never been given a definitions directory. A name the user cannot see is a
+	// name they will not type.
+	for _, def := range candidateDefinitions() {
+		for _, label := range def.Labels {
+			names = append(names, label)
 		}
 	}
 	return narrow(names, toComplete), cobra.ShellCompDirectiveNoFileComp
 }
 
+// completeZoneArgs offers zone names for a command's zone and nothing after it.
+// The effect name `effect` takes second would have to come from a definition
+// file chosen for the connected board, and a shell asks while nothing is
+// plugged in, so a list built without a keyboard is the whole of what can be
+// offered honestly.
+func completeZoneArgs(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+	if len(args) > 0 {
+		return nil, cobra.ShellCompDirectiveNoFileComp
+	}
+	return completeZoneNames(cmd, args, toComplete)
+}
+
+// completeLoadArgs offers a profile name first and a zone second, which is the
+// order the command reads them in. The zone goes through completeZoneNames
+// directly rather than through completeZoneArgs, which stops after one argument
+// because for every other command the zone is that first argument.
+func completeLoadArgs(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+	if len(args) == 0 {
+		return completeProfileNames(cmd, args, toComplete)
+	}
+	return completeZoneNames(cmd, args, toComplete)
+}
+
 // completeDeviceNumbers offers the keyboard numbers keyboard info prints, which
 // come from enumeration and need no open keyboard.
 func completeDeviceNumbers(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
@@ -78,9 +103,10 @@ func narrow(names []string, prefix string) []string {
 }
 
 // registerFlagCompletions teaches the shell what the persistent flags accept. The
-// definition file is a path, so the shell completes paths for it on its own.
+// definition file is a path, so the shell completes paths for it on its own. The
+// zones are positional, so they are completed by the commands that take one, in
+// completeZoneArgs.
 func registerFlagCompletions(root *cobra.Command) {
-	_ = root.RegisterFlagCompletionFunc("zone", completeZoneNames)
 	_ = root.RegisterFlagCompletionFunc("device", completeDeviceNumbers)
 	_ = root.MarkFlagFilename("definition", "*.json")
 }

+ 112 - 4
cmd/qmk-rgb-tool/completion_test.go

@@ -12,10 +12,10 @@ import (
 	intdevice "netdome.biz/paul/qmk-rgb/internal/device"
 )
 
-// The tool knows how to resolve --zone, but the shell only offers what the binary
-// tells it about, and it told it nothing: the flag was offered, its values were
-// not. These cases fix that, and they are worth fixing because --zone and the
-// profile names are the two places a user types something the tool already knows.
+// The tool knows what a zone may be, but the shell only offers what the binary
+// tells it about. The zone is a positional argument, so the completion hangs off
+// the command rather than off a flag, and a board's own channel names come from
+// a definition file the tool reads.
 func TestZoneCompletionOffersSubsystemsAndTheBoardsOwnNames(t *testing.T) {
 	dir := t.TempDir()
 	writeDefinition(t, dir, "impact80.json", `{
@@ -62,6 +62,114 @@ func TestZoneCompletionNarrowsToThePrefix(t *testing.T) {
 	}
 }
 
+// `all` is a zone name like any other, and the shell has to offer it: it is the
+// one spelling that reaches every channel, and a user who cannot see it will not
+// reach every channel.
+func TestZoneCompletionOffersAll(t *testing.T) {
+	t.Cleanup(forceDefinitionsDir(t, t.TempDir()))
+
+	got, _ := completeZoneNames(nil, nil, "")
+	if !contains(got, "all") {
+		t.Errorf("completeZoneNames() = %v, want it to offer \"all\"", got)
+	}
+}
+
+// A board's own names come from a definition file, and the one built into the
+// binary counts as much as one in the data directory. `go install` creates no
+// data directory, so a shell that only read that one would offer a board nothing
+// but the subsystem names — and the subsystem names a board renamed are not the
+// names it answers to.
+func TestZoneCompletionOffersTheBuiltInDefinitionsToo(t *testing.T) {
+	t.Cleanup(forceDefinitionsDir(t, t.TempDir()))
+
+	got, _ := completeZoneNames(nil, nil, "")
+	// The Impact 80's file is the one that ships, and it names three channels.
+	for _, want := range []string{"logo", "Backlight", "side"} {
+		if !contains(got, want) {
+			t.Errorf("completeZoneNames() = %v, want the built-in definition's name %q", got, want)
+		}
+	}
+}
+
+// The zone is a positional argument now, so the completion hangs off the command
+// rather than off a flag, and it stops after the first argument rather than
+// offering a channel where a value belongs.
+func TestZoneCompletionIsOnTheCommandNotTheFlag(t *testing.T) {
+	lighting := map[string]*cobra.Command{
+		"effect":     NewEffectCmd(),
+		"brightness": NewBrightnessCmd(),
+		"speed":      NewSpeedCmd(),
+		"color":      NewColorCmd(),
+		"enable":     NewEnableCmd(),
+		"disable":    NewDisableCmd(),
+		"info":       NewInfoCmd(),
+	}
+
+	for name, cmd := range lighting {
+		t.Run(name, func(t *testing.T) {
+			if cmd.ValidArgsFunction == nil {
+				t.Fatalf("%s has no ValidArgsFunction, want the shell offered the zone names", name)
+			}
+			got, _ := cmd.ValidArgsFunction(cmd, nil, "")
+			if !contains(got, "all") {
+				t.Errorf("%s completion = %v, want it to offer the zones", name, got)
+			}
+			// After the zone, the next argument is a value or a name the tool
+			// cannot know without opening the keyboard.
+			after, directive := cmd.ValidArgsFunction(cmd, []string{"logo"}, "")
+			if len(after) != 0 {
+				t.Errorf("%s completion after a zone = %v, want nothing", name, after)
+			}
+			if directive != cobra.ShellCompDirectiveNoFileComp {
+				t.Errorf("%s directive = %v, want NoFileComp", name, directive)
+			}
+		})
+	}
+}
+
+// `load` takes a profile name first and a zone second, in that order, so the two
+// completions cannot be swapped without offering something that does not exist.
+func TestLoadCompletionIsAProfileThenAZone(t *testing.T) {
+	t.Cleanup(forceDefinitionsDir(t, t.TempDir()))
+
+	cmd := NewProfileLoadCmd()
+	first, _ := cmd.ValidArgsFunction(cmd, nil, "")
+	if !contains(first, "lava") {
+		t.Errorf("load completion = %v, want the profile names", first)
+	}
+
+	second, _ := cmd.ValidArgsFunction(cmd, []string{"lava"}, "")
+	if contains(second, "lava") {
+		t.Errorf("load completion after a name = %v, want zones, not more profiles", second)
+	}
+	if !contains(second, "all") {
+		t.Errorf("load completion after a name = %v, want the zone names", second)
+	}
+}
+
+// A command that cannot address a channel offers no zone, so the shell does not
+// suggest one where there is none to use. `delete` completes a profile name, so
+// what matters is that the candidates are the names, not the channels.
+func TestCommandsWithoutAChannelOfferNoZone(t *testing.T) {
+	for name, cmd := range map[string]*cobra.Command{
+		"list": NewProfileListCmd(),
+		"save": NewProfileSaveCmd(),
+	} {
+		t.Run(name, func(t *testing.T) {
+			if cmd.ValidArgsFunction != nil {
+				t.Errorf("%s offers completion, want only the commands that name a channel to", name)
+			}
+		})
+	}
+
+	profiles, _ := NewProfileDeleteCmd().ValidArgsFunction(nil, nil, "")
+	for _, candidate := range profiles {
+		if candidate == "all" || candidate == "rgb_matrix" {
+			t.Errorf("delete completion = %v, want profile names, not channels", profiles)
+		}
+	}
+}
+
 // A definition that is not there must not break completion: a shell prints the
 // error text, which is worse than offering nothing.
 func TestZoneCompletionSurvivesAMissingDataDirectory(t *testing.T) {

+ 3 - 1
cmd/qmk-rgb-tool/definition.go

@@ -61,7 +61,9 @@ func NewKeyboardDefinitionsCmd() *cobra.Command {
 }
 
 func runKeyboardFetch(cmd *cobra.Command, args []string) error {
-	target, err := prepareTarget()
+	// A definition is a property of a board, not of a lighting channel, so this
+	// asks for the board alone and names no zone.
+	target, err := prepareTarget("")
 	if err != nil {
 		return err
 	}

+ 1 - 1
cmd/qmk-rgb-tool/definition_test.go

@@ -233,7 +233,7 @@ func stubHTTPGet(t *testing.T, fn func(url string) ([]byte, int, error)) {
 func stubPrepareTarget(t *testing.T, vendorID, productID uint16) {
 	t.Helper()
 	original := prepareTarget
-	prepareTarget = func() (targetDeviceData, error) { return stubTargetData(vendorID, productID), nil }
+	prepareTarget = func(string) (targetDeviceData, error) { return stubTargetData(vendorID, productID), nil }
 	t.Cleanup(func() { prepareTarget = original })
 }
 

+ 6 - 6
cmd/qmk-rgb-tool/disable.go

@@ -7,12 +7,12 @@ import (
 )
 
 func NewDisableCmd() *cobra.Command {
-	return &cobra.Command{
-		Use:   "disable",
-		Short: "Disable RGB lighting",
-		Args:  cobra.NoArgs,
+	return withZoneArgs(&cobra.Command{
+		Use:   "disable <zone>",
+		Short: "Disable RGB lighting on a zone",
+		Long:  writesLighting("Turn the lighting off on the named channels, by writing brightness 0."),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			proto, _, channels, err := openTarget()
+			proto, _, channels, err := openTarget(args[0])
 			if err != nil {
 				return err
 			}
@@ -25,5 +25,5 @@ func NewDisableCmd() *cobra.Command {
 			fmt.Fprintln(cmd.OutOrStdout(), "RGB disabled")
 			return nil
 		},
-	}
+	}, 1, 1, "a zone")
 }

+ 26 - 30
cmd/qmk-rgb-tool/effect.go

@@ -4,44 +4,41 @@ import (
 	"errors"
 	"fmt"
 	"strconv"
-	"strings"
 
 	"github.com/spf13/cobra"
 	intrgb "netdome.biz/paul/qmk-rgb/internal/rgb"
 	"netdome.biz/paul/qmk-rgb/internal/via"
 )
 
-var listEffects bool
-
 // resolveEffectTargets turns a name into one target per channel. Whether the
-// user named a channel is passed in, because a board with a single channel is
-// not an explicit request for it.
-func resolveEffectTargets(catalog *intrgb.Catalog, name string, channels []via.Channel) ([]intrgb.EffectTarget, []string, error) {
-	return intrgb.ResolveEffect(catalog, name, channels, targetZone != "")
+// channels were named one by one is passed in, because a name the board does not
+// have on a channel it was not asked about is a skip, and the same name on a
+// channel it was asked about is a mistake worth reporting.
+func resolveEffectTargets(catalog *intrgb.Catalog, name string, channels []via.Channel, named bool) ([]intrgb.EffectTarget, []string, error) {
+	return intrgb.ResolveEffect(catalog, name, channels, named)
 }
 
 func NewEffectCmd() *cobra.Command {
-	cmd := &cobra.Command{
-		Use:   "effect [name|index]",
-		Short: "Set or list RGB effects",
-		Long: "Set the RGB lighting effect on the connected keyboard. Without an argument, lists all effects.\n" +
-			"Effect names come from a per-board catalog; a keyboard without one has no names\n" +
-			"and is driven with `effect <index>` instead.\n" +
-			"A number is an effect ID; anything else is a name.",
-		Args: cobra.MaximumNArgs(1),
+	return withZoneArgs(&cobra.Command{
+		Use:   "effect <zone> [name|index]",
+		Short: "Set or list RGB effects on a zone",
+		Long: writesLighting("Set the RGB lighting effect on the named zones. Effect names come from a\n" +
+			"per-board catalog; a keyboard without one has no names and is driven with an\n" +
+			"effect index instead. A number is an effect ID; anything else is a name.\n" +
+			"\n" +
+			"Without a name this lists the effects each named zone has. Listing is not a\n" +
+			"write, so `effect all` reports every channel the keyboard has."),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if listEffects || len(args) == 0 {
-				return listAllEffects(cmd)
+			if len(args) == 1 {
+				return listZoneEffects(cmd, args[0])
 			}
-			return runEffectSet(cmd, args)
+			return runEffectSet(cmd, args[0], args[1])
 		},
-	}
-	cmd.Flags().BoolVar(&listEffects, "list", false, "List all available effects per zone")
-	return cmd
+	}, 1, 2, "a zone, and an effect name or index to set")
 }
 
-func listAllEffects(cmd *cobra.Command) error {
-	proto, target, channels, err := openTarget()
+func listZoneEffects(cmd *cobra.Command, zone string) error {
+	proto, target, channels, err := openTarget(zone)
 	if err != nil {
 		return err
 	}
@@ -118,14 +115,14 @@ func parseEffectArgument(arg string) (effectArgument, error) {
 	return effectArgument{Name: arg}, nil
 }
 
-func runEffectSet(cmd *cobra.Command, args []string) error {
-	proto, target, channels, err := openTarget()
+func runEffectSet(cmd *cobra.Command, zone, arg string) error {
+	proto, target, channels, err := openTarget(zone)
 	if err != nil {
 		return err
 	}
 	defer proto.Close()
 
-	want, err := parseEffectArgument(args[0])
+	want, err := parseEffectArgument(arg)
 	if err != nil {
 		return err
 	}
@@ -137,13 +134,12 @@ func runEffectSet(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	targets, skipped, err := resolveEffectTargets(catalog, want.Name, channels)
+	// A zone was named, so a channel that cannot do this effect is a mistake and
+	// not a reason to write the other ones and report a success.
+	targets, _, err := resolveEffectTargets(catalog, want.Name, channels, true)
 	if err != nil {
 		return err
 	}
-	if len(skipped) > 0 {
-		fmt.Fprintf(cmd.ErrOrStderr(), "Warning: effect not supported on channel(s): %s\n", strings.Join(skipped, ", "))
-	}
 
 	results, err := setEffectVerified(proto, targets, target.Display, catalog)
 	if err != nil {

+ 5 - 4
cmd/qmk-rgb-tool/effect_list_test.go

@@ -7,19 +7,20 @@ import (
 )
 
 // Every other JSON command emits an object, so a consumer can add fields
-// later without breaking. `effect --list` returned a bare array because the
+// later without breaking. The effect list returned a bare array because the
 // EffectList wrapper it built was bypassed in favour of its slice.
 func TestEffectListEmitsAnObjectNotABareArray(t *testing.T) {
 	t.Cleanup(vendoredDefinitions(t))
 	withJSON(t)
+	t.Cleanup(impact80Target(t, &fakeZoneProtocol{failAt: -1}))
 	var out, errOut bytes.Buffer
 	cmd := NewEffectCmd()
 	cmd.SetOut(&out)
 	cmd.SetErr(&errOut)
-	cmd.SetArgs([]string{"--list"})
+	cmd.SetArgs([]string{zoneAll})
 
 	if err := cmd.Execute(); err != nil {
-		t.Fatalf("effect --list returned error: %v", err)
+		t.Fatalf("effect %s returned error: %v", zoneAll, err)
 	}
 
 	var parsed struct {
@@ -33,7 +34,7 @@ func TestEffectListEmitsAnObjectNotABareArray(t *testing.T) {
 		} `json:"zones"`
 	}
 	if err := json.Unmarshal(out.Bytes(), &parsed); err != nil {
-		t.Fatalf("effect --list output is not a JSON object: %v (output %q)", err, out.String()[:min(80, out.Len())])
+		t.Fatalf("effect all output is not a JSON object: %v (output %q)", err, out.String()[:min(80, out.Len())])
 	}
 	if parsed.Catalog != "impact80" {
 		// The catalog is named by the definition file it was read from, so the name

+ 61 - 28
cmd/qmk-rgb-tool/effect_test.go

@@ -23,8 +23,27 @@ func impact80Catalog(t *testing.T) *intrgb.Catalog {
 	return def.Catalog
 }
 
-func TestResolveEffectTargetsBacklightOnly(t *testing.T) {
-	targets, skipped, err := resolveEffectTargets(impact80Catalog(t), "rainbow_moving_chevron", impact80Channels())
+// A name the board has but a named channel does not is refused, however many
+// channels can do it. `effect all` asks for every channel, so a name only one of
+// them has is a request the tool cannot carry out rather than one channel written
+// and reported as done.
+func TestResolveEffectTargetsRefusesANameNotEveryNamedChannelHas(t *testing.T) {
+	targets, skipped, err := resolveEffectTargets(impact80Catalog(t), "rainbow_moving_chevron", impact80Channels(), true)
+	if err == nil {
+		t.Fatal("resolveEffectTargets() expected error, got nil")
+	}
+	if len(targets) != 0 {
+		t.Errorf("resolveEffectTargets() targets = %v, want none", targets)
+	}
+	if len(skipped) != 0 {
+		t.Errorf("resolveEffectTargets() skipped = %v, want none", skipped)
+	}
+}
+
+// Without a channel named, a name the board does not have everywhere is skipped
+// with a warning, which is the shape `load` uses when it applies a whole profile.
+func TestResolveEffectTargetsSkipsWhenNoChannelWasNamed(t *testing.T) {
+	targets, skipped, err := resolveEffectTargets(impact80Catalog(t), "rainbow_moving_chevron", impact80Channels(), false)
 	if err != nil {
 		t.Fatalf("resolveEffectTargets() unexpected error: %v", err)
 	}
@@ -39,7 +58,7 @@ func TestResolveEffectTargetsBacklightOnly(t *testing.T) {
 }
 
 func TestResolveEffectTargetsBreathing(t *testing.T) {
-	targets, skipped, err := resolveEffectTargets(impact80Catalog(t), "breathing", impact80Channels())
+	targets, skipped, err := resolveEffectTargets(impact80Catalog(t), "breathing", impact80Channels(), true)
 	if err != nil {
 		t.Fatalf("resolveEffectTargets() unexpected error: %v", err)
 	}
@@ -67,13 +86,7 @@ func TestResolveEffectTargetsExplicitUnsupported(t *testing.T) {
 	}
 	for _, tc := range cases {
 		t.Run(tc.name+string(tc.zones[0]), func(t *testing.T) {
-			// A name the board has but the named channel does not is refused,
-			// so this only means anything when a zone was actually named.
-			originalZone := targetZone
-			t.Cleanup(func() { targetZone = originalZone })
-			targetZone = tc.zones[0].Subsystem()
-
-			targets, skipped, err := resolveEffectTargets(impact80Catalog(t), tc.name, tc.zones)
+			targets, skipped, err := resolveEffectTargets(impact80Catalog(t), tc.name, tc.zones, true)
 			if err == nil {
 				t.Fatal("resolveEffectTargets() expected error, got nil")
 			}
@@ -88,7 +101,7 @@ func TestResolveEffectTargetsExplicitUnsupported(t *testing.T) {
 }
 
 func TestResolveEffectTargetsUnknownName(t *testing.T) {
-	targets, skipped, err := resolveEffectTargets(impact80Catalog(t), "not_an_effect", impact80Channels())
+	targets, skipped, err := resolveEffectTargets(impact80Catalog(t), "not_an_effect", impact80Channels(), true)
 	if err == nil {
 		t.Fatal("resolveEffectTargets() expected error, got nil")
 	}
@@ -101,7 +114,7 @@ func TestResolveEffectTargetsUnknownName(t *testing.T) {
 }
 
 func TestResolveEffectTargetsStaticCompatibility(t *testing.T) {
-	targets, skipped, err := resolveEffectTargets(impact80Catalog(t), "static", impact80Channels())
+	targets, skipped, err := resolveEffectTargets(impact80Catalog(t), "static", impact80Channels(), true)
 	if err != nil {
 		t.Fatalf("resolveEffectTargets() unexpected error: %v", err)
 	}
@@ -118,11 +131,8 @@ func TestResolveEffectTargetsStaticCompatibility(t *testing.T) {
 	}
 }
 
-func executeEffectCommand(t *testing.T, protocol rgbProtocol, target string, args ...string) (string, string, error) {
+func executeEffectCommand(t *testing.T, protocol rgbProtocol, zone string, args ...string) (string, string, error) {
 	t.Helper()
-	originalZone := targetZone
-	t.Cleanup(func() { targetZone = originalZone })
-	targetZone = target
 	t.Cleanup(stubOpenTarget(t, protocol, impact80Display(), impact80Channels(), 0x36B0, 0x309F))
 
 	var stdout bytes.Buffer
@@ -132,29 +142,52 @@ func executeEffectCommand(t *testing.T, protocol rgbProtocol, target string, arg
 	cmd.SetErr(&stderr)
 	cmd.SilenceErrors = true
 	cmd.SilenceUsage = true
-	cmd.SetArgs(args)
+	// The zone is the first argument, so the effect name follows it, which is
+	// the spelling a user would type.
+	cmd.SetArgs(append([]string{zone}, args...))
 	err := cmd.Execute()
 	return stdout.String(), stderr.String(), err
 }
 
-func TestEffectCommandWarnsForSkippedZones(t *testing.T) {
+// `effect all breathing` reaches every channel, because the name means the same
+// index on all three subsystems of this board.
+func TestEffectAllWritesEveryChannelTheNameIsOn(t *testing.T) {
 	t.Cleanup(vendoredDefinitions(t))
 	protocol := &fakeZoneProtocol{failAt: -1}
-	stdout, stderr, err := executeEffectCommand(t, protocol, "", "rainbow_moving_chevron")
+	stdout, _, err := executeEffectCommand(t, protocol, "all", "breathing")
 	if err != nil {
 		t.Fatalf("Execute() unexpected error: %v", err)
 	}
-	if !strings.Contains(stderr, "effect not supported on channel(s): rgblight, audio") {
-		t.Errorf("stderr = %q, want skipped-zone warning", stderr)
-	}
-	if !strings.Contains(stdout, `Effect set to "rainbow_moving_chevron"`) {
+	if !strings.Contains(stdout, `Effect set to "breathing"`) {
 		t.Errorf("stdout = %q, want success output", stdout)
 	}
-	if len(protocol.reports) != 1 {
-		t.Fatalf("reports = %v, want one Backlight report", protocol.reports)
+	if len(protocol.reports) != 3 {
+		t.Fatalf("reports = %v, want one per channel", protocol.reports)
+	}
+	want := []commandReport{
+		{channel: 2, param: 2, value: 4},
+		{channel: 3, param: 2, value: 5},
+		{channel: 4, param: 2, value: 4},
+	}
+	if !reflect.DeepEqual(protocol.reports, want) {
+		t.Errorf("reports = %v, want %v", protocol.reports, want)
+	}
+}
+
+// Several zones in one argument reach exactly those channels and no others.
+func TestEffectWritesEveryChannelOfAZoneList(t *testing.T) {
+	t.Cleanup(vendoredDefinitions(t))
+	protocol := &fakeZoneProtocol{failAt: -1}
+	_, _, err := executeEffectCommand(t, protocol, "side,logo", "breathing")
+	if err != nil {
+		t.Fatalf("Execute() unexpected error: %v", err)
+	}
+	want := []commandReport{
+		{channel: 2, param: 2, value: 4},
+		{channel: 4, param: 2, value: 4},
 	}
-	if protocol.reports[0].channel != 3 || protocol.reports[0].param != 2 || protocol.reports[0].value != 17 {
-		t.Errorf("report = %+v, want Backlight Effect 17", protocol.reports[0])
+	if !reflect.DeepEqual(protocol.reports, want) {
+		t.Errorf("reports = %v, want %v", protocol.reports, want)
 	}
 }
 
@@ -175,7 +208,7 @@ func TestEffectCommandWritesNothingForAnUnsupportedEffect(t *testing.T) {
 func TestEffectCommandSuppressesSuccessAfterLaterWriteFailure(t *testing.T) {
 	t.Cleanup(vendoredDefinitions(t))
 	protocol := &fakeZoneProtocol{failAt: 1}
-	stdout, _, err := executeEffectCommand(t, protocol, "", "breathing")
+	stdout, _, err := executeEffectCommand(t, protocol, "all", "breathing")
 	if err == nil {
 		t.Fatal("Execute() expected write error, got nil")
 	}

+ 4 - 6
cmd/qmk-rgb-tool/effect_verify_test.go

@@ -120,14 +120,12 @@ func TestEffectCommandReportsHeldEffect(t *testing.T) {
 func TestEffectCommandStaysQuietWhenApplied(t *testing.T) {
 	t.Cleanup(vendoredDefinitions(t))
 	proto := &verifyingProtocol{
-		applied: map[via.Channel]uint8{
-			via.ChannelRgblight:  1,
-			via.ChannelRgbMatrix: 28,
-			via.ChannelAudio:     1,
-		},
+		applied: map[via.Channel]uint8{via.ChannelRgblight: 1},
 	}
 
-	stdout, _, err := executeEffectCommand(t, proto, "", "wave")
+	// wave is ID 1 on rgblight; the backlight channel numbers its own effects
+	// differently, so a zone is named rather than every channel.
+	stdout, _, err := executeEffectCommand(t, proto, "rgblight", "wave")
 	if err != nil {
 		t.Fatalf("effect returned error: %v", err)
 	}

+ 8 - 7
cmd/qmk-rgb-tool/enable.go

@@ -7,12 +7,13 @@ import (
 )
 
 func NewEnableCmd() *cobra.Command {
-	return &cobra.Command{
-		Use:   "enable",
-		Short: "Enable RGB lighting",
-		Args:  cobra.NoArgs,
+	return withZoneArgs(&cobra.Command{
+		Use:   "enable <zone>",
+		Short: "Enable RGB lighting on a zone",
+		Long: writesLighting("Turn the lighting on on the named channels, at the first effect the\n" +
+			"keyboard's own definition names for each of them and a brightness of 160."),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			proto, target, channels, err := openTarget()
+			proto, target, channels, err := openTarget(args[0])
 			if err != nil {
 				return err
 			}
@@ -25,7 +26,7 @@ func NewEnableCmd() *cobra.Command {
 			for _, ch := range channels {
 				if _, ok := catalog.DefaultEffect(ch); !ok {
 					return fmt.Errorf("this keyboard has no effect names, so `enable` cannot choose an effect: " +
-						"run `keyboard fetch` for its VIA definition, or set one with `effect <index>`")
+						"run `keyboard fetch` for its VIA definition, or set one with `effect <zone> <index>`")
 				}
 			}
 
@@ -36,5 +37,5 @@ func NewEnableCmd() *cobra.Command {
 			fmt.Fprintln(cmd.OutOrStdout(), "RGB enabled")
 			return nil
 		},
-	}
+	}, 1, 1, "a zone")
 }

+ 39 - 18
cmd/qmk-rgb-tool/flags_test.go

@@ -4,6 +4,8 @@ import (
 	"os"
 	"strings"
 	"testing"
+
+	"github.com/spf13/cobra"
 )
 
 // The --device flag accepts the 1-based number that `keyboard info` prints.
@@ -27,10 +29,12 @@ func TestDeviceFlagUsageDescribesANumberNotAPath(t *testing.T) {
 // and renders it where the type name would go, turning `--device into
 // `--device keyboard info`. The help must therefore contain no backticks.
 func TestFlagUsageHasNoValuePlaceholder(t *testing.T) {
-	for _, name := range []string{"device", "zone"} {
-		flag := newRootCommand().PersistentFlags().Lookup(name)
+	root := newRootCommand()
+	for _, name := range []string{"device", "definition", "json"} {
+		flag := root.PersistentFlags().Lookup(name)
 		if flag == nil {
-			t.Fatalf("--%s flag not found", name)
+			t.Errorf("--%s is registered on the root, want it", name)
+			continue
 		}
 		if strings.Contains(flag.Usage, "`") {
 			t.Errorf("--%s usage = %q, backticks make cobra render a bogus value placeholder", name, flag.Usage)
@@ -83,7 +87,7 @@ func TestDocsDoNotTeachAZoneNameTheResolverRejects(t *testing.T) {
 			continue
 		}
 		body := string(data)
-		for _, wrong := range []string{"--zone logo|backlight", "--zone matrix", "zone=matrix"} {
+		for _, wrong := range []string{"zone logo|backlight", "zone matrix", "zone=matrix"} {
 			if strings.Contains(body, wrong) {
 				t.Errorf("%s contains %q, want only the canonical zone names", path, wrong)
 			}
@@ -91,21 +95,38 @@ func TestDocsDoNotTeachAZoneNameTheResolverRejects(t *testing.T) {
 	}
 }
 
-func TestZoneFlagUsageNamesTheChannelVocabulary(t *testing.T) {
-	flag := newRootCommand().PersistentFlags().Lookup("zone")
-	if flag == nil {
-		t.Fatal("--zone flag not found")
+// The zone is a positional argument now, so its help is the Long text the
+// command carries rather than a flag's usage string. Every subsystem name has to
+// be discoverable there, because a user reading `brightness --help` has nowhere
+// else to learn what a zone may be.
+func TestZoneHelpNamesTheChannelVocabulary(t *testing.T) {
+	cmds := map[string]*cobra.Command{
+		"effect":     NewEffectCmd(),
+		"brightness": NewBrightnessCmd(),
+		"speed":      NewSpeedCmd(),
+		"color":      NewColorCmd(),
+		"enable":     NewEnableCmd(),
+		"disable":    NewDisableCmd(),
 	}
 
-	// Every subsystem name must be discoverable from the help text.
-	for _, zone := range []string{"backlight", "rgblight", "rgb_matrix", "audio", "led_matrix"} {
-		if !strings.Contains(flag.Usage, zone) {
-			t.Errorf("--zone usage = %q, want it to list %q", flag.Usage, zone)
-		}
-	}
-	// The board-supplied names are not a fixed vocabulary, so the help must not
-	// claim they are.
-	if strings.Contains(flag.Usage, "logo") || strings.Contains(flag.Usage, "side") {
-		t.Errorf("--zone usage = %q, must not name channels a board supplies", flag.Usage)
+	for name, cmd := range cmds {
+		t.Run(name, func(t *testing.T) {
+			for _, zone := range []string{"backlight", "rgblight", "rgb_matrix", "audio", "led_matrix"} {
+				if !strings.Contains(cmd.Long, zone) {
+					t.Errorf("%s help does not name the subsystem %q", name, zone)
+				}
+			}
+			if !strings.Contains(cmd.Long, "all") {
+				t.Errorf("%s help does not say that all is a zone", name)
+			}
+			if !strings.Contains(cmd.Long, "comma") {
+				t.Errorf("%s help does not say that several zones may be written at once", name)
+			}
+			// The board-supplied names are not a fixed vocabulary, so the help
+			// must not claim they are.
+			if strings.Contains(cmd.Long, "logo") || strings.Contains(cmd.Long, "side") {
+				t.Errorf("%s help names channels a board supplies", name)
+			}
+		})
 	}
 }

+ 11 - 5
cmd/qmk-rgb-tool/info.go

@@ -106,12 +106,18 @@ func getInfoValue(proto infoGetter, channel via.Channel, param uint8, size int)
 }
 
 func NewInfoCmd() *cobra.Command {
-	return &cobra.Command{
-		Use:   "info",
+	return withZoneArgs(&cobra.Command{
+		Use:   "info [zone]",
 		Short: "Show current RGB state",
-		Args:  cobra.NoArgs,
+		Long: "Report what the keyboard holds, one record per channel. Without a zone this\n" +
+			"reports every channel the keyboard has.",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			proto, target, channels, err := openTarget()
+			zone := ""
+			if len(args) == 1 {
+				zone = args[0]
+			}
+
+			proto, target, channels, err := openTarget(zone)
 			if err != nil {
 				return err
 			}
@@ -130,5 +136,5 @@ func NewInfoCmd() *cobra.Command {
 			}
 			return queryErr
 		},
-	}
+	}, 0, 1, "at most a zone")
 }

+ 33 - 3
cmd/qmk-rgb-tool/keyboard_info_test.go

@@ -28,12 +28,15 @@ func runRealKeyboardInfo(t *testing.T) (stdout, stderr string, err error) {
 	t.Helper()
 
 	var out, errOut bytes.Buffer
-	origOut, origErr := keyboardInfoCmd.OutOrStdout(), keyboardInfoCmd.ErrOrStderr()
 	keyboardInfoCmd.SetOut(&out)
 	keyboardInfoCmd.SetErr(&errOut)
+	// Restored to nil rather than to whatever it was: OutOrStdout() answers
+	// os.Stdout, and setting that back leaves the command with a writer of its
+	// own, which a root's SetOut can no longer override. The next test that ran
+	// this command through a root would then print its help to the terminal.
 	t.Cleanup(func() {
-		keyboardInfoCmd.SetOut(origOut)
-		keyboardInfoCmd.SetErr(origErr)
+		keyboardInfoCmd.SetOut(nil)
+		keyboardInfoCmd.SetErr(nil)
 	})
 
 	err = keyboardInfoCmd.RunE(keyboardInfoCmd, nil)
@@ -121,3 +124,30 @@ func TestKeyboardInfoPropagatesDiscoveryError(t *testing.T) {
 }
 
 var errStub = errors.New("enumerate: stub failure")
+
+// A command that a test gave a writer of its own keeps it, and a writer set that
+// way outranks the root's. Restoring os.Stdout rather than nil therefore left the
+// shipped `keyboard info` writing to the terminal while a later test believed it
+// was capturing its help, and the failure showed up as noise in an unrelated
+// test's output rather than as a failing one.
+func TestKeyboardInfoHelpGoesWhereTheRootSendsIt(t *testing.T) {
+	// In a subtest, so the restore has happened by the time the help runs: that
+	// ordering is the whole defect, and doing both here would not test it.
+	t.Run("a test gave it a writer", func(t *testing.T) {
+		runRealKeyboardInfo(t)
+	})
+
+	var out bytes.Buffer
+	root := newRootCommand()
+	registerCommands(root)
+	root.SetOut(&out)
+	root.SetErr(&out)
+	root.SetArgs([]string{"keyboard", "info", "--help"})
+
+	if err := root.Execute(); err != nil {
+		t.Fatalf("keyboard info --help returned error: %v", err)
+	}
+	if !strings.Contains(out.String(), "Usage:") {
+		t.Errorf("captured %q, want the help, so the root's writer is in charge", out.String())
+	}
+}

+ 66 - 8
cmd/qmk-rgb-tool/main.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"strings"
 
 	"github.com/spf13/cobra"
 	"netdome.biz/paul/qmk-rgb/internal/device"
@@ -20,10 +21,21 @@ const (
 	groupKeyboard = "keyboard"
 )
 
-var (
-	targetDevice string
-	targetZone   string
-)
+var targetDevice string
+
+// zoneSelection is the rule every command that writes lighting carries in its own
+// help. It is repeated per command rather than left to the root help because a
+// subcommand's --help does not print the root's, and this is the one thing a user
+// has to know before running one of them.
+const zoneSelection = "The zone is a VIA lighting channel: backlight, rgblight, rgb_matrix, audio or\n" +
+	"led_matrix, or the name this keyboard's definition gives it. Several are written\n" +
+	"comma separated and the word all means every channel the keyboard reports."
+
+// writesLighting is the help of a command that writes to the keyboard: what the
+// command does, then the one rule they share.
+func writesLighting(long string) string {
+	return long + "\n\n" + zoneSelection
+}
 
 func newRootCommand() *cobra.Command {
 	cmd := &cobra.Command{
@@ -42,7 +54,6 @@ func newRootCommand() *cobra.Command {
 		&cobra.Group{ID: groupKeyboard, Title: "Keyboard Commands:"},
 	)
 	cmd.PersistentFlags().StringVar(&targetDevice, "device", "", "Keyboard number as printed by 'keyboard info', starting at 1")
-	cmd.PersistentFlags().StringVar(&targetZone, "zone", "", "RGB lighting channel: backlight, rgblight, rgb_matrix, audio or led_matrix, or the name this keyboard's definition gives it")
 	cmd.PersistentFlags().StringVar(&definitionFlag, "definition", "", "VIA definition file to read effect names from, instead of the one in the data directory")
 	cmd.PersistentFlags().BoolVar(&jsonOutput, "json", false, "Print JSON instead of text")
 	return cmd
@@ -140,7 +151,11 @@ func registerCommands(root *cobra.Command) {
 		NewKeyboardDefinitionsCmd(),
 	)
 
-	root.AddCommand(
+	// The zone is a positional argument, not a flag. A flag every help lists told
+	// `keyboard info` and `list` about a channel they have none of, and it was
+	// ignored there rather than refused. A positional argument is spelled only
+	// where it is read, so there is nothing to ignore.
+	lighting := []*cobra.Command{
 		inGroup(NewEnableCmd(), groupLighting),
 		inGroup(NewDisableCmd(), groupLighting),
 		inGroup(NewInfoCmd(), groupLighting),
@@ -148,10 +163,53 @@ func registerCommands(root *cobra.Command) {
 		inGroup(NewBrightnessCmd(), groupLighting),
 		inGroup(NewSpeedCmd(), groupLighting),
 		inGroup(NewColorCmd(), groupLighting),
+	}
+	profiles := []*cobra.Command{
 		inGroup(NewProfileSaveCmd(), groupProfile),
 		inGroup(NewProfileLoadCmd(), groupProfile),
+	}
+	rest := []*cobra.Command{
 		inGroup(NewProfileListCmd(), groupProfile),
 		inGroup(NewProfileDeleteCmd(), groupProfile),
-		inGroup(keyboardCmd, groupKeyboard),
-	)
+		keyboardCmd,
+	}
+
+	commands := make([]*cobra.Command, 0, len(lighting)+len(profiles)+len(rest))
+	commands = append(commands, lighting...)
+	commands = append(commands, profiles...)
+	commands = append(commands, rest...)
+	root.AddCommand(commands...)
+}
+
+// zoneArgs is the argument count of a command that reads a zone, together with
+// the message a user gets when the count is wrong. Cobra's own "accepts 2 arg(s),
+// received 1" does not say what the argument should have been, and a command that
+// writes lighting now fails at its arity rather than at a choice of channel, so
+// the usage line is the only help there is.
+func zoneArgs(min, max int, what string) cobra.PositionalArgs {
+	return func(cmd *cobra.Command, args []string) error {
+		if len(args) < min || len(args) > max {
+			// cmd.Use already starts with the command's own name, so the path is
+			// the name and what follows it, not the name twice.
+			return fmt.Errorf("%s takes %s\nusage: %s%s", cmd.Name(), what,
+				cmd.CommandPath(), strings.TrimPrefix(cmd.Use, cmd.Name()))
+		}
+		return nil
+	}
+}
+
+// withZoneArgs declares a command's zone argument count, its usage line and the
+// completion for it, in one call. A command that names a channel and does not
+// call this is a command the shell knows nothing about, so the three belong
+// together: the count is what makes the zone required, the usage line is where
+// the vocabulary is documented, and the completion is how a user finds the names
+// a board supplies.
+//
+// A command that takes a zone and something else first — `load` takes a profile
+// name — cannot use this, because the zone is not what its first argument is. It
+// declares the count and its own completion instead.
+func withZoneArgs(cmd *cobra.Command, min, max int, what string) *cobra.Command {
+	cmd.Args = zoneArgs(min, max, what)
+	cmd.ValidArgsFunction = completeZoneArgs
+	return cmd
 }

+ 34 - 23
cmd/qmk-rgb-tool/profile.go

@@ -205,10 +205,12 @@ func applyProfileToProfile(proto rgbProtocol, channels []via.Channel, display ma
 	return nil
 }
 
-// loadProfileFromDevice reads RGB state from device and saves it. Warnings go
-// to warn, which is the command's stderr.
+// 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.
 func loadProfileFromDevice(name string, warn io.Writer) error {
-	proto, target, channels, err := openTarget()
+	proto, target, channels, err := openTarget("")
 	if err != nil {
 		return err
 	}
@@ -239,40 +241,44 @@ func loadProfileFromDevice(name string, warn io.Writer) error {
 }
 
 func NewProfileSaveCmd() *cobra.Command {
-	var name string
-	cmd := &cobra.Command{
+	return &cobra.Command{
 		Use:   "save [name]",
 		Short: "Save current RGB state to a profile",
-		Long:  "Read the current RGB settings from the keyboard and save them as a JSON profile in the profiles/ directory.",
-		Args:  cobra.MaximumNArgs(1),
+		Long: "Read the current RGB settings from every channel of the keyboard and save\n" +
+			"them as a JSON profile in the profiles/ directory.\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" +
+			"zone the profile had nothing to say about.",
+		Args: cobra.MaximumNArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if len(args) == 0 {
-				name = "default"
-			} else {
+			name := "default"
+			if len(args) > 0 {
 				name = args[0]
 			}
 			return loadProfileFromDevice(name, cmd.ErrOrStderr())
 		},
 	}
-	return cmd
 }
 
 func NewProfileLoadCmd() *cobra.Command {
-	var name string
+	// Not withZoneArgs: the profile name comes first and the zone second, so
+	// the zone completion only applies once the name has been typed.
 	cmd := &cobra.Command{
-		ValidArgsFunction: completeProfileNames,
-		Use:               "load [name]",
+		ValidArgsFunction: completeLoadArgs,
+		Use:               "load <name> [zone]",
 		Short:             "Load a profile and apply it to the keyboard",
-		Long:              "Read a JSON profile from the profiles/ directory and apply the saved RGB settings to the keyboard.",
-		Args:              cobra.MaximumNArgs(1),
+		Long: "Read a JSON profile from the profiles/ directory and apply the saved RGB\n" +
+			"settings to the keyboard. Without a zone the profile is applied to every\n" +
+			"channel it names.",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if len(args) == 0 {
-				name = "default"
-			} else {
-				name = args[0]
+			name := args[0]
+			zone := ""
+			if len(args) == 2 {
+				zone = args[1]
 			}
 
-			proto, target, selected, err := openTarget()
+			proto, target, selected, err := openTarget(zone)
 			if err != nil {
 				return err
 			}
@@ -358,11 +364,15 @@ func NewProfileLoadCmd() *cobra.Command {
 
 				for _, ch := range keyChannels {
 					// A key is applied only where the selection allows it, so
-					// `--zone logo load <name>` leaves the other channels alone.
+					// `load <name> logo` leaves the other channels alone.
 					if !selectedSet[ch] {
 						continue
 					}
-					targets, _, err := intrgb.ResolveEffect(catalog, settings.Effect, []via.Channel{ch}, targetZone != "")
+					// A zone the caller named is an explicit request for it, so an
+					// effect the channel does not have is said rather than skipped;
+					// without one the whole profile is being applied and a key the
+					// board cannot do is worth a warning instead of a refusal.
+					targets, _, err := intrgb.ResolveEffect(catalog, settings.Effect, []via.Channel{ch}, zone != "")
 					if err != nil {
 						fmt.Fprintf(cmd.ErrOrStderr(),
 							"Warning: effect %q not found on %s, skipping\n", settings.Effect, channelName(ch, target.Display))
@@ -409,6 +419,7 @@ func NewProfileLoadCmd() *cobra.Command {
 			return nil
 		},
 	}
+	cmd.Args = zoneArgs(1, 2, "a profile name and at most a zone")
 	return cmd
 }
 

+ 1 - 1
cmd/qmk-rgb-tool/profile_load_test.go

@@ -7,7 +7,7 @@ import (
 	intrgb "netdome.biz/paul/qmk-rgb/internal/rgb"
 )
 
-// README.md: "With `--zone`, commands target exactly the selected channel."
+// README.md: "Without a zone the profile is applied to every channel it names."
 // A profile key names a channel, so a load applies only the keys that resolve to
 // a selected channel. The keys themselves are resolved by resolveZoneName, which
 // channels_test.go covers; what matters here is that a key is never applied to a

+ 19 - 11
cmd/qmk-rgb-tool/rgb.go

@@ -29,15 +29,17 @@ type targetDeviceData struct {
 	// Alternatives are the names a channel also answers to besides the one its
 	// definition gives it, so a name the user already knows keeps working.
 	Alternatives map[uint16][]string
-	// Requested is nil when no --zone was given, which means every channel
-	// the keyboard has.
+	// Requested is nil when the zone was "all" or the command named none, which
+	// both mean every channel the keyboard has. It is never nil because a
+	// command forgot to ask: the ones that write refuse an unnamed zone at their
+	// own arity, before anything is opened.
 	Requested []via.Channel
 }
 
 // prepareTarget resolves the keyboard, its display names and the requested
-// channels. Enumeration does not open a HID handle, so an unusable --zone value
-// is still rejected before the device is opened.
-var prepareTarget = func() (targetDeviceData, error) {
+// channels. Enumeration does not open a HID handle, so an unusable zone is still
+// rejected before the device is opened.
+var prepareTarget = func(zone string) (targetDeviceData, error) {
 	devices, err := discoverAll()
 	if err != nil {
 		return targetDeviceData{}, fmt.Errorf("discover: %w", err)
@@ -53,7 +55,7 @@ var prepareTarget = func() (targetDeviceData, error) {
 	// follows from the channel number and is all there is.
 	display, alternatives := applyDefinitionLabels(dev.VendorID, dev.ProductID)
 
-	requested, err := resolveZoneName(targetZone, display, alternatives)
+	requested, err := resolveZoneName(zone, display, alternatives)
 	if err != nil {
 		return targetDeviceData{}, err
 	}
@@ -346,8 +348,8 @@ func enableLightingOnChannels(proto zoneProtocol, channels []via.Channel, catalo
 // openTarget opens the keyboard and resolves the requested channels against the
 // ones it actually has. It is a seam because a command needs all three: the
 // handle it writes to, the names it reports with, and the channels it may touch.
-var openTarget = func() (rgbProtocol, targetDeviceData, []via.Channel, error) {
-	target, err := prepareTarget()
+var openTarget = func(zone string) (rgbProtocol, targetDeviceData, []via.Channel, error) {
+	target, err := prepareTarget(zone)
 	if err != nil {
 		return nil, targetDeviceData{}, nil, err
 	}
@@ -366,7 +368,10 @@ var openTarget = func() (rgbProtocol, targetDeviceData, []via.Channel, error) {
 }
 
 // resolveChannels intersects the requested channels with the detected ones, and
-// refuses a name that resolves to a channel this keyboard does not have.
+// refuses a name that resolves to a channel this keyboard does not have. A
+// selection of several channels is refused as a whole when one of them is
+// missing: a command that quietly wrote two of the three channels it was asked
+// for would report a success it did not deliver.
 func resolveChannels(proto rgbProtocol, target targetDeviceData) ([]via.Channel, error) {
 	present, err := proto.DetectChannels()
 	if err != nil {
@@ -390,13 +395,16 @@ func resolveChannels(proto rgbProtocol, target targetDeviceData) ([]via.Channel,
 	}
 
 	var found []via.Channel
+	var missing []string
 	for _, ch := range target.Requested {
 		if presentSet[ch] {
 			found = append(found, ch)
+		} else {
+			missing = append(missing, channelName(ch, target.Display))
 		}
 	}
-	if len(found) == 0 {
-		return nil, fmt.Errorf("channel %s is not present on this keyboard", target.Requested[0].Subsystem())
+	if len(missing) > 0 {
+		return nil, fmt.Errorf("this keyboard has no %s channel", strings.Join(missing, " or "))
 	}
 	return found, nil
 }

+ 53 - 19
cmd/qmk-rgb-tool/rgb_test.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"bytes"
 	"reflect"
 	"strings"
 	"testing"
@@ -102,24 +103,60 @@ func TestResolveChannelsReportsWhenNoChannelIsPresent(t *testing.T) {
 	}
 }
 
-func TestZoneFlagIsInheritedByCommands(t *testing.T) {
-	stubOneImpact80(t)
-	ran := false
-	cmd := newRootCommand()
-	cmd.AddCommand(&cobra.Command{
-		Use: "probe",
-		Run: func(*cobra.Command, []string) { ran = true },
-	})
-	cmd.SetArgs([]string{"--zone", "backlight", "probe"})
+// The zone is a positional argument, so it is spelled only where a command reads
+// one. A flag the root carries is a flag every help lists, which told `keyboard
+// info` about a channel it cannot address and then ignored it.
+func TestNoCommandCarriesAZoneFlag(t *testing.T) {
+	root := newRootCommand()
+	registerCommands(root)
+
+	var found []string
+	var walk func(*cobra.Command)
+	walk = func(cmd *cobra.Command) {
+		for _, name := range []string{"zone", "all"} {
+			if cmd.Flags().Lookup(name) != nil || cmd.PersistentFlags().Lookup(name) != nil {
+				found = append(found, cmd.CommandPath()+" --"+name)
+			}
+		}
+		for _, sub := range cmd.Commands() {
+			walk(sub)
+		}
+	}
+	walk(root)
 
-	if err := cmd.Execute(); err != nil {
-		t.Fatalf("Execute() error = %v", err)
+	if len(found) > 0 {
+		t.Errorf("zone flags still registered: %v; the zone is a positional argument", found)
 	}
-	if !ran {
-		t.Fatal("child command did not run")
+}
+
+// A command that names no channel must not accept one, and one that does must
+// show it in its usage line, which is where a positional argument is documented.
+func TestZoneIsSpelledOnlyWhereACommandReadsOne(t *testing.T) {
+	usageOf := func(args ...string) string {
+		var out bytes.Buffer
+		cmd := newRootCommand()
+		registerCommands(cmd)
+		cmd.SetOut(&out)
+		cmd.SetErr(&out)
+		cmd.SetArgs(args)
+		if err := cmd.Execute(); err != nil {
+			t.Fatalf("%v --help returned error: %v", args, err)
+		}
+		return out.String()
+	}
+
+	for _, args := range [][]string{{"effect", "--help"}, {"brightness", "--help"}, {"load", "--help"}} {
+		if !strings.Contains(usageOf(args...), "zone") {
+			t.Errorf("%v --help does not show a zone argument", args)
+		}
 	}
-	if targetZone != "backlight" {
-		t.Errorf("targetZone = %q, want %q", targetZone, "backlight")
+
+	// list, delete and the keyboard commands take a name or nothing, and a zone
+	// in their usage line would promise a channel they never read.
+	for _, args := range [][]string{{"list", "--help"}, {"keyboard", "info", "--help"}} {
+		if strings.Contains(usageOf(args...), "<zone>") {
+			t.Errorf("%v --help shows a zone argument, want none", args)
+		}
 	}
 }
 
@@ -128,11 +165,8 @@ func TestZoneFlagIsInheritedByCommands(t *testing.T) {
 // the board's own names.
 func TestNoKeyboardIsReportedBeforeAnUnknownZone(t *testing.T) {
 	stubDiscovery(t, []intdevice.Device{})
-	originalZone := targetZone
-	t.Cleanup(func() { targetZone = originalZone })
-	targetZone = "matx"
 
-	_, _, _, err := openTarget()
+	_, _, _, err := openTarget("matx")
 	if err == nil {
 		t.Fatal("openTarget() expected an error, got nil")
 	}

+ 8 - 9
cmd/qmk-rgb-tool/speed.go

@@ -7,19 +7,18 @@ import (
 )
 
 func NewSpeedCmd() *cobra.Command {
-	return &cobra.Command{
-		Use:   "speed <val>",
-		Short: "Set effect speed",
-		Long: "Set the effect speed (0-255) and read it back, so a value the keyboard\n" +
-			"rescales is reported instead of silently applied.",
-		Args: cobra.ExactArgs(1),
+	return withZoneArgs(&cobra.Command{
+		Use:   "speed <zone> <val>",
+		Short: "Set effect speed on a zone",
+		Long: writesLighting("Set the effect speed (0-255) and read it back, so a value the keyboard\n" +
+			"rescales is reported instead of silently applied."),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			val, err := ParseUint8(args[0])
+			val, err := ParseUint8(args[1])
 			if err != nil {
 				return err
 			}
 
-			proto, target, channels, err := openTarget()
+			proto, target, channels, err := openTarget(args[0])
 			if err != nil {
 				return err
 			}
@@ -38,5 +37,5 @@ func NewSpeedCmd() *cobra.Command {
 			fmt.Fprintf(cmd.OutOrStdout(), "Speed set to %d\n", val)
 			return nil
 		},
-	}
+	}, 2, 2, "a zone and a speed value")
 }

+ 6 - 10
cmd/qmk-rgb-tool/speed_verify_test.go

@@ -9,21 +9,17 @@ import (
 	"netdome.biz/paul/qmk-rgb/internal/via"
 )
 
-func runSpeed(t *testing.T, applied map[via.Channel]uint8, zoneFlag string, arg string) (stdout, stderr string) {
+func runSpeed(t *testing.T, applied map[via.Channel]uint8, zone string, arg string) (stdout, stderr string) {
 	t.Helper()
 
 	proto := &verifyingProtocol{applied: applied}
-
-	originalZone := targetZone
-	t.Cleanup(func() { targetZone = originalZone })
-	targetZone = zoneFlag
-	t.Cleanup(stubOpenTarget(t, proto, impact80Display(), impact80Channels(), 0x36B0, 0x309F))
+	t.Cleanup(impact80Target(t, proto))
 
 	var out, errOut bytes.Buffer
 	cmd := NewSpeedCmd()
 	cmd.SetOut(&out)
 	cmd.SetErr(&errOut)
-	cmd.SetArgs([]string{arg})
+	cmd.SetArgs([]string{zone, arg})
 
 	if err := cmd.Execute(); err != nil {
 		t.Fatalf("speed returned error: %v", err)
@@ -39,7 +35,7 @@ func TestSpeedReportsPlainMessageWhenAllZonesMatch(t *testing.T) {
 		via.ChannelRgblight:  60,
 		via.ChannelRgbMatrix: 60,
 		via.ChannelAudio:     60,
-	}, "", "60")
+	}, "all", "60")
 
 	if strings.TrimSpace(stdout) != "Speed set to 60" {
 		t.Errorf("stdout = %q, want the exact success message", stdout)
@@ -54,7 +50,7 @@ func TestSpeedSummarisesAppliedValuesOnMismatch(t *testing.T) {
 		via.ChannelRgblight:  4,
 		via.ChannelRgbMatrix: 60,
 		via.ChannelAudio:     4,
-	}, "", "60")
+	}, "all", "60")
 
 	for _, want := range []string{"logo 4", "backlight 60", "side 4", "requested 60"} {
 		if !strings.Contains(stdout, want) {
@@ -88,7 +84,7 @@ func TestSpeedZeroIsReportedAsApplied(t *testing.T) {
 		via.ChannelRgblight:  0,
 		via.ChannelRgbMatrix: 0,
 		via.ChannelAudio:     0,
-	}, "", "0")
+	}, "all", "0")
 
 	if strings.TrimSpace(stdout) != "Speed set to 0" {
 		t.Errorf("stdout = %q, want the success message for a value the keyboard kept", stdout)

+ 4 - 2
cmd/qmk-rgb-tool/target_seam_test.go

@@ -10,16 +10,18 @@ import (
 
 // stubOpenTarget points the commands at one protocol, the display names a board
 // would supply and the channels that board has. It returns the restore function.
+// The zone the command read is the one it resolves, so a test that passes a
+// multi-channel zone gets the same channels the real seam would resolve.
 func stubOpenTarget(t *testing.T, proto rgbProtocol, display map[uint16]string, channels []via.Channel, vendorID, productID uint16) func() {
 	t.Helper()
 
 	original := openTarget
-	openTarget = func() (rgbProtocol, targetDeviceData, []via.Channel, error) {
+	openTarget = func(zone string) (rgbProtocol, targetDeviceData, []via.Channel, error) {
 		target := targetDeviceData{
 			Device:  intdevice.Device{VendorID: vendorID, ProductID: productID},
 			Display: display,
 		}
-		requested, err := resolveZoneName(targetZone, display, nil)
+		requested, err := resolveZoneName(zone, display, nil)
 		if err != nil {
 			return nil, target, nil, err
 		}

+ 304 - 0
cmd/qmk-rgb-tool/zone_selection_test.go

@@ -0,0 +1,304 @@
+package main
+
+import (
+	"bytes"
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/spf13/cobra"
+	"netdome.biz/paul/qmk-rgb/internal/via"
+)
+
+// A command that writes lighting and named no zone used to write every channel
+// the keyboard has, which is the widest thing it can do and not something anybody
+// asked for. Its argument count refuses it now, before the keyboard is opened, so
+// a script gets the same answer whichever of them it called.
+func TestLightingCommandsRefuseToWriteWithoutAZone(t *testing.T) {
+	cases := []struct {
+		name string
+		cmd  func() *cobra.Command
+		args []string
+	}{
+		{"effect", NewEffectCmd, nil},
+		{"brightness", NewBrightnessCmd, []string{"160"}},
+		{"speed", NewSpeedCmd, []string{"2"}},
+		{"color", NewColorCmd, []string{"00ff00"}},
+		{"enable", NewEnableCmd, nil},
+		{"disable", NewDisableCmd, nil},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			proto := &fakeZoneProtocol{failAt: -1}
+			t.Cleanup(impact80Target(t, proto))
+
+			var out bytes.Buffer
+			cmd := tc.cmd()
+			cmd.SetOut(&out)
+			cmd.SetErr(&out)
+			cmd.SilenceErrors = true
+			cmd.SilenceUsage = true
+			cmd.SetArgs(tc.args)
+
+			err := cmd.Execute()
+			if err == nil {
+				t.Fatal("Execute() expected the missing zone to be refused, got nil")
+			}
+			// The message has to say which argument was missing, or the user
+			// cannot act on it without reading the source.
+			if !strings.Contains(err.Error(), "zone") {
+				t.Errorf("error = %q, want it to name the missing zone", err)
+			}
+			if !strings.Contains(err.Error(), "usage:") {
+				t.Errorf("error = %q, want the usage line so the order of the arguments is visible", err)
+			}
+			if len(proto.reports) != 0 {
+				t.Errorf("reports = %v, want no write before the zone was named", proto.reports)
+			}
+			if out.Len() != 0 {
+				t.Errorf("output = %q, want nothing written before the zone was named", out.String())
+			}
+		})
+	}
+}
+
+// A list of zones reaches exactly the channels it names, and no others.
+func TestZoneListWritesEveryNamedChannel(t *testing.T) {
+	cases := []struct {
+		zone string
+		want []commandReport
+	}{
+		{"logo", []commandReport{{channel: 2, param: 1, value: 160}}},
+		{"side,logo", []commandReport{
+			{channel: 2, param: 1, value: 160},
+			{channel: 4, param: 1, value: 160},
+		}},
+		// The order written is not the order applied: channels come back in
+		// channel order, so a script sees one order for every spelling.
+		{"logo, side", []commandReport{
+			{channel: 2, param: 1, value: 160},
+			{channel: 4, param: 1, value: 160},
+		}},
+		// One name twice is one channel, not a double write.
+		{"logo,logo", []commandReport{{channel: 2, param: 1, value: 160}}},
+		// A display name and the subsystem name of the same channel are one
+		// channel, and so is a name that matches through both.
+		{"rgblight", []commandReport{{channel: 2, param: 1, value: 160}}},
+		{"Backlight", []commandReport{{channel: 3, param: 1, value: 160}}},
+		{"backlight", []commandReport{{channel: 3, param: 1, value: 160}}},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.zone, func(t *testing.T) {
+			proto := &fakeZoneProtocol{failAt: -1}
+			t.Cleanup(impact80Target(t, proto))
+
+			var out bytes.Buffer
+			cmd := NewBrightnessCmd()
+			cmd.SetOut(&out)
+			cmd.SetErr(&out)
+			cmd.SetArgs([]string{tc.zone, "160"})
+
+			if err := cmd.Execute(); err != nil {
+				t.Fatalf("brightness %s 160 returned error: %v", tc.zone, err)
+			}
+			if !reflect.DeepEqual(proto.reports, tc.want) {
+				t.Errorf("reports = %v, want %v", proto.reports, tc.want)
+			}
+		})
+	}
+}
+
+// all is every channel, and naming it next to one more channel is the same
+// request: the union is the whole keyboard either way.
+func TestAllMeansEveryChannel(t *testing.T) {
+	cases := []string{"all", "ALL", "side,all", "all,logo", " backlight , all "}
+
+	for _, zone := range cases {
+		t.Run(zone, func(t *testing.T) {
+			proto := &fakeZoneProtocol{failAt: -1}
+			t.Cleanup(impact80Target(t, proto))
+
+			var out bytes.Buffer
+			cmd := NewBrightnessCmd()
+			cmd.SetOut(&out)
+			cmd.SetErr(&out)
+			cmd.SetArgs([]string{zone, "160"})
+
+			if err := cmd.Execute(); err != nil {
+				t.Fatalf("brightness %q 160 returned error: %v", zone, err)
+			}
+			want := []commandReport{
+				{channel: 2, param: 1, value: 160},
+				{channel: 3, param: 1, value: 160},
+				{channel: 4, param: 1, value: 160},
+			}
+			if !reflect.DeepEqual(proto.reports, want) {
+				t.Errorf("reports = %v, want %v", proto.reports, want)
+			}
+		})
+	}
+}
+
+// A zone the keyboard does not have is refused, and where a list names several,
+// every one that is missing is named: writing two of the three channels asked for
+// and reporting a success is the failure this prevents.
+func TestZoneListRefusesAChannelTheKeyboardLacks(t *testing.T) {
+	cases := []struct {
+		zone string
+		want []string
+	}{
+		{"led_matrix", []string{"led_matrix"}},
+		{"logo,led_matrix", []string{"led_matrix"}},
+		{"logo,led_matrix,backlight", []string{"led_matrix"}},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.zone, func(t *testing.T) {
+			proto := &fakeZoneProtocol{failAt: -1}
+			t.Cleanup(impact80Target(t, proto))
+
+			var out bytes.Buffer
+			cmd := NewBrightnessCmd()
+			cmd.SetOut(&out)
+			cmd.SetErr(&out)
+			cmd.SetArgs([]string{tc.zone, "160"})
+
+			err := cmd.Execute()
+			if err == nil {
+				t.Fatalf("brightness %s 160 = nil error, want the missing channel refused", tc.zone)
+			}
+			for _, want := range tc.want {
+				if !strings.Contains(err.Error(), want) {
+					t.Errorf("error = %q, want it to name %q", err, want)
+				}
+			}
+			if len(proto.reports) != 0 {
+				t.Errorf("reports = %v, want nothing written when a named channel is absent", proto.reports)
+			}
+		})
+	}
+}
+
+// A name nobody wrote has to say what the alternatives are, and a list that names
+// one of them has to say which of the list it was.
+func TestUnknownZoneNamesTheVocabulary(t *testing.T) {
+	cases := []struct {
+		zone string
+		want []string
+	}{
+		{"nonsense", []string{"nonsense", "rgb_matrix", "all"}},
+		{"logo,nonsense", []string{"nonsense", `in "logo,nonsense"`}},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.zone, func(t *testing.T) {
+			proto := &fakeZoneProtocol{failAt: -1}
+			t.Cleanup(impact80Target(t, proto))
+
+			var out bytes.Buffer
+			cmd := NewBrightnessCmd()
+			cmd.SetOut(&out)
+			cmd.SetErr(&out)
+			cmd.SetArgs([]string{tc.zone, "160"})
+
+			err := cmd.Execute()
+			if err == nil {
+				t.Fatalf("brightness %s 160 = nil error, want the unknown zone refused", tc.zone)
+			}
+			for _, want := range tc.want {
+				if !strings.Contains(err.Error(), want) {
+					t.Errorf("error = %q, want it to contain %q", err, want)
+				}
+			}
+		})
+	}
+}
+
+// A trailing or doubled comma names no channel, and guessing which one was meant
+// is worse than saying so.
+func TestZoneListRefusesAnEmptyName(t *testing.T) {
+	for _, zone := range []string{"logo,", ",logo", "logo,,side", "logo, "} {
+		t.Run(zone, func(t *testing.T) {
+			proto := &fakeZoneProtocol{failAt: -1}
+			t.Cleanup(impact80Target(t, proto))
+
+			var out bytes.Buffer
+			cmd := NewBrightnessCmd()
+			cmd.SetOut(&out)
+			cmd.SetErr(&out)
+			cmd.SetArgs([]string{zone, "160"})
+
+			err := cmd.Execute()
+			if err == nil {
+				t.Fatalf("brightness %q 160 = nil error, want the empty name refused", zone)
+			}
+			if !strings.Contains(err.Error(), "empty") {
+				t.Errorf("error = %q, want it to say the list has an empty name", err)
+			}
+			if len(proto.reports) != 0 {
+				t.Errorf("reports = %v, want no write", proto.reports)
+			}
+		})
+	}
+}
+
+// A zone that a definition file gives to two channels of the same board would be
+// a silent retarget, so the list resolves it to neither.
+func TestZoneListRefusesANameTwoChannelsShare(t *testing.T) {
+	proto := &fakeZoneProtocol{failAt: -1}
+	display := map[uint16]string{2: "logo", 3: "logo", 4: "side"}
+	t.Cleanup(stubOpenTarget(t, proto, display, impact80Channels(), 0x36B0, 0x309F))
+
+	var out bytes.Buffer
+	cmd := NewBrightnessCmd()
+	cmd.SetOut(&out)
+	cmd.SetErr(&out)
+	cmd.SetArgs([]string{"logo", "160"})
+
+	err := cmd.Execute()
+	if err == nil {
+		t.Fatal("Execute() = nil error, want the ambiguous name refused")
+	}
+	if !strings.Contains(err.Error(), "several channels") {
+		t.Errorf("error = %q, want it to say the name is ambiguous", err)
+	}
+	if len(proto.reports) != 0 {
+		t.Errorf("reports = %v, want no write", proto.reports)
+	}
+}
+
+// The value is parsed before the keyboard is opened, so a typo in it costs
+// nothing and reports where the mistake is.
+func TestReversedArgumentsFailOnTheValue(t *testing.T) {
+	proto := &fakeZoneProtocol{failAt: -1}
+	opened := false
+	original := openTarget
+	t.Cleanup(func() { openTarget = original })
+	stub := openTarget
+	openTarget = func(zone string) (rgbProtocol, targetDeviceData, []via.Channel, error) {
+		opened = true
+		return stub(zone)
+	}
+
+	var out bytes.Buffer
+	cmd := NewBrightnessCmd()
+	cmd.SetOut(&out)
+	cmd.SetErr(&out)
+	cmd.SetArgs([]string{"160", "logo"})
+
+	err := cmd.Execute()
+	if err == nil {
+		t.Fatal("Execute() = nil error, want the reversed arguments refused")
+	}
+	if !strings.Contains(err.Error(), "invalid value") {
+		t.Errorf("error = %q, want it to say the value could not be read", err)
+	}
+	if opened {
+		t.Error("the keyboard was opened, want the value rejected first")
+	}
+	if len(proto.reports) != 0 {
+		t.Errorf("reports = %v, want no write", proto.reports)
+	}
+}

+ 67 - 3
cmd/qmk-rgb-tool/zones.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
@@ -8,17 +9,68 @@ import (
 	"netdome.biz/paul/qmk-rgb/internal/via"
 )
 
-// resolveZoneName maps a --zone value to the channels it may mean, without
+// zoneAll is the zone name that means every channel the keyboard reports.
+const zoneAll = "all"
+
+// resolveZoneName maps a zone argument to the channels it may mean, without
 // opening the keyboard. A display name of the connected board wins over a
 // subsystem name, and a value that would mean two channels is refused rather
 // than resolved to one of them.
 //
-// An empty name means every channel and is reported as a nil slice.
+// The argument is one name or several, comma separated, and the channels come
+// back in channel order however they were written, so a caller sees the same
+// order for every spelling of the same selection.
+//
+// An empty argument means every channel and is reported as a nil slice, which is
+// how a command that only reads reports the whole keyboard.
 func resolveZoneName(name string, display map[uint16]string, alternatives map[uint16][]string) ([]via.Channel, error) {
 	if name == "" {
 		return nil, nil
 	}
 
+	parts := strings.Split(name, ",")
+	var channels []via.Channel
+	for _, part := range parts {
+		part = strings.TrimSpace(part)
+		if part == "" {
+			return nil, fmt.Errorf("zone %q names an empty channel: write several channels comma separated, like side,logo", name)
+		}
+		// all means every channel, so naming it next to one more channel is the
+		// same request as naming it alone: the union is the whole keyboard.
+		if strings.EqualFold(part, zoneAll) {
+			return nil, nil
+		}
+
+		found, err := resolveOneZoneName(part, display, alternatives)
+		if errors.Is(err, errNotAZoneName) {
+			return nil, unknownZoneError(part, name)
+		}
+		if err != nil {
+			return nil, err
+		}
+		channels = append(channels, found...)
+	}
+	return sortChannels(dedupeChannels(channels)), nil
+}
+
+// errNotAZoneName says the argument is not a name this board or this tool knows.
+// The caller turns it into the message, because the message has to quote the
+// whole list when there is one: "unknown zone" in a list of four is otherwise a
+// claim about a word the user never wrote on its own.
+var errNotAZoneName = errors.New("not a zone name")
+
+// unknownZoneError names the part that could not be placed, the list it came
+// from, and the two vocabularies that would have placed it.
+func unknownZoneError(part, whole string) error {
+	quoted := fmt.Sprintf("unknown zone %q", part)
+	if part != whole {
+		quoted += fmt.Sprintf(" in %q", whole)
+	}
+	return fmt.Errorf("%s: use a channel name such as rgb_matrix, the name this keyboard's definition gives it, or %s for all of them", quoted, zoneAll)
+}
+
+// resolveOneZoneName maps a single channel name to the channels it may mean.
+func resolveOneZoneName(name string, display map[uint16]string, alternatives map[uint16][]string) ([]via.Channel, error) {
 	if channels := dedupeChannels(append(displayNameChannels(display, name), alternateNameChannels(alternatives, name)...)); len(channels) > 1 {
 		return nil, fmt.Errorf("zone %q matches several channels of this keyboard; its definition gives the same name to more than one", name)
 	} else if len(channels) == 1 {
@@ -31,7 +83,7 @@ func resolveZoneName(name string, display map[uint16]string, alternatives map[ui
 		}
 	}
 
-	return nil, fmt.Errorf("unknown zone %q: use a channel name such as rgb_matrix, or the name this keyboard's definition gives it", name)
+	return nil, errNotAZoneName
 }
 
 // displayNameChannels returns every channel the board names displayName. The
@@ -103,6 +155,18 @@ func channelName(c via.Channel, display map[uint16]string) string {
 	return c.Subsystem()
 }
 
+// sortChannels puts channels in channel order, which is the order the tool
+// reports them in everywhere else. A list of names is written in whatever order
+// suits the user; the channels it resolves to still come back in channel order,
+// so a script sees one order for every spelling of the same selection.
+func sortChannels(channels []via.Channel) []via.Channel {
+	if len(channels) < 2 {
+		return channels
+	}
+	sort.Slice(channels, func(i, j int) bool { return channels[i] < channels[j] })
+	return channels
+}
+
 // dedupeChannels drops repeats, keeping the order. A name can reach the same
 // channel twice — once as the definition's label and once as the subsystem name
 // it replaces — and that is one channel, not a conflict.

+ 2 - 2
internal/rgb/catalog.go

@@ -41,7 +41,7 @@ type Catalog struct {
 }
 
 // NewCatalog returns a catalog for a board described by effect entries, as a VIA
-// definition file provides them. The board name is what `effect --list` reports;
+// definition file provides them. The board name is what the effect list reports;
 // the entries may leave gaps, because a definition names only the effects a board's
 // firmware implements.
 //
@@ -81,7 +81,7 @@ var defaultAliases = map[via.Channel]map[string]string{
 	},
 }
 
-// Name returns the catalog's board name, which `effect --list` reports.
+// Name returns the catalog's board name, which the effect list reports.
 func (c *Catalog) Name() string {
 	if c == nil {
 		return ""

+ 1 - 1
internal/rgb/catalog_test.go

@@ -183,7 +183,7 @@ func TestResolveEffectAcceptsTheStaticAliasOnEveryPath(t *testing.T) {
 
 // Whether a command may skip an unsupported effect is the caller's knowledge,
 // not something the channel count can tell it. A board with a single lighting
-// channel and no --zone must still behave like a default command.
+// channel and no zone named must still behave like a default command.
 func TestResolveEffectDistinguishesExplicitFromDefault(t *testing.T) {
 	catalog := impact80(t)
 	both := []via.Channel{via.ChannelRgblight, via.ChannelRgbMatrix}