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 text can be paged and searched. The usage text of a usage
error stays on stderr with exit 2. Cobra prints both through the
command's output writer, which stays stderr; the help function points
it at stdout before printing.

README "Settings" and goal 4 say so, and the help tests read stdout.

Model: opus-5-5

README.md

@@ -253,9 +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.** stdout holds only what was asked for:
+4. **Keep the streams apart.** Everything on stdout is machine-readable data.
-   machine-readable data, or help. Everything else for a human reader, such as
+   Everything for a human reader, such as progress and warnings, goes to stderr.
-   progress and warnings, goes to stderr.
+   The one exception is help that was asked for, which goes to stdout.
 
 ### Subcommands
 

internal/sfdupes/cli.go

@@ -3,7 +3,6 @@
 package sfdupes
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -177,32 +176,9 @@ 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
 
@@ -219,62 +195,15 @@ func run(args []string, stdout, stderr io.Writer) int {
 
 		return exitFatal
 	default:
-		// A usage error, which cobra or printHelp has already reported
+		// A usage error, which cobra has already reported on stderr.
-		// 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, or help that was
-// asked for, which run prints. The rest of the human-facing output
+// asked for. The rest of the human-facing output (usage, errors) goes
-// (usage, errors) goes to stderr.
+// to stderr.
 func newRootCommand(stdout, stderr io.Writer) *cobra.Command {
 	var showVersion bool
 
@@ -313,6 +242,15 @@ func newRootCommand(stdout, stderr io.Writer) *cobra.Command {
 	root.SetErr(stderr)
 	root.CompletionOptions.DisableDefaultCmd = true
 
+	// Cobra's default help also prints to the output writer. Help that
+	// was asked for, with --help, -h or the help subcommand, goes to
+	// stdout instead.
+	printHelp := root.HelpFunc()
+	root.SetHelpFunc(func(cmd *cobra.Command, args []string) {
+		cmd.SetOut(stdout)
+		printHelp(cmd, args)
+	})
+
 	// Cobra's built-in version flag prints to the output writer,
 	// stderr; this one prints to stdout.
 	root.Flags().BoolVarP(&showVersion, "version", "v", false,

internal/sfdupes/cli_test.go

@@ -628,14 +628,6 @@ 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
@@ -645,13 +637,7 @@ 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{unknown}, unknownReport},
+		"unknown subcommand": {[]string{"nope"}, "unknown command"},
-		"unknown subcommand with --help": {
-			[]string{unknown, helpFlag}, unknownReport,
-		},
-		"help for an unknown subcommand": {
-			[]string{helpCommand, unknown}, unknownReport,
-		},
 	}
 
 	for name, tc := range cases {
@@ -737,13 +723,6 @@ func TestRunHelp(t *testing.T) {
 				args, stdout.String())
 		}
 
-		// 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)
 		}
@@ -773,36 +752,6 @@ func TestRunHelp(t *testing.T) {
 	}
 }
 
-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()
 
@@ -979,32 +928,6 @@ 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