Selaa lähdekoodia

tell a definition that names no effects apart from an unknown name

A board whose file refers to VIA's built-in lighting menu carries no
effect names, because that menu's names are in VIA's own code and not
in the file. `keyboard fetch` succeeds on such a board and stores the
file, and every channel then reports zero effects, so `effect <zone>
<name>` reported an unknown effect and pointed at a typo in a name the
tool holds none of.

The catalog now reports whether it names anything, and ResolveEffect
separates the two ways of having no names: no definition, which
`keyboard fetch` fixes, and a definition that names none, which nothing
the user can run fixes. A name the board does have among the ones it does
name is still an unknown effect.
Paul-Dieter Klumpp 1 viikko sitten
vanhempi
commit
5ab751afab
2 muutettua tiedostoa jossa 76 lisäystä ja 0 poistoa
  1. 21 0
      internal/rgb/catalog.go
  2. 55 0
      internal/rgb/catalog_test.go

+ 21 - 0
internal/rgb/catalog.go

@@ -98,6 +98,19 @@ func (c *Catalog) Effects(ch via.Channel) []Effect {
 	return c.names[ch]
 }
 
+// HasNames reports whether the catalog names any effect at all, on any channel. A
+// definition can be present and name nothing, which is not the same as having no
+// definition: a board whose file refers to VIA's built-in lighting menu by name
+// rather than listing its effects carries no names, because the names for that menu
+// live in VIA's own code. The two situations are told apart in a message, because
+// the fix for one is no use at all for the other.
+func (c *Catalog) HasNames() bool {
+	if c == nil {
+		return false
+	}
+	return len(c.names) > 0
+}
+
 // Names returns the effect names of a channel in effect-ID order, or nil when the
 // catalog says nothing about it.
 func (c *Catalog) Names(ch via.Channel) []string {
@@ -208,6 +221,14 @@ func ResolveEffect(catalog *Catalog, name string, channels []via.Channel, explic
 		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 <zone> <index>`")
 	}
+	if !catalog.HasNames() {
+		// The definition was found and it names nothing, so it is not a missing file
+		// and fetching again changes nothing. Saying "unknown effect" here would
+		// point at a typo in a name the tool holds none of.
+		return nil, nil, fmt.Errorf("the VIA definition for %s names no effects, so no effect name can be resolved; "+
+			"this board's names are not in the file, and an effect is set by number with `effect <zone> <index>`",
+			catalog.Name())
+	}
 
 	// The compatibility spelling every catalog shares, so a caller cannot resolve
 	// "static" differently from another.

+ 55 - 0
internal/rgb/catalog_test.go

@@ -249,3 +249,58 @@ func TestResolveEffectKeepsUnknownEffectForANameTheBoardLacks(t *testing.T) {
 		t.Errorf("error = %q, want it to stay an unknown effect", err)
 	}
 }
+
+// A definition can be found and still name nothing: it refers to VIA's built-in
+// lighting menu, whose names are in VIA's own code and not in the file. The GMMK
+// Pro is one of these. Reporting an unknown effect there points at a typo in a name
+// the tool does not hold a single one of, and fetching again changes nothing.
+func TestResolveEffectSaysTheDefinitionNamesNoEffects(t *testing.T) {
+	catalog := NewCatalog("GMMK Pro", nil)
+
+	_, _, err := ResolveEffect(catalog, "breathing", []via.Channel{via.ChannelRgbMatrix}, true)
+	if err == nil {
+		t.Fatal("ResolveEffect() expected an error, got nil")
+	}
+	if !strings.Contains(err.Error(), "GMMK Pro") {
+		t.Errorf("error = %q, want it to name the board", err)
+	}
+	if strings.Contains(err.Error(), "unknown effect") {
+		t.Errorf("error = %q, want it not to blame the name the user typed", err)
+	}
+	if !strings.Contains(err.Error(), "effect <zone> <index>") {
+		t.Errorf("error = %q, want it to point at the ID form, which is all a board with no names accepts", err)
+	}
+}
+
+// The two ways of having no names are different and the message has to tell them
+// apart: one is fixed by fetching a definition, the other is not fixed by anything
+// the user can run.
+func TestResolveEffectTellsAMissingDefinitionFromOneThatNamesNothing(t *testing.T) {
+	_, _, missing := ResolveEffect(nil, "breathing", []via.Channel{via.ChannelRgbMatrix}, true)
+	_, _, empty := ResolveEffect(NewCatalog("GMMK Pro", nil), "breathing", []via.Channel{via.ChannelRgbMatrix}, true)
+
+	if missing == nil || empty == nil {
+		t.Fatal("both cases have to be errors")
+	}
+	if missing.Error() == empty.Error() {
+		t.Errorf("both messages read %q, want them told apart", empty)
+	}
+	if !strings.Contains(missing.Error(), "keyboard fetch") {
+		t.Errorf("missing-definition error = %q, want it to point at `keyboard fetch`", missing)
+	}
+}
+
+// A catalog that names one channel names effects, so the empty-catalog message must
+// not swallow a real lookup on that channel.
+func TestHasNamesIsAboutTheWholeCatalog(t *testing.T) {
+	var none *Catalog
+	if none.HasNames() {
+		t.Error("a nil catalog has names, want none")
+	}
+	if NewCatalog("GMMK Pro", nil).HasNames() {
+		t.Error("a catalog with no entries has names, want none")
+	}
+	if !impact80(t).HasNames() {
+		t.Error("the vendored definition has no names, want its effect list")
+	}
+}