store a user's effect names on their own, merged over the definition

A VIA definition file belongs to the manufacturer. Writing one meant
authoring a menu description for someone else's keyboard and handing
the user a nested JSON array to type a name into: to set one name you
had to find menüs[0].content[1].options[7][0] in a document the
manufacturer wrote. The names are the part that is missing and the part
that is anyone's to supply, so they are stored apart from the
definition, flat:

	"channels": {"rgb_matrix": {"7": "rainbow_moving_chevron"}}

A names file alone makes a catalog, which is the case it exists for: a
board with no definition is exactly the board someone writes names for,
because the file the manufacturer would have published is the one
missing, not the names. A name there replaces the vendor's for that one
effect ID and leaves every other entry alone, so overriding one name
does not mean restating the ones that were right. An ID the definition
does not have is added rather than refused, since a definition may stop
short of the board's highest and a user naming that one is saying
something the file did not.

Each name now carries where it came from. A name is the one value this
tool cannot read back off a keyboard to check, so the distinction
between "the vendor published this" and "a person typed this" has to be
visible wherever a name is printed, and Effect carries a Source for it.

ChannelFromSubsystem lives beside LightingChannels so the vocabulary
has one reading of it rather than a second table, and aliases are stored
as written because the catalog looks them up by exact string.

internal/rgb/catalog.go

@@ -22,6 +22,12 @@ const unknownEffectName = "unknown"
 type Effect struct {
 	ID   uint8
 	Name string
+	// Source says whether the name is the manufacturer's or the user's. It is
+	// carried per effect because one board's names can come from both, and a name
+	// is the one value this tool cannot read back off a keyboard to check: an
+	// empty source means the definition file the vendor published said this, and
+	// SourceUser means a person typed it.
+	Source NameSource
 }
 
 // Catalog is one board's effect names, per channel. The keyboard holds numbers,

internal/rgb/names.go

