From c55925d550bdb6423e44dadb1384c426e68b1665 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 3 May 2026 10:57:13 +0700 Subject: [PATCH] fix(cli): -h/--help exits 0 instead of error code parseDoctorArgs hand-rolled its own flag loop and surfaced help text as an error; parseTestArgs used flag.ContinueOnError but propagated flag.ErrHelp to main() which printed "error: flag: help requested" and exited 1. Switch parseDoctorArgs to flag.NewFlagSet matching parseTestArgs, then recognise flag.ErrHelp in main() so all subcommands exit 0 on -h. --- cmd/sanderling/doctor.go | 26 ++++++++------------------ cmd/sanderling/doctor_test.go | 19 +++++++++++++++---- cmd/sanderling/main.go | 14 ++++++++------ 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/cmd/sanderling/doctor.go b/cmd/sanderling/doctor.go index fa0c183..ad94303 100644 --- a/cmd/sanderling/doctor.go +++ b/cmd/sanderling/doctor.go @@ -2,6 +2,7 @@ package main import ( "context" + "flag" "fmt" "io" "os" @@ -111,24 +112,13 @@ type doctorOptions struct { platform string } -func parseDoctorArgs(args []string) (doctorOptions, error) { - options := doctorOptions{platform: "all"} - for index := 0; index < len(args); index++ { - argument := args[index] - switch { - case argument == "--platform": - index++ - if index >= len(args) { - return doctorOptions{}, fmt.Errorf("--platform requires a value") - } - options.platform = args[index] - case len(argument) > len("--platform=") && argument[:len("--platform=")] == "--platform=": - options.platform = argument[len("--platform="):] - case argument == "-h" || argument == "--help": - return doctorOptions{}, fmt.Errorf("doctor [--platform=web|android|ios|all]") - default: - return doctorOptions{}, fmt.Errorf("unknown doctor argument: %q", argument) - } +func parseDoctorArgs(args []string, stderr io.Writer) (doctorOptions, error) { + flagSet := flag.NewFlagSet("doctor", flag.ContinueOnError) + flagSet.SetOutput(stderr) + var options doctorOptions + flagSet.StringVar(&options.platform, "platform", "all", "target platform: web, android, ios, all") + if err := flagSet.Parse(args); err != nil { + return doctorOptions{}, err } switch options.platform { case "web", "android", "ios", "all": diff --git a/cmd/sanderling/doctor_test.go b/cmd/sanderling/doctor_test.go index d7e70ce..d33a34c 100644 --- a/cmd/sanderling/doctor_test.go +++ b/cmd/sanderling/doctor_test.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "errors" + "flag" + "io" "strings" "testing" ) @@ -139,7 +141,7 @@ func TestDoctorChecksFor_UnknownPlatform(t *testing.T) { } func TestParseDoctorArgs_DefaultAll(t *testing.T) { - options, err := parseDoctorArgs(nil) + options, err := parseDoctorArgs(nil, io.Discard) if err != nil { t.Fatal(err) } @@ -153,7 +155,7 @@ func TestParseDoctorArgs_ExplicitPlatform(t *testing.T) { {"--platform", "web"}, {"--platform=web"}, } { - options, err := parseDoctorArgs(form) + options, err := parseDoctorArgs(form, io.Discard) if err != nil { t.Fatalf("%v: %v", form, err) } @@ -164,10 +166,19 @@ func TestParseDoctorArgs_ExplicitPlatform(t *testing.T) { } func TestParseDoctorArgs_RejectsUnknown(t *testing.T) { - if _, err := parseDoctorArgs([]string{"--platform=fuchsia"}); err == nil { + if _, err := parseDoctorArgs([]string{"--platform=fuchsia"}, io.Discard); err == nil { t.Error("expected error for unsupported platform") } - if _, err := parseDoctorArgs([]string{"--bogus"}); err == nil { + if _, err := parseDoctorArgs([]string{"--bogus"}, io.Discard); err == nil { t.Error("expected error for unknown argument") } } + +func TestParseDoctorArgs_HelpReturnsErrHelp(t *testing.T) { + if _, err := parseDoctorArgs([]string{"-h"}, io.Discard); !errors.Is(err, flag.ErrHelp) { + t.Errorf("expected flag.ErrHelp for -h, got %v", err) + } + if _, err := parseDoctorArgs([]string{"--help"}, io.Discard); !errors.Is(err, flag.ErrHelp) { + t.Errorf("expected flag.ErrHelp for --help, got %v", err) + } +} diff --git a/cmd/sanderling/main.go b/cmd/sanderling/main.go index aac5085..e14fe37 100644 --- a/cmd/sanderling/main.go +++ b/cmd/sanderling/main.go @@ -76,15 +76,12 @@ func runTest(options testOptions, stdout io.Writer) error { return runTestPipeline(ctx, options, stdout) } -func runDoctor(args []string, stdout io.Writer) error { - options, err := parseDoctorArgs(args) +func runDoctor(args []string, stdout, stderr io.Writer) error { + options, err := parseDoctorArgs(args, stderr) if err != nil { return err } checks := doctorChecksFor(options.platform) - if checks == nil { - return fmt.Errorf("no checks for platform %q", options.platform) - } return runDoctorChecks(context.Background(), checks, stdout) } @@ -107,7 +104,7 @@ func run(args []string, stdout, stderr io.Writer) error { } return runInspect(options, stdout) case "doctor": - return runDoctor(args[2:], stdout) + return runDoctor(args[2:], stdout, stderr) case "version", "-v", "--version": fmt.Fprintln(stdout, Version) return nil @@ -118,6 +115,11 @@ func run(args []string, stdout, stderr io.Writer) error { func main() { if err := run(os.Args, os.Stdout, os.Stderr); err != nil { + // flag.ErrHelp means -h/--help was requested; flag already printed + // usage to stderr, so exit 0 rather than treating it as a failure. + if errors.Is(err, flag.ErrHelp) { + return + } fmt.Fprintf(os.Stderr, "error: %v\n", err) os.Exit(1) }