From 080dfdebcab2ffed7939f51136017c5e017a585a Mon Sep 17 00:00:00 2001 From: Thomas Hartwig Date: Fri, 24 Apr 2026 13:15:33 +0200 Subject: [PATCH 1/4] =?UTF-8?q?=E2=9C=A8=20feat(completion):=20improve=20@?= =?UTF-8?q?N=20UX=20with=20grouped=20section=20menu?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typing `matter @1` previously offered only sibling nodes that happened to share the @1 prefix. Users had to already know to type space (for device commands) or slash (for endpoints) to see anything useful. This change wires up a four-section completion menu whenever the typed token is an exact numeric node ID: Devices – sibling nodes matching the prefix (@1, @11, ...) Endpoints – node's endpoints (@1/0, @1/1, ...) Device Commands – target-aware commands (tree, rename, ...) Cluster Commands – cluster shorthands (OnOff, LevelControl, ...) Tools – code, help, ... Implementation: • cli/completion/completer.go - Add TopLevelCommand struct and ExpandSeparator constant. - TargetCompletionFunc gains a topLevelCommands func() parameter. - Stage-1b: when toComplete is a bare numeric @N that exactly matches a commissioned node, emit @N/EP endpoint tokens and @N+ expansion tokens for each target-aware top-level command. - RootCompletionFunc accepts the same parameter and forwards it. • cli/cluster.go - Add topLevelCommandsForCompletion(), which snapshots the visible root cobra commands and labels each as device/cluster/tool. Non- TargetAware commands (commission, discover, fabric, …) are excluded. • cli/root.go - Execute() skips @target extraction for __complete invocations so the target token reaches the completion function instead of being consumed by the normal arg-parsing path. • cli/completion.go (zsh script) - Split legacy target_cmds bucket into device_targets (@N) and endpoint_targets (@N/EP). - Parse @N+ expansion tokens: look up in _matter_group_map, bucket into exp_device/cluster/tool arrays. - Emit expansion groups with compadd -U -p "@N " so the candidate displays as the bare command name but inserts as "@N ". • cli/completion/completer_test.go - Tests for stage-1b endpoint emission on exact match. - Tests for @N+ token generation with/without topLevelCommands. - Tests confirming alias-based and non-existent node prefixes do not trigger expansion. Closes #59 --- cli/cluster.go | 36 +++++++ cli/completion.go | 43 ++++++++- cli/completion/completer.go | 108 ++++++++++++++++++++- cli/completion/completer_test.go | 160 +++++++++++++++++++++++++++++-- cli/decommission.go | 2 +- cli/fabric.go | 2 +- cli/rename.go | 2 +- cli/root.go | 39 ++++++-- 8 files changed, 366 insertions(+), 26 deletions(-) diff --git a/cli/cluster.go b/cli/cluster.go index e2a374b..e1307fc 100644 --- a/cli/cluster.go +++ b/cli/cluster.go @@ -61,6 +61,42 @@ func completionClusterFilter() map[uint32]bool { return targetEndpointClusterIDs } +// topLevelCommandsForCompletion snapshots the visible root subcommands and +// labels each with its completion group ("device", "cluster", or "tool") and +// whether it remains relevant once a node target has been selected. The +// completion package uses the result to populate "@N+" expansion tokens +// so that Tab after an exact node match offers device commands, tools, and +// help alongside endpoint completions. +// +// Shorthand cluster commands and hidden commands are omitted — they are +// surfaced elsewhere (by cluster-name matching or via the cluster parent +// command) and would otherwise flood the menu. +func topLevelCommandsForCompletion() []completion.TopLevelCommand { + cmds := allRootCommands() + out := make([]completion.TopLevelCommand, 0, len(cmds)) + for _, c := range cmds { + if c.Hidden || isShorthandCluster(c) || !c.IsAvailableCommand() { + continue + } + var group string + switch c.GroupID { + case groupDevices: + group = "device" + case groupClusters: + group = "cluster" + case groupTools: + group = "tool" + } + out = append(out, completion.TopLevelCommand{ + Name: c.Name(), + Short: c.Short, + Group: group, + TargetAware: !targetUnawareCommands[c.Name()], + }) + } + return out +} + func init() { allRootCommands = func() []*cobra.Command { return rootCmd.Commands() } rootCmd.AddCommand(withGroup(newClusterCmd(), groupClusters)) diff --git a/cli/completion.go b/cli/completion.go index fbb2d9b..5ca0b8a 100644 --- a/cli/completion.go +++ b/cli/completion.go @@ -497,7 +497,8 @@ compdef _matter matter # Group headers: requires group-name '' to be set (oh-my-zsh sets this # globally; we set it locally for matter so vanilla zsh also benefits). zstyle ':completion:*:matter:*' group-name '' -zstyle ':completion:*:matter:*:targets' format $'\e[35m── Targets ──\e[0m' +zstyle ':completion:*:matter:*:devices' format $'\e[35m── Devices ──\e[0m' +zstyle ':completion:*:matter:*:endpoints' format $'\e[35m── Endpoints ──\e[0m' zstyle ':completion:*:matter:*:device-commands' format $'\e[36m── Device Commands ──\e[0m' zstyle ':completion:*:matter:*:cluster-commands' format $'\e[32m── Cluster Commands ──\e[0m' zstyle ':completion:*:matter:*:tools' format $'\e[33m── Tools ──\e[0m' @@ -513,8 +514,11 @@ _matter_group_map=( _matter() { local -a request_cmd - local -a device_cmds cluster_cmds tool_cmds target_cmds other_cmds invoke_cmds attr_cmds + local -a device_targets endpoint_targets + local -a device_cmds cluster_cmds tool_cmds other_cmds invoke_cmds attr_cmds + local -a exp_device_cmds exp_cluster_cmds exp_tool_cmds local out word desc entry tag i directive _matter_line + local exp_target="" # Build the __complete call from the current word list. request_cmd=("${words[1]}" "__complete") @@ -537,8 +541,24 @@ _matter() { [[ -z "$word" || "$word" == :* || "$word" == _activeHelp_* ]] && continue # Escape colons in word and description (zsh _describe uses : as separator). entry="${word//:/\\:}:${desc//:/\\:}" - if [[ "$word" == @* ]]; then - target_cmds+=("$entry") + if [[ "$word" == @*+* ]]; then + # Expansion token "@N+": splits into the @N target prefix and a + # bare subcommand name. The display is the subcommand alone; selection + # inserts "@N " (as two shell words) via compadd -U -p below. + local _exp_prefix="${word%%+*}" + local _exp_cmd="${word#*+}" + exp_target="$_exp_prefix" + tag="${_matter_group_map[$_exp_cmd]}" + local _exp_entry="${_exp_cmd//:/\\:}:${desc//:/\\:}" + case "$tag" in + device) exp_device_cmds+=("$_exp_entry") ;; + cluster) exp_cluster_cmds+=("$_exp_entry") ;; + tool) exp_tool_cmds+=("$_exp_entry") ;; + esac + elif [[ "$word" == @*/* ]]; then + endpoint_targets+=("$entry") + elif [[ "$word" == @* ]]; then + device_targets+=("$entry") else tag="${_matter_group_map[$word]}" case "$tag" in @@ -566,7 +586,20 @@ _matter() { local -a nospace (( directive & 2 )) && nospace=(-S '') - (( ${#target_cmds} )) && _describe -t targets "Targets" target_cmds "${nospace[@]}" + (( ${#device_targets} )) && _describe -t devices "Devices" device_targets "${nospace[@]}" + (( ${#endpoint_targets} )) && _describe -t endpoints "Endpoints" endpoint_targets "${nospace[@]}" + + # Expansion entries share the same "@N " target prefix. -U disables prefix + # matching against the typed @N word so the bare subcommand names are kept + # as candidates; -p prepends "@N " to the inserted text so the subcommand + # becomes a separate shell word after the target. + if [[ -n "$exp_target" ]]; then + local -a exp_prefix_arg=(-U -p "${exp_target} ") + (( ${#exp_device_cmds} )) && _describe -t device-commands "Device Commands" exp_device_cmds "${exp_prefix_arg[@]}" + (( ${#exp_cluster_cmds} )) && _describe -t cluster-commands "Cluster Commands" exp_cluster_cmds "${exp_prefix_arg[@]}" + (( ${#exp_tool_cmds} )) && _describe -t tools "Tools" exp_tool_cmds "${exp_prefix_arg[@]}" + fi + (( ${#device_cmds} )) && _describe -t device-commands "Device Commands" device_cmds "${nospace[@]}" (( ${#cluster_cmds} )) && _describe -t cluster-commands "Cluster Commands" cluster_cmds "${nospace[@]}" (( ${#tool_cmds} )) && _describe -t tools "Tools" tool_cmds "${nospace[@]}" diff --git a/cli/completion/completer.go b/cli/completion/completer.go index cdc7517..bac8dd5 100644 --- a/cli/completion/completer.go +++ b/cli/completion/completer.go @@ -263,10 +263,34 @@ func NodeIDCompletionFunc() func(cmd *cobra.Command, args []string, toComplete s } } +// TopLevelCommand describes a single root-level cobra subcommand for the +// purposes of @target completion expansion. When the user types "@N" +// against an existing node, these commands are surfaced as "@N+" +// tokens so the shell can offer them alongside endpoint and sibling-node +// completions in a single menu. +type TopLevelCommand struct { + // Name is the subcommand name as registered with cobra (e.g. "tree"). + Name string + // Short is the one-line description shown next to the command. + Short string + // Group is "device", "cluster", "tool", or "" — used by the zsh script + // to route each entry into the matching section header. + Group string + // TargetAware reports whether the command accepts/requires a node + // target. Commands that operate on the fabric as a whole (e.g. + // "commission", "discover") are skipped for @N expansion so the menu + // stays relevant. + TargetAware bool +} + // RootCompletionFunc returns a cobra ValidArgsFunction for the root command -// that handles two completion types: +// that handles three completion types: // // - @target tokens (e.g. "@1/2") — delegated to TargetCompletionFunc. +// When a numeric @N exactly matches an existing node, the returned set +// is enriched with endpoint tokens (@N/0, @N/1, ...) and "@N+" +// expansion tokens for the commands returned by topLevelCommands, so +// the user does not need to type " " or "/" to see what comes next. // - cluster shorthand commands — case-insensitive prefix/substring match of // cluster names, so typing "on" offers "OnOff" and "level" offers // "LevelControl". @@ -278,11 +302,15 @@ func NodeIDCompletionFunc() func(cmd *cobra.Command, args []string, toComplete s // to the set of cluster IDs present on the current target endpoint. A nil map // means no filter (show all clusters); a non-nil but empty map means no // clusters are applicable (e.g. node-only target without an endpoint). +// +// topLevelCommands, if non-nil, is called when expanding an exact @N match to +// seed the "Device Commands" / "Tools" / "Cluster Commands" sections. func RootCompletionFunc( registry *clusters.Registry, allowedClusters func() map[uint32]bool, + topLevelCommands func() []TopLevelCommand, ) func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - targetFn := TargetCompletionFunc() + targetFn := TargetCompletionFunc(topLevelCommands) return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { if strings.HasPrefix(toComplete, "@") { return targetFn(cmd, args, toComplete) @@ -314,6 +342,14 @@ func RootCompletionFunc( } } +// ExpandSeparator is the separator embedded in "@N+" expansion tokens. +// The zsh completion script recognises it to split the target prefix from the +// subcommand name and invoke compadd with -U -p "@N " so the subcommand is +// inserted as a separate shell word while the displayed candidate is the +// bare subcommand name. The "+" character is used because it cannot appear +// in either a valid numeric node token or a top-level cobra command name. +const ExpandSeparator = "+" + // TargetCompletionFunc returns a cobra ValidArgsFunction that completes // @target tokens in two stages. Emitted tokens are always numeric @N; // device names are shown only in the description as a visual hint. @@ -327,11 +363,24 @@ func RootCompletionFunc( // "/" to proceed to endpoint selection or " " (space) for device // commands. // +// Stage 1b — exact match expansion: +// When toComplete is a numeric @N that exactly identifies a commissioned +// node, the result set is enriched with: +// - endpoint tokens "@N/0", "@N/1", ... so the user sees endpoints without +// having to type "/" first; and +// - "@N+" expansion tokens for each top-level command returned by +// topLevelCommands (if non-nil), so device-level commands and tools +// appear in the same menu as the endpoint list. The zsh script strips +// the "@N+" prefix before display and inserts the selection as a +// separate word after "@N ". +// // Stage 2 — endpoint selection ("/" present): // When the user types "@1/", completions are the non-root endpoints on // the matched node (e.g. "@1/1", "@1/2"). Normal trailing space is // applied so the user can proceed to a command after selection. -func TargetCompletionFunc() func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { +func TargetCompletionFunc( + topLevelCommands func() []TopLevelCommand, +) func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { // Only complete if the user is typing a @target token. if !strings.HasPrefix(toComplete, "@") { @@ -356,6 +405,18 @@ func TargetCompletionFunc() func(cmd *cobra.Command, args []string, toComplete s partial := strings.ToLower(toComplete[1:]) namePart, epPart, hasSlash := strings.Cut(partial, "/") + // Look for an exact numeric match so we can enrich stage-1 with + // endpoint + subcommand expansion for that node. + var exactNode *store.Node + if !hasSlash && isAllDigits(namePart) { + for _, n := range nodes { + if fmt.Sprintf("%d", n.ID) == namePart { + exactNode = n + break + } + } + } + var completions []string for _, n := range nodes { idStr := fmt.Sprintf("%d", n.ID) @@ -419,9 +480,48 @@ func TargetCompletionFunc() func(cmd *cobra.Command, args []string, toComplete s } } + // ── Stage 1b: enrich exact-match with endpoints + command expansion ── + if exactNode != nil { + idStr := fmt.Sprintf("%d", exactNode.ID) + alias := idStr + if exactNode.Name != "" { + alias = strings.ReplaceAll(strings.ToLower(exactNode.Name), " ", "-") + } + + // Endpoint tokens. User sees these in the same menu; they match + // the "@N" prefix so the shell keeps them. + for _, ep := range exactNode.Endpoints { + epStr := fmt.Sprintf("%d", ep.ID) + epDesc := endpointDescription(ep) + var desc string + if alias != idStr { + desc = fmt.Sprintf("%s [%s]", epDesc, alias) + } else { + desc = epDesc + } + completions = append(completions, fmt.Sprintf("@%s/%s\t%s", idStr, epStr, desc)) + } + + // Command expansion tokens. The zsh script splits on + // ExpandSeparator and inserts "@N " as a word prefix via + // compadd -U -p so the subcommand is displayed bare but + // dispatched as a separate shell word. + if topLevelCommands != nil { + for _, tc := range topLevelCommands() { + if !tc.TargetAware { + continue + } + completions = append(completions, + fmt.Sprintf("@%s%s%s\t%s", idStr, ExpandSeparator, tc.Name, tc.Short)) + } + } + } + if !hasSlash { // Node-only stage: suppress trailing space so the user can type - // "/" for endpoint or " " for device-level commands. + // "/" for endpoint or " " for device-level commands. The shell + // script intercepts "@N+" tokens separately and inserts + // them with a literal space via compadd -U -p. return completions, cobra.ShellCompDirectiveNoSpace | cobra.ShellCompDirectiveNoFileComp } return completions, cobra.ShellCompDirectiveNoFileComp diff --git a/cli/completion/completer_test.go b/cli/completion/completer_test.go index 71b10a6..7440bb8 100644 --- a/cli/completion/completer_test.go +++ b/cli/completion/completer_test.go @@ -98,7 +98,17 @@ func testNodes() []*store.Node { func runTargetCompletion(t *testing.T, toComplete string) ([]string, cobra.ShellCompDirective) { t.Helper() - fn := TargetCompletionFunc() + fn := TargetCompletionFunc(nil) + cmd := &cobra.Command{Use: "test"} + completions, directive := fn(cmd, nil, toComplete) + return completions, directive +} + +func runTargetCompletionWithCommands( + t *testing.T, toComplete string, cmds []TopLevelCommand, +) ([]string, cobra.ShellCompDirective) { + t.Helper() + fn := TargetCompletionFunc(func() []TopLevelCommand { return cmds }) cmd := &cobra.Command{Use: "test"} completions, directive := fn(cmd, nil, toComplete) return completions, directive @@ -434,7 +444,7 @@ func TestRootCompletionFunc_ClusterShorthand(t *testing.T) { defer cleanup() reg := testRegistry() - fn := RootCompletionFunc(reg, nil) + fn := RootCompletionFunc(reg, nil, nil) cmd := &cobra.Command{Use: "test"} cases := []struct { @@ -479,7 +489,7 @@ func TestRootCompletionFunc_ClusterShorthand_NoMatch(t *testing.T) { defer cleanup() reg := testRegistry() - fn := RootCompletionFunc(reg, nil) + fn := RootCompletionFunc(reg, nil, nil) cmd := &cobra.Command{Use: "test"} completions, directive := fn(cmd, nil, "zzznomatch") @@ -498,7 +508,7 @@ func TestRootCompletionFunc_AtTarget(t *testing.T) { defer cleanup() reg := testRegistry() - fn := RootCompletionFunc(reg, nil) + fn := RootCompletionFunc(reg, nil, nil) cmd := &cobra.Command{Use: "test"} completions, _ := fn(cmd, nil, "@") @@ -523,7 +533,7 @@ func TestRootCompletionFunc_ClusterFilter(t *testing.T) { cmd := &cobra.Command{Use: "test"} t.Run("nil map → all clusters", func(t *testing.T) { - fn := RootCompletionFunc(reg, func() map[uint32]bool { return nil }) + fn := RootCompletionFunc(reg, func() map[uint32]bool { return nil }, nil) completions, _ := fn(cmd, nil, "") if len(completions) != 3 { t.Errorf("expected 3 completions (all registry clusters), got %d: %v", len(completions), completions) @@ -531,7 +541,7 @@ func TestRootCompletionFunc_ClusterFilter(t *testing.T) { }) t.Run("empty map → no clusters", func(t *testing.T) { - fn := RootCompletionFunc(reg, func() map[uint32]bool { return map[uint32]bool{} }) + fn := RootCompletionFunc(reg, func() map[uint32]bool { return map[uint32]bool{} }, nil) completions, _ := fn(cmd, nil, "") if len(completions) != 0 { t.Errorf("expected no completions for empty allow set, got %v", completions) @@ -540,7 +550,7 @@ func TestRootCompletionFunc_ClusterFilter(t *testing.T) { t.Run("restricted set → only allowed clusters", func(t *testing.T) { allowed := map[uint32]bool{0x0006: true} // OnOff only - fn := RootCompletionFunc(reg, func() map[uint32]bool { return allowed }) + fn := RootCompletionFunc(reg, func() map[uint32]bool { return allowed }, nil) completions, _ := fn(cmd, nil, "") if len(completions) != 1 { t.Fatalf("expected exactly 1 completion, got %v", completions) @@ -552,7 +562,7 @@ func TestRootCompletionFunc_ClusterFilter(t *testing.T) { }) t.Run("filter does not affect @target completion", func(t *testing.T) { - fn := RootCompletionFunc(reg, func() map[uint32]bool { return map[uint32]bool{} }) + fn := RootCompletionFunc(reg, func() map[uint32]bool { return map[uint32]bool{} }, nil) completions, _ := fn(cmd, nil, "@") if len(completions) == 0 { t.Error("empty allow set should not suppress @target completions") @@ -560,6 +570,140 @@ func TestRootCompletionFunc_ClusterFilter(t *testing.T) { }) } +// TestTargetCompletionFunc_Stage1b_EndpointsInExactMatch verifies that when the +// user types an exact numeric node ID (e.g. "@1"), the completion set includes +// both the bare @N token and endpoint tokens (@N/0, @N/1) without requiring the +// user to type "/". +func TestTargetCompletionFunc_Stage1b_EndpointsInExactMatch(t *testing.T) { + cleanup := setupTestStore(t, 1, testNodes()) + defer cleanup() + + completions, directive := runTargetCompletion(t, "@1") + + if len(completions) == 0 { + t.Fatal("expected completions for @1, got none") + } + + // Must include NoSpace so the user can type "/" for endpoints without a + // trailing space being inserted. + if directive&cobra.ShellCompDirectiveNoSpace == 0 { + t.Errorf("expected ShellCompDirectiveNoSpace for exact @N match") + } + + tokens := make(map[string]bool) + for _, c := range completions { + token := strings.SplitN(c, "\t", 2)[0] + tokens[token] = true + } + + // Node itself must be present. + if !tokens["@1"] { + t.Errorf("expected @1 in completions, got %v", completions) + } + + // Endpoint tokens for node 1 (endpoints 0 and 1). + for _, want := range []string{"@1/0", "@1/1"} { + if !tokens[want] { + t.Errorf("expected endpoint token %q in stage-1b completions, got %v", want, completions) + } + } + + // No expansion tokens expected (no topLevelCommands supplied). + for tok := range tokens { + if strings.Contains(tok, ExpandSeparator) { + t.Errorf("unexpected expansion token %q without topLevelCommands", tok) + } + } +} + +// TestTargetCompletionFunc_Stage1b_CommandsInExactMatch verifies that when a +// topLevelCommands func is provided and the user types an exact @N, the result +// set includes "@N+" expansion tokens for each target-aware command. +func TestTargetCompletionFunc_Stage1b_CommandsInExactMatch(t *testing.T) { + cleanup := setupTestStore(t, 1, testNodes()) + defer cleanup() + + cmds := []TopLevelCommand{ + {Name: "tree", Short: "Show device tree", Group: "device", TargetAware: true}, + {Name: "OnOff", Short: "On/Off cluster", Group: "cluster", TargetAware: true}, + {Name: "code", Short: "Parse or generate pairing codes", Group: "tool", TargetAware: false}, + {Name: "commission", Short: "Commission a device", Group: "device", TargetAware: false}, + } + + completions, _ := runTargetCompletionWithCommands(t, "@1", cmds) + + tokens := make(map[string]bool) + for _, c := range completions { + token := strings.SplitN(c, "\t", 2)[0] + tokens[token] = true + } + + // TargetAware commands get expansion tokens. + if !tokens["@1"+ExpandSeparator+"tree"] { + t.Errorf("expected @1+tree expansion token, got %v", completions) + } + if !tokens["@1"+ExpandSeparator+"OnOff"] { + t.Errorf("expected @1+OnOff expansion token, got %v", completions) + } + + // Non-TargetAware commands must NOT produce expansion tokens. + if tokens["@1"+ExpandSeparator+"code"] { + t.Errorf("unexpected @1+code expansion token for non-TargetAware command") + } + if tokens["@1"+ExpandSeparator+"commission"] { + t.Errorf("unexpected @1+commission expansion token for non-TargetAware command") + } +} + +// TestTargetCompletionFunc_Stage1b_NamedNoExpansion verifies that alias-based +// prefixes (e.g. "@kitchen") do NOT trigger stage-1b expansion even when they +// fully match a named node — expansion only fires for numeric IDs. +func TestTargetCompletionFunc_Stage1b_NamedNoExpansion(t *testing.T) { + cleanup := setupTestStore(t, 1, testNodes()) + defer cleanup() + + cmds := []TopLevelCommand{ + {Name: "tree", Short: "Show device tree", Group: "device", TargetAware: true}, + } + + completions, _ := runTargetCompletionWithCommands(t, "@kitchen-light", cmds) + + for _, c := range completions { + token := strings.SplitN(c, "\t", 2)[0] + if strings.Contains(token, ExpandSeparator) { + t.Errorf("unexpected expansion token %q for alias-based prefix", token) + } + if strings.Contains(token, "/") { + t.Errorf("unexpected endpoint token %q for alias-based prefix", token) + } + } +} + +// TestTargetCompletionFunc_Stage1b_NonExistentNodeNoExpansion verifies that a +// numeric prefix that does not match any commissioned node produces no expansion +// tokens and no endpoint tokens. +func TestTargetCompletionFunc_Stage1b_NonExistentNodeNoExpansion(t *testing.T) { + cleanup := setupTestStore(t, 1, testNodes()) + defer cleanup() + + cmds := []TopLevelCommand{ + {Name: "tree", Short: "Show device tree", Group: "device", TargetAware: true}, + } + + // Node 99 doesn't exist in the test store. + completions, _ := runTargetCompletionWithCommands(t, "@99", cmds) + + for _, c := range completions { + token := strings.SplitN(c, "\t", 2)[0] + if strings.Contains(token, ExpandSeparator) { + t.Errorf("unexpected expansion token %q for non-existent node", token) + } + if strings.Contains(token, "/") { + t.Errorf("unexpected endpoint token %q for non-existent node", token) + } + } +} + // TestNodeSummary ensures the helper returns a non-empty string for a node // that has a recognisable device type on its first non-root endpoint. func TestNodeSummary(t *testing.T) { diff --git a/cli/decommission.go b/cli/decommission.go index 4205786..c3959fa 100644 --- a/cli/decommission.go +++ b/cli/decommission.go @@ -58,7 +58,7 @@ For a local-only delete (e.g. when the device is permanently offline), use matter @1 decommission matter decommission @1 --force # delete locally even if device is unreachable`, Args: cobra.MaximumNArgs(1), - ValidArgsFunction: completion.TargetCompletionFunc(), + ValidArgsFunction: completion.TargetCompletionFunc(nil), RunE: runDecommission, } // Note: -f is reserved globally for --format, so --force has no short alias. diff --git a/cli/fabric.go b/cli/fabric.go index 2ad835b..14fca0f 100644 --- a/cli/fabric.go +++ b/cli/fabric.go @@ -213,7 +213,7 @@ use "matter decommission" instead.`, matter fabric remove @42 matter @1 fabric remove`, Args: cobra.MaximumNArgs(1), - ValidArgsFunction: completion.TargetCompletionFunc(), + ValidArgsFunction: completion.TargetCompletionFunc(nil), RunE: func(cmd *cobra.Command, args []string) error { // Support both `matter fabric remove @1` (positional arg) and // `matter @1 fabric remove` (inline @target resolved via PersistentPreRunE). diff --git a/cli/rename.go b/cli/rename.go index 12f1e49..eddd995 100644 --- a/cli/rename.go +++ b/cli/rename.go @@ -55,7 +55,7 @@ name, also clearing NodeLabel on the device.`, matter rename @1 --reset matter rename @1 "Porch Lamp" --local`, Args: cobra.ArbitraryArgs, - ValidArgsFunction: completion.TargetCompletionFunc(), + ValidArgsFunction: completion.TargetCompletionFunc(nil), RunE: runRename, } cmd.Flags().Bool("reset", false, "reset name by re-reading ProductName from the device") diff --git a/cli/root.go b/cli/root.go index e77fcad..afaa105 100644 --- a/cli/root.go +++ b/cli/root.go @@ -96,8 +96,12 @@ func init() { // Enable @target completion on the root command so that typing "@" then // Tab at any position offers device targets. Cluster completions are // filtered to those present on the current target endpoint via - // completionClusterFilter (populated in PersistentPreRunE). - rootCmd.ValidArgsFunction = completion.RootCompletionFunc(clusters.Global, completionClusterFilter) + // completionClusterFilter (populated in PersistentPreRunE). The + // top-level command snapshot feeds the "@N+" expansion when the + // user Tab-completes an exact numeric node match. + rootCmd.ValidArgsFunction = completion.RootCompletionFunc( + clusters.Global, completionClusterFilter, topLevelCommandsForCompletion, + ) rootCmd.AddCommand(withGroup(newVersionCmd(), groupTools)) rootCmd.AddCommand(withGroup(newCompletionCmd(), groupTools)) @@ -113,13 +117,22 @@ func init() { // "@1" or "@1/2") and extracts it so that cobra never sees it. The parsed // target is stored in extractedTarget and applied during PersistentPreRunE via // resolveTarget(). +// +// Cobra's built-in completion subcommands (__complete, __completeNoDesc) are +// exempt: their toComplete argument may contain a bare @N token that our +// completion handler needs to see in order to offer context-aware candidates +// (endpoints, device commands, etc.). Extracting it here would leave cobra +// with fewer args than __complete requires and completion would silently +// return nothing. func Execute() error { if len(os.Args) > 1 { args := os.Args[1:] - cleaned, target := ExtractTargetFromArgs(args) - if target != nil { - extractedTarget = target - args = cleaned + if !isCompletionInvocation(args) { + cleaned, target := ExtractTargetFromArgs(args) + if target != nil { + extractedTarget = target + args = cleaned + } } normalized := normalizeShorthandArgs(args, clusters.Global) rootCmd.SetArgs(normalized) @@ -127,6 +140,20 @@ func Execute() error { return rootCmd.Execute() } +// isCompletionInvocation reports whether args start with one of cobra's +// completion-script-facing subcommands. When true, @target extraction must be +// skipped so that the completion handler sees the token as toComplete. +func isCompletionInvocation(args []string) bool { + if len(args) == 0 { + return false + } + switch args[0] { + case "__complete", "__completeNoDesc": + return true + } + return false +} + // normalizeShorthandArgs rewrites cluster shorthand command and sub-command // tokens in args to their canonical PascalCase forms so that cobra's // case-sensitive dispatch works regardless of how the user typed them. From ce73109d840aad8412e27e7e34ba06ef9ecb15ea Mon Sep 17 00:00:00 2001 From: Thomas Hartwig Date: Fri, 24 Apr 2026 14:47:22 +0200 Subject: [PATCH 2/4] fix(completion): address review feedback on @N UX - Extract @targets from committed completion args (not toComplete) so cobra can still resolve subcommands like "matter @1/1 OnOff ". - Gate "@N+" expansion tokens behind MATTER_COMPLETION_EXPAND=zsh so bash, fish, and PowerShell completions do not display the literal zsh-specific encoding. - Route ungrouped expansion commands (e.g. "help") to the Tools bucket in the zsh script's case statement instead of silently dropping them. - Clarify that TopLevelCommand.Group is metadata for shell-specific helpers; it is not encoded in the emitted "@N+" token. Co-Authored-By: Claude Opus 4.7 --- cli/completion.go | 9 +++++++- cli/completion/completer.go | 21 +++++++++++++++---- cli/completion/completer_test.go | 32 +++++++++++++++++++++++++++++ cli/root.go | 35 ++++++++++++++++++++++++-------- 4 files changed, 83 insertions(+), 14 deletions(-) diff --git a/cli/completion.go b/cli/completion.go index 5ca0b8a..ee6fd17 100644 --- a/cli/completion.go +++ b/cli/completion.go @@ -527,7 +527,10 @@ _matter() { done request_cmd+=("${words[$CURRENT]}") - out=$("${request_cmd[@]}" 2>/dev/null) + # MATTER_COMPLETION_EXPAND=zsh opts this shell into the "@N+" expansion + # tokens the loop below parses. Other shells (bash/fish/powershell) do not + # set this and therefore never see the zsh-specific encoding. + out=$(MATTER_COMPLETION_EXPAND=zsh "${request_cmd[@]}" 2>/dev/null) # Extract the cobra ShellCompDirective from the trailing :N line so we can # honour flags like ShellCompDirectiveNoSpace (bit 1, value 2). @@ -554,6 +557,10 @@ _matter() { device) exp_device_cmds+=("$_exp_entry") ;; cluster) exp_cluster_cmds+=("$_exp_entry") ;; tool) exp_tool_cmds+=("$_exp_entry") ;; + # Ungrouped commands (e.g. "help", which cobra does not register in + # any group) are routed to the Tools bucket so they remain visible + # instead of being silently dropped. + *) exp_tool_cmds+=("$_exp_entry") ;; esac elif [[ "$word" == @*/* ]]; then endpoint_targets+=("$entry") diff --git a/cli/completion/completer.go b/cli/completion/completer.go index bac8dd5..fdae351 100644 --- a/cli/completion/completer.go +++ b/cli/completion/completer.go @@ -9,6 +9,7 @@ package completion import ( "fmt" + "os" "sort" "strings" "time" @@ -273,8 +274,10 @@ type TopLevelCommand struct { Name string // Short is the one-line description shown next to the command. Short string - // Group is "device", "cluster", "tool", or "" — used by the zsh script - // to route each entry into the matching section header. + // Group is "device", "cluster", "tool", or "". It is metadata for + // shell-specific completion helpers and is not encoded in the emitted + // "@N+" token; the zsh script derives grouping from its own + // statically-generated _matter_group_map, keyed by command name. Group string // TargetAware reports whether the command accepts/requires a node // target. Commands that operate on the fabric as a whole (e.g. @@ -350,6 +353,13 @@ func RootCompletionFunc( // in either a valid numeric node token or a top-level cobra command name. const ExpandSeparator = "+" +// ExpandEnvVar is the environment variable the zsh completion script sets +// (to "zsh") before invoking "matter __complete ...". Its presence opts the +// caller into receiving "@N+" expansion tokens. Other shells (bash, +// fish, PowerShell) do not set it, so they continue to receive only plain +// @N / @N/ tokens and never display the literal expansion syntax. +const ExpandEnvVar = "MATTER_COMPLETION_EXPAND" + // TargetCompletionFunc returns a cobra ValidArgsFunction that completes // @target tokens in two stages. Emitted tokens are always numeric @N; // device names are shown only in the description as a visual hint. @@ -505,8 +515,11 @@ func TargetCompletionFunc( // Command expansion tokens. The zsh script splits on // ExpandSeparator and inserts "@N " as a word prefix via // compadd -U -p so the subcommand is displayed bare but - // dispatched as a separate shell word. - if topLevelCommands != nil { + // dispatched as a separate shell word. Only emitted when the + // caller opted in via ExpandEnvVar: bash, fish, and PowerShell + // completions do not rewrite these tokens and would otherwise + // surface the literal "@N+" text to the user. + if topLevelCommands != nil && os.Getenv(ExpandEnvVar) == "zsh" { for _, tc := range topLevelCommands() { if !tc.TargetAware { continue diff --git a/cli/completion/completer_test.go b/cli/completion/completer_test.go index 7440bb8..9745514 100644 --- a/cli/completion/completer_test.go +++ b/cli/completion/completer_test.go @@ -108,6 +108,10 @@ func runTargetCompletionWithCommands( t *testing.T, toComplete string, cmds []TopLevelCommand, ) ([]string, cobra.ShellCompDirective) { t.Helper() + // Expansion tokens are only emitted when the caller opts in via + // ExpandEnvVar (set by the zsh completion script). t.Setenv handles + // cleanup automatically. + t.Setenv(ExpandEnvVar, "zsh") fn := TargetCompletionFunc(func() []TopLevelCommand { return cmds }) cmd := &cobra.Command{Use: "test"} completions, directive := fn(cmd, nil, toComplete) @@ -655,6 +659,34 @@ func TestTargetCompletionFunc_Stage1b_CommandsInExactMatch(t *testing.T) { } } +// TestTargetCompletionFunc_Stage1b_ExpandEnvVarGate verifies that when the +// ExpandEnvVar is not set (simulating a non-zsh shell like bash/fish), no +// "@N+" expansion tokens are emitted even if topLevelCommands is provided. +// Those other shells do not know how to rewrite these tokens and would +// otherwise surface the literal "@N+" text to the user. +func TestTargetCompletionFunc_Stage1b_ExpandEnvVarGate(t *testing.T) { + cleanup := setupTestStore(t, 1, testNodes()) + defer cleanup() + + // Explicitly clear the env var to simulate a non-zsh shell. t.Setenv + // guarantees the original value is restored when the test returns. + t.Setenv(ExpandEnvVar, "") + + cmds := []TopLevelCommand{ + {Name: "tree", Short: "Show device tree", Group: "device", TargetAware: true}, + } + fn := TargetCompletionFunc(func() []TopLevelCommand { return cmds }) + cmd := &cobra.Command{Use: "test"} + completions, _ := fn(cmd, nil, "@1") + + for _, c := range completions { + token := strings.SplitN(c, "\t", 2)[0] + if strings.Contains(token, ExpandSeparator) { + t.Errorf("unexpected expansion token %q when ExpandEnvVar is unset", token) + } + } +} + // TestTargetCompletionFunc_Stage1b_NamedNoExpansion verifies that alias-based // prefixes (e.g. "@kitchen") do NOT trigger stage-1b expansion even when they // fully match a named node — expansion only fires for numeric IDs. diff --git a/cli/root.go b/cli/root.go index afaa105..13e3b02 100644 --- a/cli/root.go +++ b/cli/root.go @@ -118,16 +118,32 @@ func init() { // target is stored in extractedTarget and applied during PersistentPreRunE via // resolveTarget(). // -// Cobra's built-in completion subcommands (__complete, __completeNoDesc) are -// exempt: their toComplete argument may contain a bare @N token that our -// completion handler needs to see in order to offer context-aware candidates -// (endpoints, device commands, etc.). Extracting it here would leave cobra -// with fewer args than __complete requires and completion would silently -// return nothing. +// Cobra's built-in completion subcommands (__complete, __completeNoDesc) get +// special treatment: the final arg is the partial word being completed +// (toComplete) and must be preserved verbatim so the completion handler can +// see it as the current token. @targets in the *committed* args before +// toComplete are still extracted so cobra's command traversal can resolve +// subcommands past the @target (e.g. for "matter @1/1 OnOff " the +// "@1/1" would otherwise stop traversal at root and OnOff's completion +// function would never run). func Execute() error { if len(os.Args) > 1 { args := os.Args[1:] - if !isCompletionInvocation(args) { + if isCompletionInvocation(args) && len(args) >= 3 { + // Preserve args[0] (__complete) and the last element (toComplete); + // extract @targets from the committed args in between. + toComplete := args[len(args)-1] + committed := args[1 : len(args)-1] + cleaned, target := ExtractTargetFromArgs(committed) + if target != nil { + extractedTarget = target + } + rebuilt := make([]string, 0, len(cleaned)+2) + rebuilt = append(rebuilt, args[0]) + rebuilt = append(rebuilt, cleaned...) + rebuilt = append(rebuilt, toComplete) + args = rebuilt + } else if !isCompletionInvocation(args) { cleaned, target := ExtractTargetFromArgs(args) if target != nil { extractedTarget = target @@ -141,8 +157,9 @@ func Execute() error { } // isCompletionInvocation reports whether args start with one of cobra's -// completion-script-facing subcommands. When true, @target extraction must be -// skipped so that the completion handler sees the token as toComplete. +// completion-script-facing subcommands. When true, the final argument is the +// partial word being completed (toComplete) and must be preserved verbatim +// so the completion handler still sees it as the current token. func isCompletionInvocation(args []string) bool { if len(args) == 0 { return false From f1de03ab9b2c942115f5a4e64b229fed56bbe15e Mon Sep 17 00:00:00 2001 From: Thomas Hartwig Date: Fri, 24 Apr 2026 15:05:08 +0200 Subject: [PATCH 3/4] fix: Suppress zsh quoting in completion for device target prefix Prevents backslash-escaping of spaces in "@N " so completions insert as separate shell words, improving CLI ergonomics. --- cli/completion.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cli/completion.go b/cli/completion.go index ee6fd17..bd6f379 100644 --- a/cli/completion.go +++ b/cli/completion.go @@ -599,9 +599,11 @@ _matter() { # Expansion entries share the same "@N " target prefix. -U disables prefix # matching against the typed @N word so the bare subcommand names are kept # as candidates; -p prepends "@N " to the inserted text so the subcommand - # becomes a separate shell word after the target. + # becomes a separate shell word after the target. -Q suppresses zsh's + # default quoting so the space in "@N " stays a plain word separator + # rather than being inserted as a backslash-escaped "\ ". if [[ -n "$exp_target" ]]; then - local -a exp_prefix_arg=(-U -p "${exp_target} ") + local -a exp_prefix_arg=(-U -Q -p "${exp_target} ") (( ${#exp_device_cmds} )) && _describe -t device-commands "Device Commands" exp_device_cmds "${exp_prefix_arg[@]}" (( ${#exp_cluster_cmds} )) && _describe -t cluster-commands "Cluster Commands" exp_cluster_cmds "${exp_prefix_arg[@]}" (( ${#exp_tool_cmds} )) && _describe -t tools "Tools" exp_tool_cmds "${exp_prefix_arg[@]}" From fcbcb2e304f8297d8e29f57944c74bb0eef96583 Mon Sep 17 00:00:00 2001 From: Thomas Hartwig Date: Fri, 24 Apr 2026 15:26:53 +0200 Subject: [PATCH 4/4] chore(mise): clean golangci-lint cache before each lint run Stale cross-package staticcheck facts were causing phantom SA5011 diagnostics locally while CI (which starts with no lint cache) passed. Cleaning the cache up-front costs ~10s but makes local results match CI. Co-Authored-By: Claude Opus 4.7 --- mise.toml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mise.toml b/mise.toml index 9093661..f3b90f4 100644 --- a/mise.toml +++ b/mise.toml @@ -47,7 +47,10 @@ run = "go vet ./..." [tasks.lint] description = "Run golangci-lint (includes vet)" depends = ["vet"] -run = "golangci-lint run ./..." +run = """ +golangci-lint cache clean +golangci-lint run ./... +""" [tasks.fmt] description = "Format Go source files"