From 523879bb0f68d895599d50a50dbf98e741b2721b Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 13 Nov 2026 21:55:05 +0100 Subject: [PATCH] Print help that was asked for to stdout (closes #179) --help, -h and the help subcommand now print to stdout and exit 0, so the help can be paged and searched. The usage text of a usage error stays on stderr with exit 2, as does help for an unknown subcommand ("sfdupes help scna"). A failed write of the help exits 1. Cobra prints help to the command's output writer, which stays stderr, drops a failed write, and returns no error after help. So run replaces the help function and the help subcommand: the help goes into a buffer, then to stdout, and the error sets the exit code. README "Settings" and goal 4 say so. Model: opus-5-5 --- README.md | 18 +++-- internal/sfdupes/cli.go | 82 +++++++++++++++++++- internal/sfdupes/cli_test.go | 127 +++++++++++++++++++++++++------ internal/sfdupes/dwim_test.go | 14 ++-- internal/sfdupes/exclude_test.go | 4 +- 5 files changed, 202 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index 354a9c2..023b8e6 100644 --- a/README.md +++ b/README.md @@ -253,8 +253,9 @@ In order: `trees` check only that each copy they name still exists, and leave out a copy that is gone. `scan` is built to run from `cron`. The reports run at any time, against what the database holds. -4. **Keep the streams apart.** Everything on stdout is machine-readable data. - Everything for a human reader, such as progress and warnings, goes to stderr. +4. **Keep the streams apart.** stdout holds only what was asked for: + machine-readable data, or help. Everything else for a human reader, such as + progress and warnings, goes to stderr. ### Subcommands @@ -324,12 +325,13 @@ The reference below is also the program's specification. "Operands"). - `--version`, `-v` prints `sfdupes VERSION` (see "Install") to stdout and exits 0. It writes nothing to stderr. -- `--help`, `-h`, alone or after a subcommand, prints the help text to stderr - and exits 0. It writes nothing to stdout. The help of `sfdupes` and of `scan` - names the database. The help of `report`, `trees`, `info` and `dwim` says what - each prints. `sfdupes scan --help` and `sfdupes dwim --help` also list the - default excludes of the system they run on, and the filesystem types the walk - never records anything on. +- `--help`, `-h`, alone or after a subcommand, prints the help text to stdout + and exits 0. It writes nothing to stderr. `sfdupes help` and + `sfdupes help COMMAND` do the same. The help of `sfdupes` and of `scan` names + the database. The help of `report`, `trees`, `info` and `dwim` says what each + prints. `sfdupes scan --help` and `sfdupes dwim --help` also list the default + excludes of the system they run on, and the filesystem types the walk never + records anything on. ### Duplicate detection diff --git a/internal/sfdupes/cli.go b/internal/sfdupes/cli.go index d994c21..1be133b 100644 --- a/internal/sfdupes/cli.go +++ b/internal/sfdupes/cli.go @@ -3,6 +3,7 @@ package sfdupes import ( + "bytes" "context" "errors" "fmt" @@ -176,9 +177,32 @@ func run(args []string, stdout, stderr io.Writer) int { } root := newRootCommand(stdout, stderr) + + // Help that was asked for, with --help, -h or the help subcommand, + // goes to stdout. Execute returns no error after help, so helpErr + // keeps the one printHelp returns. Cobra's own help subcommand is + // replaced because it answers "sfdupes help scna" with the help of + // sfdupes. + var helpErr error + + cobraHelp := root.HelpFunc() + root.SetHelpFunc(func(cmd *cobra.Command, _ []string) { + helpErr = printHelp(cmd, cmd.Flags().Args(), cobraHelp, stdout) + }) + root.SetHelpCommand(&cobra.Command{ + Use: "help [command]", + Short: "Help about any command", + Run: func(_ *cobra.Command, names []string) { + helpErr = printHelp(root, names, cobraHelp, stdout) + }, + }) + root.SetArgs(args) err := root.Execute() + if err == nil { + err = helpErr + } var fatal fatalError @@ -195,14 +219,62 @@ func run(args []string, stdout, stderr io.Writer) int { return exitFatal default: - // A usage error, which cobra has already reported on stderr. + // A usage error, which cobra or printHelp has already reported + // on stderr. return exitUsage } } +// printHelp prints help that was asked for to stdout. It is the help of +// cmd, or, for a command with subcommands, of the subcommand that +// operands name: scan in "sfdupes help scan". Operands that name none, +// as in "sfdupes help scna" or "sfdupes scna --help", are a usage +// error, reported on stderr as for "sfdupes scna". +// +// cobraHelp is cobra's own help function. It prints to the command's +// output writer, which is stderr, and drops a failed write. So the help +// goes into a buffer first, and a failed write of the buffer to stdout +// is a fatal error. +func printHelp( + cmd *cobra.Command, operands []string, + cobraHelp func(*cobra.Command, []string), stdout io.Writer, +) error { + if cmd.HasSubCommands() { + cmd, operands, _ = cmd.Find(operands) + } + + // When the help subcommand runs instead of cmd, cobra has not yet + // added the --help flag that cmd's help and usage text list. + cmd.InitDefaultHelpFlag() + + // Operands left over here name no subcommand. They are reported as + // cobra reports "sfdupes scna", which cobra.NoArgs rejects. + if cmd.HasSubCommands() && len(operands) > 0 { + err := cobra.NoArgs(cmd, operands) + cmd.PrintErrln(cmd.ErrPrefix(), err.Error()) + cmd.Println(cmd.UsageString()) + + return err + } + + out := cmd.OutOrStderr() + help := new(bytes.Buffer) + cmd.SetOut(help) + cobraHelp(cmd, nil) + cmd.SetOut(out) + + _, err := stdout.Write(help.Bytes()) + if err != nil { + return fatalError{err: fmt.Errorf("write stdout: %w", err)} + } + + return nil +} + // newRootCommand builds the command tree. Everything on stdout is -// machine-readable data, the version line included; all human-facing -// output (help, usage, errors) goes to stderr. +// machine-readable data, the version line included, or help that was +// asked for, which run prints. The rest of the human-facing output +// (usage, errors) goes to stderr. func newRootCommand(stdout, stderr io.Writer) *cobra.Command { var showVersion bool @@ -235,11 +307,13 @@ func newRootCommand(stdout, stderr io.Writer) *cobra.Command { return errNoSubcommand }, } + // Cobra prints the usage text of a usage error to the command's + // output writer, so that writer is stderr. root.SetOut(stderr) root.SetErr(stderr) root.CompletionOptions.DisableDefaultCmd = true - // Cobra's built-in version flag prints through the help writer, + // Cobra's built-in version flag prints to the output writer, // stderr; this one prints to stdout. root.Flags().BoolVarP(&showVersion, "version", "v", false, "print the version to stdout") diff --git a/internal/sfdupes/cli_test.go b/internal/sfdupes/cli_test.go index cb7c653..8cb93f5 100644 --- a/internal/sfdupes/cli_test.go +++ b/internal/sfdupes/cli_test.go @@ -26,6 +26,9 @@ const usageMarker = "Usage:" // helpFlag asks for the help text. const helpFlag = "--help" +// helpCommand is the subcommand that prints the help text. +const helpCommand = "help" + // workersFlag is the scan and dwim flag that sets the worker count. const workersFlag = "--workers" @@ -625,6 +628,14 @@ func TestRunNamesDefaultDatabase(t *testing.T) { func TestRunUsageErrors(t *testing.T) { // Usage errors keep exiting 2 with cobra's own report on stderr. + // An unknown subcommand is reported the same way with or without a + // request for help: the line naming it, then the usage text. + const ( + unknown = "nope" + unknownReport = "unknown command \"" + unknown + + "\" for \"sfdupes\"\n" + usageMarker + ) + cases := map[string]struct { args []string want string @@ -634,7 +645,13 @@ func TestRunUsageErrors(t *testing.T) { "info with args": {[]string{cmdInfo, "x"}, usageMarker}, "dwim with args": {[]string{cmdDwim, "x"}, usageMarker}, "unknown flag": {[]string{cmdScan, "--nope", "/"}, usageMarker}, - "unknown subcommand": {[]string{"nope"}, "unknown command"}, + "unknown subcommand": {[]string{unknown}, unknownReport}, + "unknown subcommand with --help": { + []string{unknown, helpFlag}, unknownReport, + }, + "help for an unknown subcommand": { + []string{helpCommand, unknown}, unknownReport, + }, } for name, tc := range cases { @@ -700,9 +717,12 @@ func TestRunScanRejectsWorkersBelowOne(t *testing.T) { func TestRunHelp(t *testing.T) { t.Parallel() - // README §Settings: help goes to stderr, exits 0, and leaves stdout + // README §Settings: help goes to stdout, exits 0, and leaves stderr // empty. - cases := [][]string{{helpFlag}, {"-h"}, {cmdScan, helpFlag}} + cases := [][]string{ + {helpFlag}, {"-h"}, {helpCommand}, + {cmdScan, helpFlag}, {helpCommand, cmdScan}, + } for _, args := range cases { var stdout, stderr bytes.Buffer @@ -712,14 +732,20 @@ func TestRunHelp(t *testing.T) { t.Errorf("run(%v) = %d, want %d", args, code, exitOK) } - if !strings.Contains(stderr.String(), usageMarker) { - t.Errorf("run(%v) stderr = %q, want the help text", - args, stderr.String()) + if !strings.Contains(stdout.String(), usageMarker) { + t.Errorf("run(%v) stdout = %q, want the help text", + args, stdout.String()) } - if got := stdout.String(); got != "" { - t.Errorf("run(%v) stdout = %q, want nothing (data only)", - args, got) + // The help subcommand prints the same help as --help, which + // lists --help itself. + if !strings.Contains(stdout.String(), "-h, "+helpFlag) { + t.Errorf("run(%v) stdout = %q, want it to list %s", + args, stdout.String(), helpFlag) + } + + if got := stderr.String(); got != "" { + t.Errorf("run(%v) stderr = %q, want nothing", args, got) } // README §Location: the help of sfdupes and of scan says where @@ -729,9 +755,9 @@ func TestRunHelp(t *testing.T) { "scan and dwim store", databaseEnv, rootDatabasePath, stateHomeEnv, "~/.local/state", "not an absolute path is an error", } { - if !strings.Contains(stderr.String(), want) { - t.Errorf("run(%v) stderr = %q, want it to name %q", - args, stderr.String(), want) + if !strings.Contains(stdout.String(), want) { + t.Errorf("run(%v) stdout = %q, want it to name %q", + args, stdout.String(), want) } } } @@ -741,17 +767,48 @@ func TestRunHelp(t *testing.T) { run([]string{cmdScan, "-h"}, &stdout, &stderr) - if !strings.Contains(stderr.String(), excludeFlag+" PATH") { - t.Errorf("scan -h stderr = %q, want %s PATH", stderr.String(), + if !strings.Contains(stdout.String(), excludeFlag+" PATH") { + t.Errorf("scan -h stdout = %q, want %s PATH", stdout.String(), excludeFlag) } } +func TestRunHelpScanPrintsScanHelp(t *testing.T) { + t.Parallel() + + // "sfdupes help scan" and "sfdupes --help scan" print exactly what + // "sfdupes scan --help" prints: the help of scan, not of sfdupes. + var want bytes.Buffer + + run([]string{cmdScan, helpFlag}, &want, io.Discard) + + for _, args := range [][]string{ + {helpCommand, cmdScan}, {helpFlag, cmdScan}, + } { + var stdout, stderr bytes.Buffer + + code := run(args, &stdout, &stderr) + if code != exitOK { + t.Errorf("run(%v) = %d, want %d", args, code, exitOK) + } + + if got := stdout.String(); got != want.String() { + t.Errorf("run(%v) stdout = %q, want what scan %s prints, %q", + args, got, helpFlag, want.String()) + } + + if got := stderr.String(); got != "" { + t.Errorf("run(%v) stderr = %q, want nothing", args, got) + } + } +} + func TestRunScanUsageLine(t *testing.T) { t.Parallel() // README §Subcommands shows this usage line for scan, and the help - // and the usage error both print it. + // and the usage error both print it. TestRunHelp and + // TestRunUsageErrors check which stream each goes to. const want = "\n sfdupes scan [--workers N] [-x] [--no-default-excludes] " + "[--exclude PATH]... PATH...\n" @@ -764,13 +821,13 @@ func TestRunScanUsageLine(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - var stdout, stderr bytes.Buffer + var output bytes.Buffer - run(args, &stdout, &stderr) + run(args, &output, &output) - if !strings.Contains(stderr.String(), want) { - t.Errorf("run(%v) stderr = %q, want the usage line %q", - args, stderr.String(), want) + if !strings.Contains(output.String(), want) { + t.Errorf("run(%v) output = %q, want the usage line %q", + args, output.String(), want) } }) } @@ -836,9 +893,9 @@ func TestRunHelpDescribesOutput(t *testing.T) { } for _, want := range tc.want { - if !strings.Contains(stderr.String(), want) { + if !strings.Contains(stdout.String(), want) { t.Errorf("run(%v) help lacks %q:\n%s", - args, want, stderr.String()) + args, want, stdout.String()) } } } @@ -922,6 +979,32 @@ func TestRunVersionWriteFailureIsFatal(t *testing.T) { } } +func TestRunHelpWriteFailureIsFatal(t *testing.T) { + t.Parallel() + + // README §Error handling: a failed write to stdout is a fatal error, + // exit 1, reported on stderr starting with "sfdupes:". Help goes to + // stdout, so that holds for help too. + cases := [][]string{ + {helpFlag}, {"-h"}, {helpCommand}, + {cmdScan, helpFlag}, {helpCommand, cmdScan}, + } + + for _, args := range cases { + var stderr bytes.Buffer + + code := run(args, failingWriter{}, &stderr) + if code != exitFatal { + t.Errorf("run(%v) = %d, want %d", args, code, exitFatal) + } + + want := "sfdupes: write stdout: " + errWriteFailed.Error() + "\n" + if got := stderr.String(); got != want { + t.Errorf("run(%v) stderr = %q, want %q", args, got, want) + } + } +} + // scanFixture builds a small tree holding one duplicate pair and one // unreadable file and scans it into the database the caller has // pointed SFDUPES_DATABASE at. The unreadable file makes the scan warn diff --git a/internal/sfdupes/dwim_test.go b/internal/sfdupes/dwim_test.go index d903c45..acca2fd 100644 --- a/internal/sfdupes/dwim_test.go +++ b/internal/sfdupes/dwim_test.go @@ -190,21 +190,21 @@ func TestRunDwimHelpListsScanFlags(t *testing.T) { run([]string{cmdDwim, "-h"}, &stdout, &stderr) - if !strings.Contains(stderr.String(), want) { - t.Errorf("dwim -h stderr = %q, want the usage line %q", - stderr.String(), want) + if !strings.Contains(stdout.String(), want) { + t.Errorf("dwim -h stdout = %q, want the usage line %q", + stdout.String(), want) } - if !strings.Contains(stderr.String(), excludeFlag+" PATH") { - t.Errorf("dwim -h stderr = %q, want %s PATH", stderr.String(), + if !strings.Contains(stdout.String(), excludeFlag+" PATH") { + t.Errorf("dwim -h stdout = %q, want %s PATH", stdout.String(), excludeFlag) } // README "Settings": the help lists the default excludes and the // filesystem types the walk never records anything on. help := strings.TrimSpace(defaultExcludesHelp(runtime.GOOS)) - if !strings.Contains(stderr.String(), help) { - t.Errorf("dwim -h stderr = %q, want it to hold %q", stderr.String(), + if !strings.Contains(stdout.String(), help) { + t.Errorf("dwim -h stdout = %q, want it to hold %q", stdout.String(), help) } } diff --git a/internal/sfdupes/exclude_test.go b/internal/sfdupes/exclude_test.go index 062950e..afbd0f9 100644 --- a/internal/sfdupes/exclude_test.go +++ b/internal/sfdupes/exclude_test.go @@ -196,8 +196,8 @@ func TestDefaultExcludesHelp(t *testing.T) { run([]string{cmdScan, "-h"}, &stdout, &stderr) help := strings.TrimSpace(defaultExcludesHelp(runtime.GOOS)) - if !strings.Contains(stderr.String(), help) { - t.Errorf("scan --help = %q, want it to hold %q", stderr.String(), help) + if !strings.Contains(stdout.String(), help) { + t.Errorf("scan --help = %q, want it to hold %q", stdout.String(), help) } }