소스 검색

spell the zone in the effect ID form, and pin the examples to the arity

Six documented invocations predated the zone becoming the first argument, so
each either failed at the argument count or was read as a channel named after
the value: `brightness 160`, `color <hex>`, `speed 0`, `effect 46`, `effect
none` and `effect rainbow`. Five of the six were in prose rather than in a
code block, which is why reading the examples by eye had missed them.

The error messages were worse than the docs. Three of them pointed at
`effect <index>`, advice that does not run: the zone is required, so the index
alone is read as a channel name. enable.go already said `effect <zone> <index>`
while catalog.go and profile.go said otherwise, and three tests pinned the
broken string. The usage line is the reference now, and the README's reference
table is pulled to it, where the name is optional and the brackets say so.

`off is accepted on every channel` was false. defaultAliases covers rgblight,
rgb_matrix and audio only, so a keyboard with a backlight or led_matrix channel
is refused an alias by name. This board has neither, which is why the gap was
invisible from here.

Two tests keep it that way. One pins the table to the line the command prints
and keeps the bare form out of every message. The other reads both shapes a
reader may copy, the fenced blocks and the inline spans, and checks each one
against the arity the commands enforce and against the zone that has to come
first. Flags are read off the command tree rather than guessed, which is what
made an earlier version of it read `--json effect all` as the command `all`.

Also: a dead "see the table above" in the skill, a duplicated example, the
missing definitions/ package, an over-broad claim about where an argument count
is declared, and the vendor's own [0,160] brightness ranges beside the measured
ones.
Paul-Dieter Klumpp 1 주 전
부모
커밋
db1611a246

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

@@ -85,9 +85,12 @@ qmk-rgb-tool keyboard definitions            # every definition in use, from the
 `fetch` needs the board to be in VIA's collection. Many are not, including the
 Impact 80. Then the vendor's own definition is the source: find the board's
 support or driver page, download the VIA JSON it links, and put it in the
-per-user `definitions/` directory (see the table above) or pass it with
+per-user `definitions/` directory — `~/.config/qmk-rgb-tool/definitions` on
+Linux, `~/Library/Application Support/qmk-rgb-tool/definitions` on macOS,
+`%AppData%\qmk-rgb-tool\definitions` on Windows — or pass it with
 `--definition <path>`. A file is used for its own board only; one for another
-board is refused by name.
+board is refused by name. This is not the repository's own `definitions/`, which
+is read at build time and built into the binary.
 
 Never write an effect name into this repo that no source states. Either the
 vendor's file names it, or the user has looked at the board and named what they
@@ -159,11 +162,17 @@ user that the tool's own output did not confirm.
 - `rainbow` → varies by zone (resolves automatically)
 - `solid` → varies by zone (resolves automatically)
 
-Aliases are channel-aware. `qmk-rgb-tool effect rainbow` means different effects
-on the `rgblight` channel than on `rgb_matrix`. They also bridge a definition
-file's spelling: where a vendor writes `breathe` or `fixed wave`, the tool's
-`breathing` and `fixed_wave` still resolve to the same effect, and the output
-shows the spelling of the source in use.
+Aliases are channel-aware, and the zone comes first because it is the command's
+first argument: `qmk-rgb-tool effect logo rainbow` and
+`qmk-rgb-tool effect backlight rainbow` select different effects. They also
+bridge a definition file's spelling: where a vendor writes `breathe` or
+`fixed wave`, the tool's `breathing` and `fixed_wave` still resolve to the same
+effect, and the output shows the spelling of the source in use.
+
+The alias table covers `rgblight`, `rgb_matrix` and `audio` only. On a keyboard
+with a `backlight` or `led_matrix` channel an alias is refused by name rather
+than resolved, so do not promise one there; the Impact 80 has neither, which is
+why the gap does not show up on it.
 
 ## Known limitations
 

+ 16 - 12
AGENTS.md

@@ -85,9 +85,13 @@ The zone is a **positional argument**, not a flag, and that is not a style choic
 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.