@@ -0,0 +1,265 @@
+package rgb
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"sort"
+	"strconv"
+	"strings"
+
+	"netdome.biz/paul/qmk-rgb/internal/via"
+)
+
+// NameSource says where an effect name came from. It is on the effect rather
+// than on the file because one board's names can come from both: a definition
+// the manufacturer wrote and a name the user wrote over the top of one of its
+// entries. The tool's rule is that it never reports a value it has not verified,
+// and a name is the one value it cannot read back off a keyboard, so which of
+// the two a name is has to be visible wherever the name is.
+type NameSource string
+
+const (
+	// SourceVendor is a name from the board's own definition file. It is what the
+	// manufacturer wrote, so it is a fact about the board.
+	SourceVendor NameSource = ""
+	// SourceUser is a name from the per-user names file. It is what the user
+	// called the effect, which is a fact about them and not about the board.
+	SourceUser NameSource = "you"
+)
+
+// namesDocument is the file as it is stored. The channel is keyed by its QMK
+// subsystem name and the effect by its number, both as strings because that is
+// what a JSON object key is, and both are read back through the one vocabulary in
+// internal/via rather than a second table here.
+type namesDocument struct {
+	Name      string                       `json:"name,omitempty"`
+	VendorID  string                       `json:"vendorId"`
+	ProductID string                       `json:"productId"`
+	Channels  map[string]map[string]string `json:"channels"`
+	Aliases   map[string]map[string]string `json:"aliases,omitempty"`
+}
+
+// Names is one board's effect names a user wrote, or overrode.
+//
+// It exists because a VIA definition file belongs to the manufacturer. The tool
+// used to write one of its own, which meant authoring a menu description for
+// someone else's keyboard and handing the user a nested JSON array to type a
+// name into. The names are the part that is missing and the part that is
+// anyone's to supply, so they are stored on their own and merged over whatever
+// the definition says.
+type Names struct {
+	Path      string
+	Name      string
+	VendorID  uint16
+	ProductID uint16
+	// effects and aliases are keyed by channel, then by effect ID or by spelling.
+	effects map[via.Channel]map[uint8]string
+	aliases map[via.Channel]map[string]string
+}
+
+// LoadNamesFile reads one names file. A file that is not a names file is an
+// error rather than nothing: the user put it there, and a silent "no names" would
+// read as the keyboard having none.
+func LoadNamesFile(path string) (*Names, error) {
+	raw, err := os.ReadFile(path)
+	if err != nil {
+		return nil, err
+	}
+	var doc namesDocument
+	if err := json.Unmarshal(raw, &doc); err != nil {
+		return nil, fmt.Errorf("parse %s: %w", path, err)
+	}
+	vendorID, productID, err := namesIdentifiers(doc.VendorID, doc.ProductID)
+	if err != nil {
+		return nil, fmt.Errorf("parse %s: %w", path, err)
+	}
+	names := &Names{
+		Path:      path,
+		Name:      doc.Name,
+		VendorID:  vendorID,
+		ProductID: productID,
+		effects:   map[via.Channel]map[uint8]string{},
+		aliases:   map[via.Channel]map[string]string{},
+	}
+	for key, byID := range doc.Channels {
+		ch, ok := via.ChannelFromSubsystem(key)
+		if !ok {
+			return nil, fmt.Errorf("parse %s: %q is not a QMK lighting channel", path, key)
+		}
+		for rawID, name := range byID {
+			id, err := parseEffectID(rawID)
+			if err != nil {
+				return nil, fmt.Errorf("parse %s: %s channel %q: %w", path, key, rawID, err)
+			}
+			if names.effects[ch] == nil {
+				names.effects[ch] = map[uint8]string{}
+			}
+			names.effects[ch][id] = name
+		}
+	}
+	for key, byName := range doc.Aliases {
+		ch, ok := via.ChannelFromSubsystem(key)
+		if !ok {
+			return nil, fmt.Errorf("parse %s: %q is not a QMK lighting channel", path, key)
+		}
+		if names.aliases[ch] == nil {
+			names.aliases[ch] = map[string]string{}
+		}
+		for spelling, name := range byName {
+			names.aliases[ch][spelling] = name
+		}
+	}
+	return names, nil
+}
+
+// Matches reports whether these names are the ones for a board.
+func (n *Names) Matches(vendorID, productID uint16) bool {
+	return n.VendorID == vendorID && n.ProductID == productID
+}
+
+// Apply returns a catalog with these names over the top of the ones it holds.
+//
+// It works on a nil catalog, because a board with no definition file is exactly
+// the board a user writes names for: the file the manufacturer would have
+// published is the one that is missing, not the names. A name here replaces the
+// vendor's for that one effect ID and leaves every other entry alone, so
+// overriding one name does not mean restating the ones that were right.
+func (n *Names) Apply(base *Catalog) *Catalog {
+	merged := make(map[via.Channel][]Effect)
+	var aliases = map[via.Channel]map[string]string{}
+	board := ""
+	// A nil catalog is the normal case here, not a degenerate one: it is what a
+	// board with no definition file gives, and that board is the one a user writes
+	// names for.
+	if base != nil {
+		board = base.board
+		for ch, effects := range base.names {
+			merged[ch] = append([]Effect(nil), effects...)
+		}
+		for ch, byName := range base.aliases {
+			copied := make(map[string]string, len(byName))
+			for k, v := range byName {
+				copied[k] = v
+			}
+			aliases[ch] = copied
+		}
+	}
+	// An effect the definition does not have is added, because the definition may
+	// stop short of the board's highest ID and a user naming that one is telling
+	// us something the file did not.
+	for ch, byID := range n.effects {
+		for id, name := range byID {
+			merged[ch] = upsertEffect(merged[ch], Effect{ID: id, Name: name, Source: SourceUser})
+		}
+	}
+	for ch, byName := range n.aliases {
+		if aliases[ch] == nil {
+			aliases[ch] = map[string]string{}
+		}
+		for spelling, name := range byName {
+			// Stored as written. An alias is looked up by the exact string the
+			// catalog already uses, so normalising it here would make a spelling
+			// resolve that the definition's own aliases do not.
+			aliases[ch][spelling] = name
+		}
+	}
+	if n.Name != "" {
+		board = n.Name
+	}
+	out := &Catalog{board: board, names: merged, aliases: aliases}
+	for ch := range merged {
+		sort.Slice(out.names[ch], func(i, j int) bool { return out.names[ch][i].ID < out.names[ch][j].ID })
+	}
+	return out
+}
+
+// upsertEffect replaces the entry for an ID or adds one, keeping the list sorted.
+func upsertEffect(effects []Effect, want Effect) []Effect {
+	for i := range effects {
+		if effects[i].ID == want.ID {
+			effects[i] = want
+			return effects
+		}
+	}
+	effects = append(effects, want)
+	sort.Slice(effects, func(i, j int) bool { return effects[i].ID < effects[j].ID })
+	return effects
+}
+
+// Channels returns the lighting channels this file names, in channel order, so a
+// command that reports on the file can walk it the way it walks a board.
+func (n *Names) Channels() []via.Channel {
+	var out []via.Channel
+	for _, ch := range via.LightingChannels {
+		if len(n.effects[ch]) > 0 || len(n.aliases[ch]) > 0 {
+			out = append(out, ch)
+		}
+	}
+	return out
+}
+
+// Effects returns the names on one channel, in ID order.
+func (n *Names) Effects(ch via.Channel) []Effect {
+	ids := make([]int, 0, len(n.effects[ch]))
+	for id := range n.effects[ch] {
+		ids = append(ids, int(id))
+	}
+	sort.Ints(ids)
+	out := make([]Effect, 0, len(ids))
+	for _, id := range ids {
+		out = append(out, Effect{ID: uint8(id), Name: n.effects[ch][uint8(id)], Source: SourceUser})
+	}
+	return out
+}
+
+// parseEffectID reads an effect ID from a names file, where it is a JSON object
+// key and therefore a string. A plain number is what a person writes, and 0x is
+// accepted because both spellings appear in the tool's own output.
+func parseEffectID(raw string) (uint8, error) {
+	var n uint64
+	var err error
+	if base, value, found := strings.Cut(raw, "0x"); found {
+		_ = base
+		n, err = strconv.ParseUint(value, 16, 16)
+	} else {
+		n, err = strconv.ParseUint(raw, 10, 16)
+	}
+	if err != nil {
+		return 0, fmt.Errorf("%q is not an effect ID", raw)
+	}
+	return uint8(n), nil
+}
+
+// namesIdentifiers returns the board a names file is for. It is the same pair of
+// spellings a definition file carries, read through the same hex parser, so a
+// names file and a definition file for one board are recognisably the same board.
+func namesIdentifiers(vendorID, productID string) (uint16, uint16, error) {
+	if vendorID == "" || productID == "" {
+		return 0, 0, fmt.Errorf("neither vendorId nor productId; not a names file")
+	}
+	v, err := parseHexID(vendorID)
+	if err != nil {
+		return 0, 0, fmt.Errorf("vendorId %q: %w", vendorID, err)
+	}
+	p, err := parseHexID(productID)
+	if err != nil {
+		return 0, 0, fmt.Errorf("productId %q: %w", productID, err)
+	}
+	return v, p, nil
+}
+
+// EffectSource says whether the name for an effect ID on a channel is the
+// manufacturer's or the user's, which is what lets a command print where a name
+// came from instead of presenting both as the same kind of fact.
+func (c *Catalog) EffectSource(ch via.Channel, id uint8) NameSource {
+	if c == nil {
+		return SourceVendor
+	}
+	for _, e := range c.names[ch] {
+		if e.ID == id {
+			return e.Source
+		}
+	}
+	return SourceVendor
+}

