Skip to content

Commit 88b81a5

Browse files
committed
fix(cli): search-dir and html flag behaviour
Signed-off-by: Swarit Pandey <swarit.pandey@gmail.com>
1 parent 5d93c7c commit 88b81a5

2 files changed

Lines changed: 88 additions & 4 deletions

File tree

internal/cli/cli.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func Parse(args []string) (*Config, error) {
5959
cfg.OutputFormat = "html"
6060
cfg.OutputFormatSet = true
6161
i++
62-
if i >= len(args) {
62+
if i >= len(args) || looksLikeFlag(args[i]) {
6363
return nil, fmt.Errorf("--html requires a file path argument")
6464
}
6565
cfg.HTMLOutputFile = args[i]
@@ -77,15 +77,14 @@ func Parse(args []string) (*Config, error) {
7777
cfg.ColorMode = mode
7878
case arg == "--search-dirs":
7979
i++
80-
if i >= len(args) || strings.HasPrefix(args[i], "--") {
80+
if i >= len(args) || looksLikeFlag(args[i]) || isCommand(args[i]) {
8181
return nil, fmt.Errorf("--search-dirs requires at least one directory path argument")
8282
}
8383
if !searchDirsSet {
8484
cfg.SearchDirs = nil
8585
searchDirsSet = true
8686
}
87-
// Greedily consume non-flag arguments
88-
for i < len(args) && !strings.HasPrefix(args[i], "--") {
87+
for i < len(args) && !looksLikeFlag(args[i]) && !isCommand(args[i]) {
8988
cfg.SearchDirs = append(cfg.SearchDirs, args[i])
9089
i++
9190
}
@@ -156,3 +155,16 @@ Configuration:
156155
name, name, name,
157156
buildinfo.AgentURL)
158157
}
158+
159+
func looksLikeFlag(s string) bool {
160+
return strings.HasPrefix(s, "-")
161+
}
162+
163+
var commands = map[string]bool{
164+
"install": true, "uninstall": true, "send-telemetry": true,
165+
"configure": true, "version": true, "help": true,
166+
}
167+
168+
func isCommand(s string) bool {
169+
return commands[s]
170+
}

internal/cli/cli_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,3 +181,75 @@ func TestParse_FlagCombinations(t *testing.T) {
181181
t.Errorf("unexpected config: %+v", cfg)
182182
}
183183
}
184+
185+
// Bug 1: --search-dirs greedily consumes single-dash flags and commands
186+
187+
func TestParse_SearchDirsStopsAtCommand_Install(t *testing.T) {
188+
cfg, err := Parse([]string{"--search-dirs", "/tmp", "install"})
189+
if err != nil {
190+
t.Fatal(err)
191+
}
192+
if cfg.Command != "install" {
193+
t.Errorf("expected command=install, got %q (search-dirs consumed it: %v)", cfg.Command, cfg.SearchDirs)
194+
}
195+
if len(cfg.SearchDirs) != 1 || cfg.SearchDirs[0] != "/tmp" {
196+
t.Errorf("expected SearchDirs=[/tmp], got %v", cfg.SearchDirs)
197+
}
198+
}
199+
200+
func TestParse_SearchDirsRejectsSingleDashFlag(t *testing.T) {
201+
// --search-dirs -v should error, not silently consume -v as a directory
202+
_, err := Parse([]string{"--search-dirs", "-v"})
203+
if err == nil {
204+
t.Error("expected error when --search-dirs is followed by a flag")
205+
}
206+
}
207+
208+
func TestParse_SearchDirsStopsAtCommand_Uninstall(t *testing.T) {
209+
cfg, err := Parse([]string{"--search-dirs", "/opt", "uninstall"})
210+
if err != nil {
211+
t.Fatal(err)
212+
}
213+
if cfg.Command != "uninstall" {
214+
t.Errorf("expected command=uninstall, got %q (search-dirs consumed it: %v)", cfg.Command, cfg.SearchDirs)
215+
}
216+
if len(cfg.SearchDirs) != 1 || cfg.SearchDirs[0] != "/opt" {
217+
t.Errorf("expected SearchDirs=[/opt], got %v", cfg.SearchDirs)
218+
}
219+
}
220+
221+
func TestParse_SearchDirsStopsAtCommand_SendTelemetry(t *testing.T) {
222+
cfg, err := Parse([]string{"--search-dirs", "/data", "send-telemetry"})
223+
if err != nil {
224+
t.Fatal(err)
225+
}
226+
if cfg.Command != "send-telemetry" {
227+
t.Errorf("expected command=send-telemetry, got %q (search-dirs consumed it: %v)", cfg.Command, cfg.SearchDirs)
228+
}
229+
}
230+
231+
func TestParse_SearchDirsStopsAtCommand_Configure(t *testing.T) {
232+
cfg, err := Parse([]string{"--search-dirs", "/data", "configure"})
233+
if err != nil {
234+
t.Fatal(err)
235+
}
236+
if cfg.Command != "configure" {
237+
t.Errorf("expected command=configure, got %q (search-dirs consumed it: %v)", cfg.Command, cfg.SearchDirs)
238+
}
239+
}
240+
241+
// Bug 2: --html accepts flags as its filename argument
242+
243+
func TestParse_HTMLRejectsFlag(t *testing.T) {
244+
_, err := Parse([]string{"--html", "--verbose"})
245+
if err == nil {
246+
t.Error("expected error when --html argument looks like a flag, got nil")
247+
}
248+
}
249+
250+
func TestParse_HTMLRejectsDashFlag(t *testing.T) {
251+
_, err := Parse([]string{"--html", "-v"})
252+
if err == nil {
253+
t.Error("expected error when --html argument is -v, got nil")
254+
}
255+
}

0 commit comments

Comments
 (0)