+ignore. `zoneArgs` in `cmd/qmk-rgb-tool/main.go` is the one place a *zone*
+argument count is declared — `withZoneArgs` is every command's route into it, and
+`load` calls it directly because its zone is the second argument. Commands that
+take no zone use cobra's own `NoArgs` or `MaximumNArgs`, which is correct: a
+stray token there is a mistyped command, not a missing channel. `zoneArgs` 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.
@@ -102,12 +106,12 @@ 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.
+count is what keeps it that way: `brightness` given one argument 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
@@ -172,9 +176,9 @@ Assume the next parameter needs its own wrapper, and share only the read.
 
 A board can refuse an ID that looks valid. The Impact 80's `logo` and `side`
 channels read effect ID 0 as "lighting off" and leave the mode register alone,
-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
+so `effect logo 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
   up with. Read

+ 41 - 21
README.md

@@ -155,12 +155,11 @@ into the prompt, which is worse than an empty list.
 ./qmk-rgb-tool effect backlight 17
 ./qmk-rgb-tool enable all
 ./qmk-rgb-tool disable logo
-./qmk-rgb-tool info
 
-# Reading needs no target: without a name, an effect or an info reads everything
+# Reading needs no target, and a read may still name one to narrow itself
+./qmk-rgb-tool info
 ./qmk-rgb-tool effect all
 ./qmk-rgb-tool effect logo
-./qmk-rgb-tool info
 
 # Save and load RGB profiles
 ./qmk-rgb-tool save paul
@@ -259,7 +258,7 @@ keyboard to learn which channels to list.
 
 ```bash
 ./qmk-rgb-tool keyboard info
-./qmk-rgb-tool --device 2 brightness 160
+./qmk-rgb-tool --device 2 brightness logo 160
 ```
 
 Without `--device`, commands that open a keyboard run only when exactly one is
@@ -365,13 +364,14 @@ stays `unknown` and is set by number. Its behaviour is measured and worth
 knowing: set after a moving effect, it leaves the LEDs on the pattern they are
 currently showing and stops the animation — `cycle_left_right` frozen still looks
 like a standing rainbow, because that is what it froze. It is also not the same
-as `speed 0`, which leaves the effect selected and only takes the rate to zero:
-the two differ in the registers, one on the effect and one on the speed.
+as `speed backlight 0`, which leaves the effect selected and only takes the rate
+to zero: the two differ in the registers, one on the effect and one on the speed.
 
 Wobkey's own
 [VIA definition JSON](https://drive.wobkey.com/f/d/6BtO/Impact_80.JSON) stops at
 45 and contains no word for pause, stop, freeze or hold, so VIA cannot set this
-ID from its dropdown either. A raw `effect 46` is the only way to reach it.
+ID from its dropdown either. A raw `effect backlight 46` is the only way to reach
+it.
 
 A keyboard without a catalog is still driven: `brightness`, `speed`, `color`,
 an effect index and `info` all work, because none of them needs a name. Four
@@ -408,7 +408,7 @@ the board does not have anywhere is reported as unknown:
 $ qmk-rgb-tool effect logo rainbow_moving_chevron
 Error: effect rainbow_moving_chevron is not supported on rgblight
 $ qmk-rgb-tool effect rgb_matrix nonsense
-Error: unknown effect: nonsense
+Error: unknown effect: nonsense (an effect ID from 0 to 255 is `effect <zone> <index>`)
 ```
 
 ### Where Effect Names Can Come From
@@ -513,6 +513,12 @@ those two channels is `[0, 4]`, not 0–255:
 The freeze at `0` was observed on `logo`; `side` reports the same range and the
 same collapse to 4. `backlight` is the only channel with a usable 0–255 range.
 
+The definition file's own ranges are a separate thing from what the firmware
+accepts, and the two disagree about brightness. It offers `[0, 160]` for
+brightness on all three channels, so VIA's sliders stop at 160 everywhere, while
+the firmware scales `backlight` past that to 255. The table above is what the
+keyboard does, measured; the ranges in the file are what VIA's UI offers.
+
 The command exits 0 either way: a value the firmware cannot represent is not a
 failure, but the summary line always states the value that was actually applied.
 
@@ -601,7 +607,7 @@ Effects fall into categories that behave differently:
 - **Gradient** effects (`gradient_up_down`, `gradient_left_right`) create a color gradient across the keyboard. They do not honor `color`.
 - **Alphas mods** (`alphas_mods`) colors modifier keys differently from alphanumeric keys. The colors are built-in and cannot be changed.
 
