Minor refactor of internal/{outfmt,pager,executil}

internal/executil/command.go

@@ -2,14 +2,17 @@
 // by the internal/open and internal/pager packages.
 package executil
 
-import "os/exec"
+import (
+	"fmt"
+	"os/exec"
+)
 
 // Command returns an exec.Cmd for the given args after first calling
 // exec.LookPath on the first argument.
 func Command(args ...string) (*exec.Cmd, error) {
 	cmdPath, err := exec.LookPath(args[0])
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("executil: exec.LookPath: %w", err)
 	}
 	return exec.Command(cmdPath, args[1:]...), nil
 }

internal/outfmt/formatter.go

@@ -6,6 +6,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"log"
 	"os"
 
 	"github.com/charmbracelet/glamour"
@@ -48,10 +49,9 @@ func IsRichMarkdown() bool {
 }
 
 // Formatter returns output wrapped with a term formatter if -fmt=term.
-func Formatter(output io.Writer) (io.WriteCloser, error) {
-	fallback := ioutil.WriteNopCloser(output)
+func Formatter(out io.WriteCloser) io.WriteCloser {
 	if Format != Term {
-		return fallback, nil
+		return out
 	}
 
 	styleOpt := glamour.WithAutoStyle()
@@ -67,24 +67,35 @@ func Formatter(output io.Writer) (io.WriteCloser, error) {
 		glamour.WithWordWrap(0),
 	)
 	if err != nil {
-		return fallback, err
+		log.Printf("failed to use output format %s: %v", Format, err)
+		return out
 	}
 
 	// Modify the style to make it more consistent with standard go doc
 	// text output.
-	err = json.Unmarshal(stylePatchData, &rdr.AnsiOptions.Styles)
-	if err != nil {
-		return fallback, err
+	if err := json.Unmarshal(stylePatchData, &rdr.AnsiOptions.Styles); err != nil {
+		log.Printf("failed to use output format %s: %v", Format, err)
+		return out
 	}
 	rdr.AnsiOptions.Styles.CodeBlock.Theme = SyntaxStyle
 
 	return ioutil.WriteCloserFunc(rdr, func() error {
+		// Close the renderer after writing all input. This triggers
+		// the final conversion to the output format.
 		if err := rdr.Close(); err != nil {
-			return err
+			return fmt.Errorf("outfmt: render: %w", err)
+		}
+		// Copy the formatted output to the originally given output
+		// writer.
+		if _, err := io.Copy(out, rdr); err != nil {
+			return fmt.Errorf("outfmt: copy: %w", err)
+		}
+		// Finally close the original output writer.
+		if err := out.Close(); err != nil {
+			return fmt.Errorf("outfmt: close: %w", err)
 		}
-		_, err := io.Copy(output, rdr)
 		return err
-	}), err
+	})
 }
 
 //go:embed style-patch.json

internal/outfmt/output.go

@@ -2,37 +2,21 @@ package outfmt
 
 import (
 	"io"
-	"log"
 
 	"aslevy.com/go-doc/internal/completion"
 	"aslevy.com/go-doc/internal/ioutil"
 	"aslevy.com/go-doc/internal/open"
 	"aslevy.com/go-doc/internal/pager"
 )
 
-func Output(w io.Writer) io.WriteCloser {
-	fallback := ioutil.WriteNopCloser(w)
+func Output(out io.Writer) io.WriteCloser {
 	if open.Requested || completion.Requested {
-		return fallback
+		return ioutil.WriteNopCloser(out)
 	}
 
 	// Set up pager and output format writers.
-	pgr, err := pager.Pager(w)
-	if err != nil {
-		log.Println("failed to use pager:", err)
-	} else {
-		fallback = pgr
-	}
-
-	fmtr, err := Formatter(fallback)
-	if err != nil {
-		log.Printf("failed to use output format %s: %v", Format, err)
-	} else {
-		fallback = fmtr
-	}
+	pgr := pager.Pager(out)
+	fmtr := Formatter(pgr)
 
-	return ioutil.WriteCloserFunc(fallback, func() error {
-		fmtr.Close()
-		return pgr.Close()
-	})
+	return fmtr
 }

internal/pager/pager.go

@@ -4,6 +4,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"log"
 	"os"
 	"strings"
 
@@ -35,37 +36,48 @@ func AddFlags(fs *flag.FlagSet) {
 // else PAGER. If no pager is set in the environment 'less -R' is used.
 //
 // If the pager fails to start, the output is returned directly with the error.
-func Pager(output io.Writer) (io.WriteCloser, error) {
+func Pager(out io.Writer) io.WriteCloser {
 	pager := getPagerEnv()
 	if pager == "-" {
 		Disabled = true
 	}
 
-	fallback := ioutil.WriteNopCloser(output)
-	if Disabled || !IsTTY(output) || open.Requested {
-		return fallback, nil
+	outNopCloser := ioutil.WriteNopCloser(out)
+	if Disabled || open.Requested || !IsTTY(out) {
+		return outNopCloser
 	}
 
-	pagerCmd, err := executil.Command(getPagerEnv())
+	pagerArgs := make([]string, 1, 2)
+	pagerArgs[0] = pager
+	if pager == "less" {
+		pagerArgs = append(pagerArgs, "-RF")
+	}
+
+	pagerCmd, err := executil.Command(pagerArgs...)
 	if err != nil {
-		return fallback, err
+		log.Println("pager:", err)
+		return outNopCloser
 	}
-	pagerCmd.Stdout = output
+	pagerCmd.Stdout = out
 	pagerCmd.Stderr = os.Stderr
 
 	pagerStdin, err := pagerCmd.StdinPipe()
 	if err != nil {
-		return fallback, fmt.Errorf("failed to obtain stdin pipe for pager: %w", err)
+		log.Println("pager:", fmt.Errorf("stdin pipe: %w", err))
+		return outNopCloser
 	}
 
 	if err := pagerCmd.Start(); err != nil {
-		return fallback, fmt.Errorf("failed to start pager: %w", err)
+		log.Println("pager:", fmt.Errorf("start: %w", err))
+		return outNopCloser
 	}
 
 	return ioutil.WriteCloserFunc(pagerStdin, func() error {
-		pagerStdin.Close()
+		if err := pagerStdin.Close(); err != nil {
+			log.Println("pager: close stdin pipe:", err)
+		}
 		return pagerCmd.Wait()
-	}), nil
+	})
 }
 
 func getPagerEnv() string {
@@ -84,8 +96,8 @@ func getPagerEnv() string {
 
 // IsTTY returns true if output is a terminal, as opposed to a pipe, or some
 // other buffer.
-func IsTTY(output io.Writer) bool {
-	f, ok := output.(*os.File)
+func IsTTY(out io.Writer) bool {
+	f, ok := out.(*os.File)
 	if !ok {
 		return false
 	}