internal/rgb/names_test.go

@@ -0,0 +1,190 @@
+package rgb
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"netdome.biz/paul/qmk-rgb/internal/via"
+)
+
+func writeNames(t *testing.T, body string) *Names {
+	t.Helper()
+	path := filepath.Join(t.TempDir(), "board.json")
+	if err := os.WriteFile(path, []byte(body), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	names, err := LoadNamesFile(path)
+	if err != nil {
+		t.Fatalf("LoadNamesFile() error = %v", err)
+	}
+	return names
+}
+
+// A name the user wrote is theirs, and the tool has to be able to say so wherever
+// it prints the name: nothing in a file can be read back off a keyboard, so a name
+// is the one value it cannot verify, and a user's name is a fact about them.
+func TestNamesCarryTheirSource(t *testing.T) {
+	names := writeNames(t, `{
+		"vendorId": "0x1234", "productId": "0x5678",
+		"channels": {"rgb_matrix": {"7": "rainbow_moving_chevron"}}
+	}`)
+
+	catalog := names.Apply(nil)
+	if got := catalog.EffectName(via.ChannelRgbMatrix, 7); got != "rainbow_moving_chevron" {
+		t.Errorf("EffectName() = %q, want the name the user wrote", got)
+	}
+	if got := catalog.EffectSource(via.ChannelRgbMatrix, 7); got != SourceUser {
+		t.Errorf("EffectSource() = %q, want %q", got, SourceUser)
+	}
+}
+
+// A board with no definition file is exactly the board a user writes names for, so
+// a names file alone has to produce a catalog. The manufacturer's file is the one
+// that is missing, not the names.
+func TestNamesAloneMakeACatalogForABoardWithNoDefinition(t *testing.T) {
+	names := writeNames(t, `{
+		"vendorId": "0x1234", "productId": "0x5678",
+		"channels": {"rgblight": {"0": "none", "1": "wave"}}
+	}`)
+
+	catalog := names.Apply(nil)
+	effects := catalog.Effects(via.ChannelRgblight)
+	if len(effects) != 2 {
+		t.Fatalf("effects = %v, want two", effects)
+	}
+	if effects[0].ID != 0 || effects[0].Name != "none" {
+		t.Errorf("effects[0] = %v, want ID 0 named none", effects[0])
+	}
+	if effects[1].ID != 1 || effects[1].Name != "wave" {
+		t.Errorf("effects[1] = %v, want ID 1 named wave", effects[1])
+	}
+	// A name the user wrote resolves back to its number, or naming it would be
+	// decoration.
+	id, ok := catalog.EffectID(via.ChannelRgblight, "wave")
+	if !ok || id != 1 {
+		t.Errorf("EffectID(\"wave\") = %d, %t, want 1, true", id, ok)
+	}
+}
+
+// A user's name replaces the vendor's for that one effect and leaves the rest of
+// the file alone: overriding one name must not mean restating the ones that were
+// already right.
+func TestNamesOverrideOneEffectAndLeaveTheRest(t *testing.T) {
+	base := NewCatalog("Test Board", map[via.Channel][]Effect{
+		via.ChannelRgbMatrix: {
+			{ID: 0, Name: "none"},
+			{ID: 1, Name: "solid_color"},
+			{ID: 2, Name: "breathing"},
+		},
+	})
+	names := writeNames(t, `{
+		"vendorId": "0x1234", "productId": "0x5678",
+		"channels": {"rgb_matrix": {"2": "pulsing"}}
+	}`)
+
+	catalog := names.Apply(base)
+	if got := catalog.EffectName(via.ChannelRgbMatrix, 2); got != "pulsing" {
+		t.Errorf("EffectName(2) = %q, want the user's name", got)
+	}
+	if got := catalog.EffectSource(via.ChannelRgbMatrix, 2); got != SourceUser {
+		t.Errorf("EffectSource(2) = %q, want %q", got, SourceUser)
+	}
+	if got := catalog.EffectName(via.ChannelRgbMatrix, 1); got != "solid_color" {
+		t.Errorf("EffectName(1) = %q, want the vendor's name left alone", got)
+	}
+	if got := catalog.EffectSource(via.ChannelRgbMatrix, 1); got != SourceVendor {
+		t.Errorf("EffectSource(1) = %q, want an empty source for the vendor's", got)
+	}
+}
+
+// A definition may stop short of the board's highest ID, and a user naming that
+// one is telling the tool something the file did not. So the entry is added rather
+// than refused.
+func TestNamesAddAnEffectTheDefinitionDoesNotHave(t *testing.T) {
+	base := NewCatalog("Test Board", map[via.Channel][]Effect{
+		via.ChannelRgbMatrix: {{ID: 0, Name: "none"}},
+	})
+	names := writeNames(t, `{
+		"vendorId": "0x1234", "productId": "0x5678",
+		"channels": {"rgb_matrix": {"46": "riverflow"}}
+	}`)
+
+	catalog := names.Apply(base)
+	effects := catalog.Effects(via.ChannelRgbMatrix)
+	if len(effects) != 2 {
+		t.Fatalf("effects = %v, want the definition's and the user's", effects)
+	}
+	if effects[1].ID != 46 || effects[1].Name != "riverflow" {
+		t.Errorf("effects[1] = %v, want ID 46 named riverflow", effects[1])
+	}
+}
+
+// Applying names must not reach back into the definition it was given, or a second
+// lookup for the same board would see the first one's overrides.
+func TestApplyDoesNotMutateTheCatalogItWasGiven(t *testing.T) {
+	base := NewCatalog("Test Board", map[via.Channel][]Effect{
+		via.ChannelRgbMatrix: {{ID: 1, Name: "solid_color"}},
+	})
+	names := writeNames(t, `{
+		"vendorId": "0x1234", "productId": "0x5678",
+		"channels": {"rgb_matrix": {"1": "steady"}}
+	}`)
+
+	names.Apply(base)
+	if got := base.EffectName(via.ChannelRgbMatrix, 1); got != "solid_color" {
+		t.Errorf("the definition's catalog now says %q, want it untouched", got)
+	}
+}
+
+// A file that is not a names file is a user error worth reporting. Answering
+// "no names" instead would read as the keyboard having none.
+func TestLoadNamesFileRejectsAChannelThatIsNotOne(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "board.json")
+	if err := os.WriteFile(path, []byte(`{
+		"vendorId": "0x1234", "productId": "0x5678",
+		"channels": {"keymap": {"1": "x"}}
+	}`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := LoadNamesFile(path); err == nil {
+		t.Error("LoadNamesFile() = nil error, want a channel that is not a lighting channel refused")
+	}
+}
+
+func TestLoadNamesFileRejectsAnEffectIDThatIsNotOne(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "board.json")
+	if err := os.WriteFile(path, []byte(`{
+		"vendorId": "0x1234", "productId": "0x5678",
+		"channels": {"rgb_matrix": {"two": "x"}}
+	}`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := LoadNamesFile(path); err == nil {
+		t.Error("LoadNamesFile() = nil error, want an effect ID that is not one refused")
+	}
+}
+
+// An effect ID is a JSON object key, so it arrives as a string, and both spellings
+// the tool prints elsewhere are worth accepting.
+func TestEffectIDIsReadInEitherSpelling(t *testing.T) {
+	tests := []struct {
+		raw  string
+		want uint8
+	}{
+		{"7", 7},
+		{"0x07", 7},
+		{"46", 46},
+		{"0x2E", 46},
+	}
+	for _, tt := range tests {
+		got, err := parseEffectID(tt.raw)
+		if err != nil {
+			t.Errorf("parseEffectID(%q) error = %v", tt.raw, err)
+			continue
+		}
+		if got != tt.want {
+			t.Errorf("parseEffectID(%q) = %d, want %d", tt.raw, got, tt.want)
+		}
+	}
+}

internal/via/channel.go

@@ -123,3 +123,16 @@ func (p *Protocol) EffectTop(ch Channel) (int, error) {
 	}
 	return int(top[0]), nil
 }
+
+// ChannelFromSubsystem returns the channel a QMK lighting subsystem name stands
+// for, and whether the name is one of them. It is the read side of LightingChannels
+// and lives beside it so that the vocabulary is written down once: a name a file
+// or a command carries is turned into a channel here and nowhere else.
+func ChannelFromSubsystem(name string) (Channel, bool) {
+	for _, ch := range LightingChannels {
+		if ch.Subsystem() == name {
+			return ch, true
+		}
+	}
+	return 0, false
+}