-To set a permanent color, use `solid_color` and then `color <hex>`.
+To set a permanent color, use `solid_color` and then `color <zone> <hex>`.
 
 The `logo` and `side` catalogs hold only seven effects each, and the color you
 set is shown by three of them: `fixed_wave`, `breathing` and `light`. `wave`,
@@ -616,10 +622,18 @@ for Logo/Side and `rainbow_moving_chevron` for Backlight, `rainbow_wave` →
 for Backlight. The legacy `static` alias remains accepted as a compatibility
 alias for `solid`.
 
-`off` is accepted on every channel, but it cannot be set by effect ID on `logo`
-and `side`: the firmware reads ID 0 there as "lighting off" and leaves the mode
-register where it was, so the effect that was running keeps running. `disable` is
-the command that turns a channel off, because it also writes brightness 0.
+Those aliases are the three channels the Impact 80 names — `rgblight`,
+`rgb_matrix` and `audio`. `defaultAliases` in `internal/rgb/catalog.go` is the
+one place they are listed, and it has no entry for `backlight` or `led_matrix`,
+so on a keyboard that has one of those channels an alias is refused by name
+(`effect off is not supported on backlight`) rather than resolved. This board
+has neither, which is why the difference is not visible here.
+
+`off` resolves on Logo, Backlight and Side, but it cannot be set by effect ID on
+`logo` and `side`: the firmware reads ID 0 there as "lighting off" and leaves the
+mode register where it was, so the effect that was running keeps running.
+`disable` is the command that turns a channel off, because it also writes
+brightness 0.
 
 The aliases belong to the tool, not to a board, so they work for every catalog —
 including one read from a definition file. A name resolves to itself, to the name
@@ -734,12 +748,13 @@ 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 `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,
-`built-in` for one compiled into the binary:
+definition writes `Backlight`, these three reach the same channel:
+`brightness backlight 160`, `brightness Backlight 160` and
+`brightness rgb_matrix 160`. The label is the name, and 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, `built-in` for one compiled into the binary:
 
 ```
 Definitions
@@ -793,7 +808,7 @@ board", and that file is gone, so the field would have been unanswerable:
 | `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> [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 |
@@ -815,7 +830,11 @@ board", and that file is gone, so the field would have been unanswerable:
 
 `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.
+name or a raw effect ID: two arguments set it, one lists that zone's effects, so
+its form is `effect <zone> [name|index]`. A message that points at the ID form
+spells out both arguments — `effect <zone> <index>` — because the zone is
+required, and an index written in the zone's place is read as a channel named
+after it.
 `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
@@ -868,6 +887,7 @@ files instead.
 
 ```
 cmd/qmk-rgb-tool/  # Cobra-based CLI
+definitions/       # VIA definition files built into the binary with go:embed
 internal/device/  # HID discovery
 internal/hid/     # Cross-platform HID access (hidapi)
 internal/rgb/     # Effects, colors, state

+ 5 - 0
cmd/qmk-rgb-tool/channels_test.go

@@ -475,6 +475,11 @@ func TestSaveWarnsThatEffectNamesCannotBeRecorded(t *testing.T) {
 	if !strings.Contains(stderr.String(), "no effect names") {
 		t.Errorf("stderr = %q, want the missing names named", stderr.String())
 	}
+	// The way out it names has to run. `effect` takes the zone as its first
+	// argument, so a bare `effect <index>` would be read as a channel name.
+	if !strings.Contains(stderr.String(), "effect <zone> <index>") {
+		t.Errorf("stderr = %q, want the way out to name the zone as well as the ID", stderr.String())
+	}
 }
 
 // A channel's own definition label and the subsystem name it replaces can both

+ 259 - 0
cmd/qmk-rgb-tool/doc_examples_test.go

