Skip to content

Commit 604592c

Browse files
micthiesenmicthiesen
committed
Refactor subcommand flag repair and positionals
Introduce `RepairLeadingFlagSubcommand`, `EmitPositional`, and `ConsumeUntil` helpers to centralize logic for handling leaked flags, preserving subshells, and scanning for terminators. This significantly reduces boilerplate across all command handlers.
1 parent 46b0baf commit 604592c

35 files changed

Lines changed: 432 additions & 440 deletions

‎handler.go‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,151 @@ func ConsumeFusedFlag(tok string, categories []FlagCategory) (string, bool) {
5656
return "", false
5757
}
5858

59+
// EmitPositional appends a placeholder to result for a positional token,
60+
// except when tok is a subshell substitution (`$(...)`) — in that case
61+
// the token is appended verbatim so the subshell is preserved. Handlers
62+
// must never collapse a subshell token to a data placeholder.
63+
func EmitPositional(result []string, tok, placeholder string) []string {
64+
if IsSubshellToken(tok) {
65+
return append(result, tok)
66+
}
67+
return append(result, placeholder)
68+
}
69+
70+
// ConsumeUntil scans args starting at index i and returns the first
71+
// terminator token it finds along with the index of the slot after it.
72+
// If no terminator is found, it returns ("", len(args)). Callers
73+
// typically skip (or collapse to a single placeholder) everything the
74+
// helper walked past.
75+
//
76+
// Use for grammars like `find ... -exec <cmd> ... ;|+`, where the body
77+
// of a subcommand runs until a well-known terminator.
78+
func ConsumeUntil(args []string, i int, terminators map[string]bool) (string, int) {
79+
for j := i; j < len(args); j++ {
80+
if terminators[args[j]] {
81+
return args[j], j + 1
82+
}
83+
}
84+
return "", len(args)
85+
}
86+
87+
// RepairLeadingFlagSubcommand repairs the case where the outer normalizer
88+
// extracted a global flag (either `--flag` or `--flag=value`) as what it
89+
// believed was the subcommand. The flag token is already appended to
90+
// result; this helper consumes the flag's value if it takes one, walks
91+
// past any additional leading flags and their values, and emits the
92+
// real subcommand token when it finds one.
93+
//
94+
// Parameters:
95+
// - subcommand: the token the outer normalizer extracted as the
96+
// subcommand. When this is not flag-like, the helper is a no-op.
97+
// - args: the handler's remaining tokens.
98+
// - i: the current index in args.
99+
// - result: the handler's accumulating output slice.
100+
// - categories: flag categories used to decide which leading flags
101+
// consume a next-token value and which placeholder that value gets.
102+
// Pass nil when every leading flag is boolean (e.g. `ufw --dry-run`).
103+
// - verbatimValueFlags: optional flags whose values are kept verbatim
104+
// instead of being replaced with a placeholder. Useful for small
105+
// finite value sets like systemctl's `-t/--type`. Pass nil if unused.
106+
//
107+
// Returns the extended result slice, the real subcommand token (or "" if
108+
// none was found), and the new position in args.
109+
//
110+
// Fused-flag note: when the normalizer extracted a `--flag=value` token,
111+
// it was already appended to result verbatim and cannot be rewritten —
112+
// the helper simply advances and finds the real subcommand.
113+
func RepairLeadingFlagSubcommand(
114+
subcommand string,
115+
args []string,
116+
i int,
117+
result []string,
118+
categories []FlagCategory,
119+
verbatimValueFlags map[string]bool,
120+
) ([]string, string, int) {
121+
if subcommand == "" || !looksLikeFlagSubcommand(subcommand) {
122+
return result, subcommand, i
123+
}
124+
125+
// 1. Consume the leaked flag's value, if it has one. A fused flag
126+
// (`--flag=value`) already carries its value in the token that was
127+
// appended by the normalizer; nothing to consume.
128+
if !strings.ContainsRune(subcommand, '=') {
129+
if placeholder, ok := MatchFlagCategory(subcommand, categories); ok {
130+
if i < len(args) && !IsFlagToken(args[i]) {
131+
result = EmitPositional(result, args[i], placeholder)
132+
i++
133+
}
134+
} else if verbatimValueFlags[subcommand] {
135+
if i < len(args) && !IsFlagToken(args[i]) {
136+
result = append(result, args[i])
137+
i++
138+
}
139+
}
140+
}
141+
142+
// 2. Walk forward through any further leading flags (with their
143+
// values) and subshells until we hit a real subcommand token.
144+
for i < len(args) {
145+
tok := args[i]
146+
147+
if IsSubshellToken(tok) {
148+
result = append(result, tok)
149+
i++
150+
continue
151+
}
152+
153+
if fused, ok := ConsumeFusedFlag(tok, categories); ok {
154+
result = append(result, fused)
155+
i++
156+
continue
157+
}
158+
159+
if placeholder, ok := MatchFlagCategory(tok, categories); ok {
160+
result, i = ConsumeFlagArg(tok, args, i, result, placeholder)
161+
continue
162+
}
163+
164+
if verbatimValueFlags[tok] {
165+
result = append(result, tok)
166+
i++
167+
if i < len(args) && !IsFlagToken(args[i]) {
168+
result = append(result, args[i])
169+
i++
170+
}
171+
continue
172+
}
173+
174+
if IsFlagToken(tok) {
175+
result = append(result, tok)
176+
i++
177+
continue
178+
}
179+
180+
// First non-flag positional: the real subcommand.
181+
result = append(result, tok)
182+
return result, tok, i + 1
183+
}
184+
185+
return result, "", i
186+
}
187+
188+
// looksLikeFlagSubcommand reports whether a subcommand token came from
189+
// the outer normalizer extracting a flag-like thing. This covers plain
190+
// flags (`-t`, `--filter`) and fused flags (`--flag=value`).
191+
func looksLikeFlagSubcommand(tok string) bool {
192+
if IsFlagToken(tok) {
193+
return true
194+
}
195+
// A `--flag=value` token that begins with a dash is already handled
196+
// by IsFlagToken; this extra branch catches `-Xval` style fused
197+
// forms that IsFlagToken may not recognize.
198+
if strings.HasPrefix(tok, "-") && strings.ContainsRune(tok, '=') {
199+
return true
200+
}
201+
return false
202+
}
203+
59204
var RedirectConsumeNext = map[string]bool{
60205
">": true, ">>": true, "<": true,
61206
"&>": true, "&>>": true,

‎handler_test.go‎

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
package shellshape
2+
3+
import (
4+
"reflect"
5+
"testing"
6+
)
7+
8+
func TestEmitPositional(t *testing.T) {
9+
tests := []struct {
10+
name string
11+
start []string
12+
tok string
13+
placeholder string
14+
want []string
15+
}{
16+
{"plain token", nil, "foo", "<val>", []string{"<val>"}},
17+
{"preserves prefix", []string{"pre"}, "foo", "<path>", []string{"pre", "<path>"}},
18+
{"subshell preserved", nil, "$(date)", "<val>", []string{"$(date)"}},
19+
{"subshell in middle", []string{"pre"}, "$(pwd)", "<path>", []string{"pre", "$(pwd)"}},
20+
}
21+
for _, tt := range tests {
22+
t.Run(tt.name, func(t *testing.T) {
23+
got := EmitPositional(tt.start, tt.tok, tt.placeholder)
24+
if !reflect.DeepEqual(got, tt.want) {
25+
t.Errorf("EmitPositional(%v, %q, %q) = %v, want %v", tt.start, tt.tok, tt.placeholder, got, tt.want)
26+
}
27+
})
28+
}
29+
}
30+
31+
func TestConsumeUntil(t *testing.T) {
32+
terminators := map[string]bool{";": true, "+": true}
33+
34+
tests := []struct {
35+
name string
36+
args []string
37+
i int
38+
wantTerminator string
39+
wantNext int
40+
}{
41+
{"semicolon terminator", []string{"grep", "-l", "foo", "{}", ";"}, 0, ";", 5},
42+
{"plus terminator", []string{"rm", "{}", "+"}, 0, "+", 3},
43+
{"mid-stream start", []string{"a", "b", "c", ";", "d"}, 2, ";", 4},
44+
{"no terminator", []string{"grep", "-l", "foo"}, 0, "", 3},
45+
{"empty slice", nil, 0, "", 0},
46+
{"start past end", []string{"a"}, 5, "", 1},
47+
{"terminator immediately", []string{";", "a"}, 0, ";", 1},
48+
}
49+
for _, tt := range tests {
50+
t.Run(tt.name, func(t *testing.T) {
51+
term, next := ConsumeUntil(tt.args, tt.i, terminators)
52+
if term != tt.wantTerminator || next != tt.wantNext {
53+
t.Errorf("ConsumeUntil(%v, %d) = (%q, %d), want (%q, %d)",
54+
tt.args, tt.i, term, next, tt.wantTerminator, tt.wantNext)
55+
}
56+
})
57+
}
58+
}
59+
60+
func TestRepairLeadingFlagSubcommand(t *testing.T) {
61+
// Typical setup: a handler with --filter (val), -C (path), and some
62+
// boolean flags. Mimics the pnpm/systemctl family.
63+
valFlags := map[string]bool{"--filter": true, "-F": true}
64+
pathFlags := map[string]bool{"-C": true, "--config": true}
65+
categories := []FlagCategory{
66+
{Flags: valFlags, Placeholder: "<val>"},
67+
{Flags: pathFlags, Placeholder: "<path>"},
68+
}
69+
verbatimFlags := map[string]bool{"-t": true, "--type": true}
70+
71+
tests := []struct {
72+
name string
73+
subcommand string
74+
args []string
75+
startResult []string
76+
categories []FlagCategory
77+
verbatim map[string]bool
78+
wantResult []string
79+
wantRealSubcommand string
80+
wantNextI int
81+
}{
82+
{
83+
name: "not a flag: no-op",
84+
subcommand: "install",
85+
args: []string{"react"},
86+
startResult: []string{"exe", "install"},
87+
categories: categories,
88+
wantResult: []string{"exe", "install"},
89+
wantRealSubcommand: "install",
90+
wantNextI: 0,
91+
},
92+
{
93+
name: "leading val flag then real subcommand",
94+
subcommand: "--filter",
95+
args: []string{"infra", "exec", "tsgo", "--noEmit"},
96+
startResult: []string{"exe", "--filter"},
97+
categories: categories,
98+
wantResult: []string{"exe", "--filter", "<val>", "exec"},
99+
wantRealSubcommand: "exec",
100+
wantNextI: 2,
101+
},
102+
{
103+
name: "leading path flag then real subcommand",
104+
subcommand: "-C",
105+
args: []string{"/app", "add", "react"},
106+
startResult: []string{"exe", "-C"},
107+
categories: categories,
108+
wantResult: []string{"exe", "-C", "<path>", "add"},
109+
wantRealSubcommand: "add",
110+
wantNextI: 2,
111+
},
112+
{
113+
name: "two leading flags before subcommand",
114+
subcommand: "--filter",
115+
args: []string{"infra", "-C", "/app", "add"},
116+
startResult: []string{"exe", "--filter"},
117+
categories: categories,
118+
wantResult: []string{"exe", "--filter", "<val>", "-C", "<path>", "add"},
119+
wantRealSubcommand: "add",
120+
wantNextI: 4,
121+
},
122+
{
123+
name: "boolean flag leaked (no value)",
124+
subcommand: "--dry-run",
125+
args: []string{"enable"},
126+
startResult: []string{"exe", "--dry-run"},
127+
categories: nil,
128+
wantResult: []string{"exe", "--dry-run", "enable"},
129+
wantRealSubcommand: "enable",
130+
wantNextI: 1,
131+
},
132+
{
133+
name: "subshell as leading flag value preserved",
134+
subcommand: "--filter",
135+
args: []string{"$(pick-workspace)", "exec", "tsgo"},
136+
startResult: []string{"exe", "--filter"},
137+
categories: categories,
138+
wantResult: []string{"exe", "--filter", "$(pick-workspace)", "exec"},
139+
wantRealSubcommand: "exec",
140+
wantNextI: 2,
141+
},
142+
{
143+
name: "verbatim-value flag leaked",
144+
subcommand: "-t",
145+
args: []string{"service", "list-units"},
146+
startResult: []string{"exe", "-t"},
147+
categories: nil,
148+
verbatim: verbatimFlags,
149+
wantResult: []string{"exe", "-t", "service", "list-units"},
150+
wantRealSubcommand: "list-units",
151+
wantNextI: 2,
152+
},
153+
{
154+
name: "fused flag leaked",
155+
subcommand: "--filter=infra",
156+
args: []string{"exec", "tsgo"},
157+
startResult: []string{"exe", "--filter=infra"},
158+
categories: categories,
159+
wantResult: []string{"exe", "--filter=infra", "exec"},
160+
wantRealSubcommand: "exec",
161+
wantNextI: 1,
162+
},
163+
{
164+
name: "exhausted without finding subcommand",
165+
subcommand: "--filter",
166+
args: []string{"infra"},
167+
startResult: []string{"exe", "--filter"},
168+
categories: categories,
169+
wantResult: []string{"exe", "--filter", "<val>"},
170+
wantRealSubcommand: "",
171+
wantNextI: 1,
172+
},
173+
{
174+
name: "intermediate subshell before subcommand",
175+
subcommand: "--filter",
176+
args: []string{"infra", "$(date)", "exec"},
177+
startResult: []string{"exe", "--filter"},
178+
categories: categories,
179+
wantResult: []string{"exe", "--filter", "<val>", "$(date)", "exec"},
180+
wantRealSubcommand: "exec",
181+
wantNextI: 3,
182+
},
183+
}
184+
185+
for _, tt := range tests {
186+
t.Run(tt.name, func(t *testing.T) {
187+
got, realSub, nextI := RepairLeadingFlagSubcommand(
188+
tt.subcommand, tt.args, 0, tt.startResult, tt.categories, tt.verbatim,
189+
)
190+
if !reflect.DeepEqual(got, tt.wantResult) {
191+
t.Errorf("result = %v, want %v", got, tt.wantResult)
192+
}
193+
if realSub != tt.wantRealSubcommand {
194+
t.Errorf("realSubcommand = %q, want %q", realSub, tt.wantRealSubcommand)
195+
}
196+
if nextI != tt.wantNextI {
197+
t.Errorf("nextI = %d, want %d", nextI, tt.wantNextI)
198+
}
199+
})
200+
}
201+
}

‎handlers/aws.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,7 @@ func handleAws(subcommand string, tokens []string) []string {
111111
if len(tok) > 2 && tok[:2] == "--" && !booleanFlags[tok] {
112112
i++
113113
if i < len(args) && !shellshape.IsFlagToken(args[i]) {
114-
if shellshape.IsSubshellToken(args[i]) {
115-
result = append(result, args[i])
116-
} else {
117-
result = append(result, "<val>")
118-
}
114+
result = shellshape.EmitPositional(result, args[i], "<val>")
119115
} else {
120116
// Next token is a flag or missing; don't consume.
121117
continue

0 commit comments

Comments
 (0)