From c978e65ecb25e1be85eeed442c4f53494eb209e7 Mon Sep 17 00:00:00 2001 From: Zaid-edge Date: Wed, 2 Sep 2026 17:54:23 +0500 Subject: [PATCH 1/3] feat: add global --help and --version options radar had no way to report its own version, and --help was only reachable through the flag package's undefined-flag path, which printed usage to stderr and exited 2. Add a VERSION and GLOBAL OPTIONS section to the help text, and accept --help/-help and --version/-version/-V. These are matched in os.Args before flag.Parse rather than registered as flags, because -h is taken by the database host and psql compatibility means it has to stay that way; registering a help flag would shadow it. The scan runs after flag registration so printUsage can still list the collection options. Help now goes to stdout and exits 0. The version comes from the existing build-time -X main.version stamp, so release builds report the tag and unstamped builds report "dev". Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 4 +++ docs/index.md | 4 +++ radar.go | 57 +++++++++++++++++++++++++++++++---- radar_test.go | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 142 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 211a62f..333d695 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,10 @@ The tool does **not** collect: passwords, query result data, table contents, or ``` Usage: radar [options] +GLOBAL OPTIONS: + --help show help + --version, -V print the version + Options: -U string database user diff --git a/docs/index.md b/docs/index.md index 4a95cc4..d93145f 100644 --- a/docs/index.md +++ b/docs/index.md @@ -89,6 +89,10 @@ The tool does **not** collect: passwords, query result data, table contents, or ``` Usage: radar [options] +GLOBAL OPTIONS: + --help show help + --version, -V print the version + Options: -U string database user diff --git a/radar.go b/radar.go index 74bffe6..cba014b 100644 --- a/radar.go +++ b/radar.go @@ -34,6 +34,42 @@ import ( // Release builds set it to e.g. "v0.5.0"; unstamped dev builds report "dev". var version = "dev" +// errHelpRequested and errVersionRequested are returned by parseConfig when a +// global option was given instead of a collection request. main prints the +// corresponding output and exits successfully. +var ( + errHelpRequested = errors.New("help requested") + errVersionRequested = errors.New("version requested") +) + +// globalOption reports which global option appears in args, if any. These are +// matched before flag parsing because -h is taken by the database host, so +// help cannot be registered as a regular flag without shadowing it. +func globalOption(args []string) error { + for _, arg := range args { + switch arg { + case "-help", "--help": + return errHelpRequested + case "-version", "--version", "-V": + return errVersionRequested + } + } + return nil +} + +// printUsage writes the help text: the usage line, the build version, the +// global options, then the collection options from the registered flag set. +func printUsage(w io.Writer) { + fmt.Fprintf(w, "Usage: radar [options]\n\n") + fmt.Fprintf(w, "VERSION:\n %s\n\n", version) + fmt.Fprintf(w, "GLOBAL OPTIONS:\n") + fmt.Fprintf(w, " --help show help\n") + fmt.Fprintf(w, " --version, -V print the version\n\n") + fmt.Fprintf(w, "Options:\n") + flag.CommandLine.SetOutput(w) + flag.PrintDefaults() +} + // defaultDisabledTasks lists task names not run unless -include lists them. // pgstattuple_approx() reads heap pages of every user table. var defaultDisabledTasks = []string{"pgstattuple"} @@ -248,7 +284,14 @@ var ( // main is the radar entry point. func main() { cfg, err := parseConfig() - if err != nil { + switch { + case errors.Is(err, errHelpRequested): + printUsage(os.Stdout) + return + case errors.Is(err, errVersionRequested): + fmt.Printf("radar version %s\n", version) + return + case err != nil: errorLog.Println(err) flag.Usage() os.Exit(ExitUsageError) @@ -328,10 +371,7 @@ func main() { func parseConfig() (*Config, error) { cfg := &Config{} - flag.Usage = func() { - fmt.Fprintf(os.Stderr, "Usage: radar [options]\n\nOptions:\n") - flag.PrintDefaults() - } + flag.Usage = func() { printUsage(os.Stderr) } flag.StringVar(&cfg.Host, "h", "", "database host") flag.IntVar(&cfg.Port, "p", DefaultPostgresPort, "database port") @@ -350,6 +390,13 @@ func parseConfig() (*Config, error) { flag.StringVar(&includeRaw, "include", "", "comma-separated default-disabled task names to enable (e.g. pgstattuple, disabled by default)") flag.BoolVar(&cfg.Verbose, "v", false, "verbose output (summary)") flag.BoolVar(&cfg.VeryVerbose, "vv", false, "very verbose output (detailed)") + + // Checked after registration so printUsage can list the options above, + // but before parsing so --help and --version are not parse errors. + if err := globalOption(os.Args[1:]); err != nil { + return nil, err + } + flag.Parse() for _, raw := range strings.Split(excludeRaw, ",") { diff --git a/radar_test.go b/radar_test.go index 7180012..54e3296 100644 --- a/radar_test.go +++ b/radar_test.go @@ -13,6 +13,7 @@ package main import ( "archive/zip" "bytes" + "errors" "flag" "io" "os" @@ -937,6 +938,87 @@ func TestLazyZipWriterNoWrite(t *testing.T) { } } +// TestGlobalOptions tests that --help and --version are recognised before flag +// parsing, and that -h stays bound to the database host. +func TestGlobalOptions(t *testing.T) { + oldArgs := os.Args + defer func() { os.Args = oldArgs }() + + tests := []struct { + name string + args []string + want error + }{ + {"--help", []string{"radar", "--help"}, errHelpRequested}, + {"-help", []string{"radar", "-help"}, errHelpRequested}, + {"--version", []string{"radar", "--version"}, errVersionRequested}, + {"-version", []string{"radar", "-version"}, errVersionRequested}, + {"-V", []string{"radar", "-V"}, errVersionRequested}, + {"help after other flags", []string{"radar", "-d", "testdb", "--help"}, errHelpRequested}, + {"no global option", []string{"radar", "-d", "testdb"}, nil}, + {"-h is the database host", []string{"radar", "-h", "localhost"}, nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + flag.CommandLine = flag.NewFlagSet("radar", flag.ContinueOnError) + os.Args = tt.args + + cfg, err := parseConfig() + if !errors.Is(err, tt.want) { + t.Fatalf("parseConfig() error = %v, want %v", err, tt.want) + } + if tt.want != nil { + return + } + if cfg == nil { + t.Fatal("expected a config when no global option was given") + } + if tt.name == "-h is the database host" && cfg.Host != "localhost" { + t.Errorf("expected host 'localhost', got %q", cfg.Host) + } + }) + } +} + +// TestPrintUsage tests that the help text carries the version and global +// options sections alongside the registered collection flags. +func TestPrintUsage(t *testing.T) { + flag.CommandLine = flag.NewFlagSet("radar", flag.ContinueOnError) + os.Args = []string{"radar", "--help"} + if _, err := parseConfig(); !errors.Is(err, errHelpRequested) { + t.Fatalf("parseConfig() error = %v, want errHelpRequested", err) + } + + var buf bytes.Buffer + printUsage(&buf) + got := buf.String() + + want := []string{ + "Usage: radar [options]", + "VERSION:\n " + version, + "GLOBAL OPTIONS:", + "--help show help", + "--version, -V print the version", + "Options:", + "-sslmode string", + "database host", + } + for _, w := range want { + if !strings.Contains(got, w) { + t.Errorf("usage output missing %q\ngot:\n%s", w, got) + } + } + + // help and version are matched before parsing, so they must not be + // registered as flags and must not appear in the options list. + for _, name := range []string{"help", "version", "V"} { + if f := flag.CommandLine.Lookup(name); f != nil { + t.Errorf("global option -%s must not be a registered flag", name) + } + } +} + // TestPGEnvFallbacks tests PGPORT and PGDATABASE environment variable fallbacks. func TestPGEnvFallbacks(t *testing.T) { oldArgs := os.Args From 634dda7cd9c178f44b84d7982090db3caa46b2b4 Mon Sep 17 00:00:00 2001 From: Zaid-edge Date: Wed, 2 Sep 2026 18:09:22 +0500 Subject: [PATCH 2/3] Fixing Test pull request --- radar.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/radar.go b/radar.go index cba014b..e921099 100644 --- a/radar.go +++ b/radar.go @@ -59,13 +59,20 @@ func globalOption(args []string) error { // printUsage writes the help text: the usage line, the build version, the // global options, then the collection options from the registered flag set. +// The write error is discarded because flag.PrintDefaults, which emits the +// rest of the same output, discards its own. func printUsage(w io.Writer) { - fmt.Fprintf(w, "Usage: radar [options]\n\n") - fmt.Fprintf(w, "VERSION:\n %s\n\n", version) - fmt.Fprintf(w, "GLOBAL OPTIONS:\n") - fmt.Fprintf(w, " --help show help\n") - fmt.Fprintf(w, " --version, -V print the version\n\n") - fmt.Fprintf(w, "Options:\n") + _, _ = fmt.Fprintf(w, `Usage: radar [options] + +VERSION: + %s + +GLOBAL OPTIONS: + --help show help + --version, -V print the version + +Options: +`, version) flag.CommandLine.SetOutput(w) flag.PrintDefaults() } From b93105e75ba922e905a5fae843e3bf3073cc8bba Mon Sep 17 00:00:00 2001 From: Jimmy Angelakos Date: Wed, 2 Sep 2026 19:05:42 +0100 Subject: [PATCH 3/3] fix: make -help outrank -version and keep the build version out of the help text --- radar.go | 47 ++++++++++++--------------------- radar_test.go | 73 +++++++++++++++++++++++++++++++++++---------------- test-radar.sh | 2 ++ 3 files changed, 69 insertions(+), 53 deletions(-) diff --git a/radar.go b/radar.go index e921099..832d983 100644 --- a/radar.go +++ b/radar.go @@ -34,45 +34,28 @@ import ( // Release builds set it to e.g. "v0.5.0"; unstamped dev builds report "dev". var version = "dev" -// errHelpRequested and errVersionRequested are returned by parseConfig when a -// global option was given instead of a collection request. main prints the +// errHelpRequested and errVersionRequested are returned by parseConfig when +// -help or -version was given instead of a collection request. main prints the // corresponding output and exits successfully. var ( errHelpRequested = errors.New("help requested") errVersionRequested = errors.New("version requested") ) -// globalOption reports which global option appears in args, if any. These are -// matched before flag parsing because -h is taken by the database host, so -// help cannot be registered as a regular flag without shadowing it. -func globalOption(args []string) error { - for _, arg := range args { - switch arg { - case "-help", "--help": - return errHelpRequested - case "-version", "--version", "-V": - return errVersionRequested - } - } - return nil -} - -// printUsage writes the help text: the usage line, the build version, the -// global options, then the collection options from the registered flag set. -// The write error is discarded because flag.PrintDefaults, which emits the -// rest of the same output, discards its own. +// printUsage writes the help text: the usage line, the global options, then +// the collection options from the registered flag set. It carries nothing +// build-dependent, because it is pasted verbatim into README.md and +// docs/index.md. The write error is discarded because flag.PrintDefaults, +// which emits the rest of the same output, discards its own. func printUsage(w io.Writer) { - _, _ = fmt.Fprintf(w, `Usage: radar [options] - -VERSION: - %s + _, _ = fmt.Fprint(w, `Usage: radar [options] GLOBAL OPTIONS: --help show help --version, -V print the version Options: -`, version) +`) flag.CommandLine.SetOutput(w) flag.PrintDefaults() } @@ -398,10 +381,14 @@ func parseConfig() (*Config, error) { flag.BoolVar(&cfg.Verbose, "v", false, "verbose output (summary)") flag.BoolVar(&cfg.VeryVerbose, "vv", false, "very verbose output (detailed)") - // Checked after registration so printUsage can list the options above, - // but before parsing so --help and --version are not parse errors. - if err := globalOption(os.Args[1:]); err != nil { - return nil, err + // Read after registration so printUsage can list the options above, and + // before parsing so that neither a collection flag nor a typo elsewhere + // stops radar answering. Help outranks version when both are given. + switch args := os.Args[1:]; { + case slices.Contains(args, "-help"), slices.Contains(args, "--help"): + return nil, errHelpRequested + case slices.Contains(args, "-version"), slices.Contains(args, "--version"), slices.Contains(args, "-V"): + return nil, errVersionRequested } flag.Parse() diff --git a/radar_test.go b/radar_test.go index 54e3296..ec5493b 100644 --- a/radar_test.go +++ b/radar_test.go @@ -938,25 +938,39 @@ func TestLazyZipWriterNoWrite(t *testing.T) { } } -// TestGlobalOptions tests that --help and --version are recognised before flag -// parsing, and that -h stays bound to the database host. +// TestGlobalOptions tests that -help and -version are answered whatever else +// is on the command line, that no other flag takes effect when they are, and +// that -help outranks -version when both are given. func TestGlobalOptions(t *testing.T) { oldArgs := os.Args defer func() { os.Args = oldArgs }() tests := []struct { - name string - args []string - want error + name string + args []string + want error + check func(*testing.T, *Config) }{ - {"--help", []string{"radar", "--help"}, errHelpRequested}, - {"-help", []string{"radar", "-help"}, errHelpRequested}, - {"--version", []string{"radar", "--version"}, errVersionRequested}, - {"-version", []string{"radar", "-version"}, errVersionRequested}, - {"-V", []string{"radar", "-V"}, errVersionRequested}, - {"help after other flags", []string{"radar", "-d", "testdb", "--help"}, errHelpRequested}, - {"no global option", []string{"radar", "-d", "testdb"}, nil}, - {"-h is the database host", []string{"radar", "-h", "localhost"}, nil}, + {name: "--help", args: []string{"radar", "--help"}, want: errHelpRequested}, + {name: "-help", args: []string{"radar", "-help"}, want: errHelpRequested}, + {name: "--version", args: []string{"radar", "--version"}, want: errVersionRequested}, + {name: "-version", args: []string{"radar", "-version"}, want: errVersionRequested}, + {name: "-V", args: []string{"radar", "-V"}, want: errVersionRequested}, + {name: "help outranks version", args: []string{"radar", "--version", "--help"}, want: errHelpRequested}, + {name: "help outranks version whatever the order", args: []string{"radar", "--help", "--version"}, want: errHelpRequested}, + {name: "help outranks an unparseable flag", args: []string{"radar", "-nosuchflag", "--help"}, want: errHelpRequested}, + {name: "version outranks an unparseable flag", args: []string{"radar", "-nosuchflag", "-V"}, want: errVersionRequested}, + {name: "help outranks a collection flag", args: []string{"radar", "-d", "testdb", "--help"}, want: errHelpRequested}, + {name: "neither given", args: []string{"radar", "-d", "testdb"}}, + { + name: "-h is the database host", + args: []string{"radar", "-h", "localhost"}, + check: func(t *testing.T, cfg *Config) { + if cfg.Host != "localhost" { + t.Errorf("host = %q, want %q", cfg.Host, "localhost") + } + }, + }, } for _, tt := range tests { @@ -969,21 +983,28 @@ func TestGlobalOptions(t *testing.T) { t.Fatalf("parseConfig() error = %v, want %v", err, tt.want) } if tt.want != nil { + if cfg != nil { + t.Errorf("expected no config alongside %v, got %+v", tt.want, cfg) + } return } if cfg == nil { - t.Fatal("expected a config when no global option was given") + t.Fatal("expected a config when neither option was given") } - if tt.name == "-h is the database host" && cfg.Host != "localhost" { - t.Errorf("expected host 'localhost', got %q", cfg.Host) + if tt.check != nil { + tt.check(t, cfg) } }) } } -// TestPrintUsage tests that the help text carries the version and global -// options sections alongside the registered collection flags. +// TestPrintUsage tests that the help text lists the global options above the +// collection flags. It must carry nothing build-dependent, because it is +// pasted verbatim into README.md and docs/index.md. func TestPrintUsage(t *testing.T) { + oldArgs := os.Args + defer func() { os.Args = oldArgs }() + flag.CommandLine = flag.NewFlagSet("radar", flag.ContinueOnError) os.Args = []string{"radar", "--help"} if _, err := parseConfig(); !errors.Is(err, errHelpRequested) { @@ -996,7 +1017,6 @@ func TestPrintUsage(t *testing.T) { want := []string{ "Usage: radar [options]", - "VERSION:\n " + version, "GLOBAL OPTIONS:", "--help show help", "--version, -V print the version", @@ -1010,13 +1030,20 @@ func TestPrintUsage(t *testing.T) { } } - // help and version are matched before parsing, so they must not be - // registered as flags and must not appear in the options list. + if strings.Contains(got, version) { + t.Errorf("usage output must not carry the build version\ngot:\n%s", got) + } + + // The global options are answered before parsing, so registering them + // would list a second, unreachable copy among the collection flags. for _, name := range []string{"help", "version", "V"} { - if f := flag.CommandLine.Lookup(name); f != nil { - t.Errorf("global option -%s must not be a registered flag", name) + if flag.CommandLine.Lookup(name) != nil { + t.Errorf("-%s must not be a registered flag", name) } } + if f := flag.CommandLine.Lookup("h"); f == nil || f.Usage != "database host" { + t.Error("-h must stay bound to the database host") + } } // TestPGEnvFallbacks tests PGPORT and PGDATABASE environment variable fallbacks. diff --git a/test-radar.sh b/test-radar.sh index 0bff0eb..37664bd 100755 --- a/test-radar.sh +++ b/test-radar.sh @@ -18,6 +18,8 @@ NC='\033[0m' # Binary is pre-built by run-ci-local.sh and copied into container by Dockerfile echo "Using pre-built radar binary..." ./radar --help > /dev/null 2>&1 || { echo -e "${RED}✗ radar binary not found or not executable${NC}"; exit 1; } +./radar --help | grep -q '^Usage: radar \[options\]$' || { echo -e "${RED}✗ radar --help did not write usage to stdout${NC}"; exit 1; } +./radar --version | grep -q '^radar version ' || { echo -e "${RED}✗ radar --version did not report a version${NC}"; exit 1; } echo "" echo "Initializing PostgreSQL 18..."