@@ -0,0 +1,259 @@
+package main
+
+import (
+	"os"
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
+)
+
+// Every example in the docs is a command a reader may type. Six of them were not:
+// `brightness 160`, `color <hex>`, `speed 0`, `effect 46`, `effect none` and
+// `effect rainbow` all predate the zone becoming the first argument, and each
+// either fails at the argument count or is read as a channel named after the
+// value. A broken example in a reference is worse than no example, so the
+// documented command lines are checked against the arity the commands enforce.
+//
+// Both shapes a reader may copy are read: the lines of a fenced bash block, and
+// the inline code spans in prose. Five of the six were in prose, so reading only
+// the blocks would have missed them.
+func TestDocumentedInvocationsMatchTheCommandArity(t *testing.T) {
+	// The argument counts each command accepts, as withZoneArgs and zoneArgs
+	// declare them. A command missing here is one that names no channel, so its
+	// argument count is not this test's business.
+	arity := map[string][2]int{
+		"enable":     {1, 1},
+		"disable":    {1, 1},
+		"effect":     {1, 2},
+		"brightness": {2, 2},
+		"speed":      {2, 2},
+		"color":      {2, 2},
+		"info":       {0, 1},
+	}
+
+	takesValue := flagsThatTakeAValue(newRootCommand())
+	seen := 0
+
+	for _, path := range []string{
+		"../../README.md",
+		"../../AGENTS.md",
+		"../../.claude/skills/qmk-rgb/SKILL.md",
+	} {
+		data, err := os.ReadFile(path)
+		if err != nil {
+			t.Errorf("read %s: %v", path, err)
+			continue
+		}
+		for _, line := range documentedCommandLines(string(data)) {
+			command, args := splitInvocation(line.text, takesValue)
+			want, known := arity[command]
+			// A span naming the command alone is prose about it, and a quoted error
+			// message carries the command without being one.
+			if !known || len(args) == 0 || isQuotedMessage(args) {
+				continue
+			}
+			seen++
+
+			if got := len(args); got < want[0] || got > want[1] {
+				t.Errorf("%s:%d passes %s to `%s`, which takes %s: %q",
+					path, line.no, plural(got), command, describeArity(want), line.text)
+				continue
+			}
+			// Every one of these commands takes its zone first. Whether the first
+			// argument is a placeholder or a real value, it has to be a channel:
+			// `color <hex>` and `effect 46` both write the value where the zone goes,
+			// and the command reads it as a channel named after it. A completion
+			// example is the third thing entirely: what follows the command is a
+			// keypress, not an argument.
+			if isKeypress(args) {
+				continue
+			}
+			if !namesTheZone(args[0]) {
+				t.Errorf("%s:%d writes %q where `%s` takes its zone: %q",
+					path, line.no, args[0], command, line.text)
+			}
+		}
+	}
+
+	// A checker that reads nothing proves nothing, and the fence handling is the
+	// part most able to match everything by accident.
+	if seen < 10 {
+		t.Errorf("checked %d documented command lines, want enough to be worth having", seen)
+	}
+}
+
+// commandLine is one documented command line, with a prompt, a path prefix and a
+// trailing comment already stripped.
+type commandLine struct {
+	text string
+	no   int
+}
+
+// documentedCommandLines returns every line the docs offer as something to run:
+// the command lines of a fenced bash block, and the inline code spans of prose.
+// Neither a `$` prompt nor a leading `./` is part of what the command is given, so
+// both go.
+func documentedCommandLines(body string) []commandLine {
+	var out []commandLine
+	inFence := false
+	for i, raw := range strings.Split(body, "\n") {
+		line := strings.TrimSpace(raw)
+		if strings.HasPrefix(line, "```") {
+			inFence = strings.Contains(line, "bash")
+			continue
+		}
+		if inFence {
+			if idx := strings.Index(line, "#"); idx >= 0 {
+				line = strings.TrimSpace(line[:idx])
+			}
+			if stripped := stripPrompt(line); stripped != "" {
+				out = append(out, commandLine{text: stripped, no: i + 1})
+			}
+			continue
+		}
+		for _, span := range inlineCodeSpans(raw) {
+			if stripped := stripPrompt(span); stripped != "" {
+				out = append(out, commandLine{text: stripped, no: i + 1})
+			}
+		}
+	}
+	return out
+}
+
+// stripPrompt removes a `$ ` prompt and a leading `./`, and leaves the rest alone.
+// The binary's own name is left in place, because whether a line carries it says
+// something: a block usually does and a prose span usually does not, and matching
+// the command is what the caller does either way.
+func stripPrompt(line string) string {
+	line = strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(line), "$ "))
+	line = strings.TrimSpace(strings.TrimPrefix(line, "./"))
+	return strings.TrimSpace(line)
+}
+
+// inlineCodeSpans returns the backtick-quoted spans of a prose line, which is
+// where a documented command lives when it is not in a block. A double backtick
+// span is skipped, because that is how a span containing a backtick is written
+// and it is not an invocation.
+func inlineCodeSpans(line string) []string {
+	var out []string
+	parts := strings.Split(line, "``")
+	if len(parts) > 1 {
+		parts = parts[:1]
+	}
+	parts = strings.Split(parts[0], "`")
+	for i := 1; i < len(parts); i += 2 {
+		if span := strings.TrimSpace(parts[i]); span != "" {
+			out = append(out, span)
+		}
+	}
+	return out
+}
+
+// flagsThatTakeAValue reads the persistent flags off the real command tree, so
+// whether a flag swallows the word after it is answered by the flag's own
+// declaration rather than by a list written here. Guessing is what made an earlier
+// version of this file read `--json effect all` as the command `all`.
+func flagsThatTakeAValue(root *cobra.Command) map[string]bool {
+	out := map[string]bool{}
+	root.PersistentFlags().VisitAll(func(f *pflag.Flag) {
+		if f.NoOptDefVal == "" {
+			out["--"+f.Name] = true
+		}
+	})
+	return out
+}
+
+// splitInvocation returns the command a line invokes and the positional arguments
+// it is given. The binary's own name goes first, because a line may carry a flag
+// before the command: `--device 2 brightness logo 160`. A flag that carries a
+// value consumes that word, and it is the flag that has to be the first word after
+// the command, so `info --json` leaves `info` with no arguments. A redirection
+// ends the arguments, so `completion bash > file` gives `completion` one.
+func splitInvocation(line string, takesValue map[string]bool) (string, []string) {
+	args := strings.Fields(line)
+	if len(args) > 0 && args[0] == "qmk-rgb-tool" {
+		args = args[1:]
+	}
+	for len(args) > 0 && isFlag(args[0]) {
+		name, _, inline := strings.Cut(args[0], "=")
+		args = args[1:]
+		if !inline && takesValue[name] && len(args) > 0 {
+			args = args[1:]
+		}
+	}
+	if len(args) == 0 {
+		return "", nil
+	}
+
+	command := args[0]
+	args = args[1:]
+	// A flag may also follow the command, and a flag is not an argument.
+	for len(args) > 0 && isFlag(args[len(args)-1]) {
+		args = args[:len(args)-1]
+	}
+	for i, a := range args {
+		if strings.HasPrefix(a, ">") {
+			args = args[:i]
+			break
+		}
+	}
+	return command, args
+}
+
+func isFlag(arg string) bool { return len(arg) > 1 && strings.HasPrefix(arg, "-") }
+
+// isKeypress reports whether a documented line ends in `<TAB>`, which is how the
+// docs write a shell completion example. The command is named, and what follows it
+// is a key the reader presses rather than an argument they pass, so there is no
+// arity to check.
+func isKeypress(args []string) bool {
+	return strings.Contains(strings.ToLower(strings.Join(args, " ")), "<tab")
+}
+
+// namesTheZone reports whether an argument is written as a zone. A usage form may
+// spell it `<zone>`, or `[zone]` where the command reads without one. Otherwise it
+// is a channel name: a QMK subsystem name, the word `all`, a comma separated list
+// of them, or a board's own name from its definition file — of which the docs
+// name only the Impact 80's, since that is the board the file in the repository
+// describes.
+//
+// This is a list of names and not a resolver, and it is the reason a zone the docs
+// do not know cannot be checked here: the check is here to catch a value written
+// where a name belongs, not to enumerate every name a board may carry.
+func namesTheZone(arg string) bool {
+	// The brackets that mark a placeholder say nothing about what it is.
+	stripped := strings.NewReplacer("<", "", ">", "", "[", "", "]", "").Replace(arg)
+	lowered := strings.ToLower(stripped)
+	for _, zone := range []string{"zone", "all", "backlight", "rgblight", "rgb_matrix", "audio", "led_matrix", "logo", "side"} {
+		if strings.Contains(lowered, zone) {
+			return true
+		}
+	}
+	return false
+}
+
+// isQuotedMessage reports whether a span is an error message the docs quote rather
+// than a command someone would run. The messages this tool emits are English, so
+// they carry words a command line cannot: `effect off is not supported on
+// backlight` names the command without being an invocation of it.
+func isQuotedMessage(args []string) bool {
+	joined := strings.Join(args, " ")
+	return strings.Contains(joined, " is ") || strings.Contains(joined, " not ")
+}
+
+func describeArity(want [2]int) string {
+	if want[0] == want[1] {
+		return "exactly " + strconv.Itoa(want[0])
+	}
+	return strconv.Itoa(want[0]) + " to " + strconv.Itoa(want[1])
+}
+
+func plural(n int) string {
+	if n == 1 {
+		return "1 argument"
+	}
+	return strconv.Itoa(n) + " arguments"
+}

