Преглед на файлове

group the commands in the root help

Fifteen commands in one flat list is where someone starts reading --help to
find a command and gives up. They are now grouped by what a command is for:
lighting, profiles, keyboards. A group is a cobra.Group on the root and a
GroupID on the command, and it changes the help output and nothing else — no
behaviour hangs off it, so inGroup only sets the field and hands the command
back, which keeps the registration in a single AddCommand call.

The test builds the same tree the binary runs rather than a list of its own, so
a command someone adds without a group fails there instead of appearing under
"Additional Commands" where the grouping says nothing about it. That check
caught two mistakes in the first attempt: keyboard was added twice, because the
old AddCommand line was still there, and the test was checking a registration of
its own that no amount of forgetting would have made diverge.
Paul Klumpp преди 1 седмица
родител
ревизия
37bb34e82e
променени са 2 файла, в които са добавени 98 реда и са изтрити 19 реда
  1. 46 0
      cmd/qmk-rgb-tool/channels_test.go
  2. 52 19
      cmd/qmk-rgb-tool/main.go

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

@@ -425,3 +425,49 @@ func TestZoneNamePrefersTheDefinitionLabelOverAnotherChannelsSubsystem(t *testin
 		}
 	}
 }
+
+// Fifteen commands in one flat list is where a user starts reading the help to
+// find a command and gives up. The groups say what a command is for, and a
+// command that belongs to none of them is a hole in the list.
+func TestRootHelpGroupsTheCommands(t *testing.T) {
+	// The same registration the binary runs, so a command that someone adds
+	// without a group is caught here rather than in the help output.
+	root := newRootCommand()
+	registerCommands(root)
+
+	var out bytes.Buffer
+	root.SetOut(&out)
+	root.SetArgs([]string{"--help"})
+	if err := root.Execute(); err != nil {
+		t.Fatalf("--help returned error: %v", err)
+	}
+	help := out.String()
+
+	for _, want := range []string{
+		"Lighting Commands:",
+		"Profile Commands:",
+		"Keyboard Commands:",
+		"save", "load", "list", "delete", "effect", "definition", "keyboard",
+	} {
+		if !strings.Contains(help, want) {
+			t.Errorf("--help does not mention %q", want)
+		}
+	}
+
+	// Every command the tool adds has to be in a group, or it lands in
+	// "Additional Commands" where the grouping says nothing about it.
+	grouped := make(map[string]bool)
+	for _, c := range root.Commands() {
+		if c.GroupID != "" {
+			grouped[c.Name()] = true
+		}
+	}
+	for _, c := range root.Commands() {
+		if c.Name() == "help" || c.Name() == "completion" {
+			continue
+		}
+		if !grouped[c.Name()] {
+			t.Errorf("command %q is in no group, so it appears ungrouped in --help", c.Name())
+		}
+	}
+}

+ 52 - 19
cmd/qmk-rgb-tool/main.go

@@ -12,6 +12,14 @@ import (
 // version is set at build time via -ldflags.
 var version = "dev"
 
+// Group IDs for the root help. They name a group in the help output and say
+// nothing else: no behaviour hangs off them.
+const (
+	groupLighting = "lighting"
+	groupProfile  = "profile"
+	groupKeyboard = "keyboard"
+)
+
 var (
 	targetDevice string
 	targetZone   string
@@ -24,6 +32,15 @@ func newRootCommand() *cobra.Command {
 		SilenceErrors: true,
 		SilenceUsage:  true,
 	}
+	// The commands are grouped because fifteen of them in one flat list is where
+	// a user starts reading the help to find a command and gives up. A group says
+	// what a command is for, and a command in none of them lands under
+	// "Additional Commands" where the grouping says nothing.
+	cmd.AddGroup(
+		&cobra.Group{ID: groupLighting, Title: "Lighting Commands:"},
+		&cobra.Group{ID: groupProfile, Title: "Profile Commands:"},
+		&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")
@@ -45,8 +62,15 @@ func main() {
 }
 
 var keyboardCmd = &cobra.Command{
-	Use:   "keyboard",
-	Short: "Keyboard management commands",
+	Use:     "keyboard",
+	Short:   "Keyboard management commands",
+	GroupID: groupKeyboard,
+}
+
+// inGroup places a command in one of the root help's groups.
+func inGroup(cmd *cobra.Command, group string) *cobra.Command {
+	cmd.GroupID = group
+	return cmd
 }
 
 // discoverAll is a seam for tests.
@@ -91,23 +115,32 @@ var keyboardInfoCmd = &cobra.Command{
 }
 
 func init() {
-	rootCmd.AddCommand(keyboardCmd)
-	keyboardCmd.AddCommand(keyboardInfoCmd)
-
-	rootCmd.AddCommand(NewEnableCmd())
-	rootCmd.AddCommand(NewDisableCmd())
-	rootCmd.AddCommand(NewInfoCmd())
-	rootCmd.AddCommand(NewEffectCmd())
-	rootCmd.AddCommand(NewBrightnessCmd())
-	rootCmd.AddCommand(NewSpeedCmd())
-	rootCmd.AddCommand(NewColorCmd())
-	rootCmd.AddCommand(NewModeCmd())
-	rootCmd.AddCommand(NewProfileSaveCmd())
-	rootCmd.AddCommand(NewProfileLoadCmd())
-	rootCmd.AddCommand(NewProfileListCmd())
-	rootCmd.AddCommand(NewProfileDeleteCmd())
-	rootCmd.AddCommand(NewDefinitionCmd())
-
+	registerCommands(rootCmd)
 	rootCmd.Version = version
 	rootCmd.SetVersionTemplate("	qmk-rgb-tool {{.Version}}\n")
 }
+
+// registerCommands adds every command to the root, each in the group the help
+// output lists it under. It is a function of its own so that a test can build the
+// same tree the binary runs and check the grouping is real, rather than repeating
+// a list that could fall behind the one that ships.
+func registerCommands(root *cobra.Command) {
+	keyboardCmd.AddCommand(keyboardInfoCmd)
+
+	root.AddCommand(
+		inGroup(NewEnableCmd(), groupLighting),
+		inGroup(NewDisableCmd(), groupLighting),
+		inGroup(NewInfoCmd(), groupLighting),
+		inGroup(NewEffectCmd(), groupLighting),
+		inGroup(NewBrightnessCmd(), groupLighting),
+		inGroup(NewSpeedCmd(), groupLighting),
+		inGroup(NewColorCmd(), groupLighting),
+		inGroup(NewModeCmd(), groupLighting),
+		inGroup(NewProfileSaveCmd(), groupProfile),
+		inGroup(NewProfileLoadCmd(), groupProfile),
+		inGroup(NewProfileListCmd(), groupProfile),
+		inGroup(NewProfileDeleteCmd(), groupProfile),
+		inGroup(NewDefinitionCmd(), groupKeyboard),
+		inGroup(keyboardCmd, groupKeyboard),
+	)
+}