+ 7 - 2
cmd/qmk-rgb-tool/effect_id_test.go

@@ -96,7 +96,9 @@ func TestEffectStillTakesAName(t *testing.T) {
 }
 
 // An unknown name is still an unknown name, and the message points at the ID form
-// rather than at a command that no longer exists.
+// rather than at a command that no longer exists. The ID form it points at is
+// `effect <zone> <index>`: the zone is the first argument and is required, so the
+// advice has to be an invocation that actually runs.
 func TestUnknownNameErrorPointsAtTheIDForm(t *testing.T) {
 	proto := &fakeZoneProtocol{failAt: -1}
 	t.Cleanup(vendoredDefinitions(t))
@@ -108,9 +110,12 @@ func TestUnknownNameErrorPointsAtTheIDForm(t *testing.T) {
 	if strings.Contains(err.Error(), "`mode") {
 		t.Errorf("error = %q, want no reference to a removed command", err)
 	}
-	if !strings.Contains(err.Error(), "effect <index>") {
+	if !strings.Contains(err.Error(), "effect <zone> <index>") {
 		t.Errorf("error = %q, want it to point at the ID form", err)
 	}
+	if strings.Contains(err.Error(), "`effect <index>`") {
+		t.Errorf("error = %q, want the zone spelled out: a bare `effect <index>` is read as a channel name", err)
+	}
 }
 
 // The mode command is gone: an ID goes through the command that also takes names.

+ 46 - 0
cmd/qmk-rgb-tool/flags_test.go

@@ -95,6 +95,52 @@ func TestDocsDoNotTeachAZoneNameTheResolverRejects(t *testing.T) {
 	}
 }
 
+// A usage line is the one place a command's arity is declared, and the docs
+// repeat it. The two drifted here: the table spelled `effect <zone> <name|index>`
+// with the name required, while the command reads with one argument and writes
+// with two. Pin the table to the line the command actually prints, and keep every
+// message that offers the ID form from dropping the zone — `effect 17` alone is
+// read as a channel named `17`, so advice in that form does not run.
+func TestDocsSpellTheEffectUsageAsTheCommandDoes(t *testing.T) {
+	used := NewEffectCmd().Use
+	if !strings.Contains(used, "[name|index]") {
+		t.Fatalf("effect Use = %q, want it to mark the name optional", used)
+	}
+
+	want := "qmk-rgb-tool " + used
+	readme, err := os.ReadFile("../../README.md")
+	if err != nil {
+		t.Fatalf("read README.md: %v", err)
+	}
+	// The reference table lives in a markdown table, where a pipe inside a cell is
+	// escaped, so the line carries a backslash the usage string does not. Compare
+	// against the unescaped form rather than making the table write a broken cell.
+	if !strings.Contains(strings.ReplaceAll(string(readme), `\|`, "|"), want) {
+		t.Errorf("README.md does not carry the usage line %q", want)
+	}
+
+	// The bare form is the defect, so look for exactly that and not for the
+	// substring it shares with the correct one.
+	for _, path := range []string{
+		"../../README.md",
+		"../../AGENTS.md",
+		"../../.claude/skills/qmk-rgb/SKILL.md",
+		"effect.go", "enable.go", "profile.go", "../../internal/rgb/catalog.go",
+	} {
+		data, err := os.ReadFile(path)
+		if err != nil {
+			t.Errorf("read %s: %v", path, err)
+			continue
+		}
+		for i, line := range strings.Split(string(data), "\n") {
+			if strings.Contains(line, "effect <index>") {
+				t.Errorf("%s:%d points at `effect <index>`, which reads the ID as a zone: %q",
+					path, i+1, strings.TrimSpace(line))
+			}
+		}
+	}
+}
+
 // 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

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

@@ -223,10 +223,12 @@ func loadProfileFromDevice(name string, warn io.Writer) error {
 	if catalog == nil {
 		// The profile stores effect names and a board without a catalog has none
 		// to store, so every channel is written as "unknown" and cannot be
-		// restored. Say so here, where the user can still act on it.
+		// restored. Say so here, where the user can still act on it. The way out
+		// names the zone, because the command takes it as its first argument and
+		// an index on its own would be read as a channel name.
 		fmt.Fprintf(warn,
 			"Warning: this keyboard has no effect names, so the profile records effect %q and cannot restore it; "+
-				"run `keyboard fetch` for its VIA definition, or set an effect with `effect <index>`\n", "unknown")
+				"run `keyboard fetch` for its VIA definition, or set an effect with `effect <zone> <index>`\n", "unknown")
 	}
 
 	p := &Profile{

+ 1 - 1
go.mod

@@ -4,11 +4,11 @@ go 1.26.6
 
 require (
 	github.com/spf13/cobra v1.10.2
+	github.com/spf13/pflag v1.0.9
 	github.com/sstallion/go-hid v0.15.0
 )
 
 require (
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
-	github.com/spf13/pflag v1.0.9 // indirect
 	golang.org/x/sys v0.8.0 // indirect
 )

+ 5 - 3
internal/rgb/catalog.go

@@ -206,7 +206,7 @@ func ResolveEffect(catalog *Catalog, name string, channels []via.Channel, explic
 		// number, and the names are one command away. A user who reads only the
 		// error should not have to find either out elsewhere.
 		return nil, nil, fmt.Errorf("no effect names for this keyboard: run `keyboard fetch` for its VIA definition, " +
-			"or set an effect by number with `effect <index>`")
+			"or set an effect by number with `effect <zone> <index>`")
 	}
 
 	// The compatibility spelling every catalog shares, so a caller cannot resolve
@@ -227,8 +227,10 @@ func ResolveEffect(catalog *Catalog, name string, channels []via.Channel, explic
 	}
 	if !knownAnywhere {
 		// A name nobody wrote is either a typo or a number that belongs in the
-		// other form, and pointing at it saves the user from guessing.
-		return nil, nil, fmt.Errorf("unknown effect: %s (an effect ID from 0 to 255 is `effect <index>`)", name)
+		// other form, and pointing at it saves the user from guessing. The ID form
+		// is `effect <zone> <index>`: the zone is the command's first argument and
+		// is required, so an index on its own is read as a channel name.
+		return nil, nil, fmt.Errorf("unknown effect: %s (an effect ID from 0 to 255 is `effect <zone> <index>`)", name)
 	}
 
 	var targets []EffectTarget

+ 7 - 1
internal/rgb/catalog_test.go

@@ -117,14 +117,20 @@ func TestCatalogDefaultEffectPerChannel(t *testing.T) {
 	}
 }
 
+// The advice the error carries has to be an invocation that runs. The zone is
+// `effect`'s first argument and is required, so `effect <index>` on its own is
+// read as a channel name rather than as an effect ID.
 func TestResolveEffectWithoutACatalogRefuses(t *testing.T) {
 	_, _, err := ResolveEffect(nil, "wave", []via.Channel{via.ChannelRgblight}, false)
 	if err == nil {
 		t.Fatal("ResolveEffect(nil, ...) expected an error, got nil")
 	}
-	if !strings.Contains(err.Error(), "effect <index>") {
+	if !strings.Contains(err.Error(), "effect <zone> <index>") {
 		t.Errorf("error = %q, want it to point at the ID form", err)
 	}
+	if strings.Contains(err.Error(), "`effect <index>`") {
+		t.Errorf("error = %q, want the zone spelled out: a bare `effect <index>` is read as a channel name", err)
+	}
 }
 
 // ID 5 is light on channel 2 and rainbow_beacon on channel 3, so the same name

+ 1 - 1
internal/rgb/definition_test.go

@@ -373,7 +373,7 @@ func TestMissingCatalogErrorNamesBothWaysOut(t *testing.T) {
 	if err == nil {
 		t.Fatal("ResolveEffect(nil catalog) = nil error, want one")
 	}
-	for _, want := range []string{"`keyboard fetch`", "`effect <index>`"} {
+	for _, want := range []string{"`keyboard fetch`", "`effect <zone> <index>`"} {
 		if !strings.Contains(err.Error(), want) {
 			t.Errorf("error = %q, want it to mention %q", err, want)
 		}