fix(Config): colorization without pager (#687)

Fix an issue whereby colorization would output ANSI codes if a pager was
not configured.

The solution here is to stop guessing about the state of the user's
system at runtime, as well as the user's intention. The installer now
chooses an appropriate installer when generating configs, and no longer
bothers searching for pagers at runtime.

cmd/cheat/str_config.go

@@ -24,7 +24,9 @@ colorize: false
 formatter: terminal
 
 # Through which pager should output be piped?
-# pager: less -FRX # <- recommended where available
+# 'less -FRX' is recommended on Unix systems
+# 'more' is recommended on Windows
+pager: PAGER_PATH
 
 # The paths at which cheatsheets are available. Tags associated with a cheatpath
 # are automatically attached to all cheatsheets residing on that path.

configs/conf.yml

@@ -15,7 +15,9 @@ colorize: false
 formatter: terminal
 
 # Through which pager should output be piped?
-# pager: less -FRX # <- recommended where available
+# 'less -FRX' is recommended on Unix systems
+# 'more' is recommended on Windows
+pager: PAGER_PATH
 
 # The paths at which cheatsheets are available. Tags associated with a cheatpath
 # are automatically attached to all cheatsheets residing on that path.

internal/config/config.go

@@ -3,9 +3,7 @@ package config
 import (
 	"fmt"
 	"os"
-	"os/exec"
 	"path/filepath"
-	"runtime"
 	"strings"
 
 	cp "github.com/cheat/cheat/internal/cheatpath"
@@ -93,28 +91,11 @@ func New(opts map[string]interface{}, confPath string, resolve bool) (Config, er
 		conf.Cheatpaths[i].Path = expanded
 	}
 
-	// if an editor was not provided in the configs, look to envvars
+	// if an editor was not provided in the configs, attempt to choose one
+	// that's appropriate for the environment
 	if conf.Editor == "" {
-		if os.Getenv("VISUAL") != "" {
-			conf.Editor = os.Getenv("VISUAL")
-		} else if os.Getenv("EDITOR") != "" {
-			conf.Editor = os.Getenv("EDITOR")
-		} else if runtime.GOOS == "windows" {
-			conf.Editor = "notepad"
-		} else {
-			// try to fall back to `nano`
-			path, err := exec.LookPath("nano")
-			if err != nil {
-				return Config{}, fmt.Errorf("failed to locate nano: %s", err)
-			}
-
-			// use `nano` if we found it
-			if path != "" {
-				conf.Editor = "nano"
-				// otherwise, give up
-			} else {
-				return Config{}, fmt.Errorf("no editor set")
-			}
+		if conf.Editor, err = Editor(); err != nil {
+			return Config{}, err
 		}
 	}
 
@@ -128,39 +109,8 @@ func New(opts map[string]interface{}, confPath string, resolve bool) (Config, er
 		conf.Formatter = "terminal"
 	}
 
-	// attempt to fall back to `PAGER` if a pager is not specified in configs
+	// load the pager
 	conf.Pager = strings.TrimSpace(conf.Pager)
-	if conf.Pager == "" {
-		// look for `pager`, `less`, and `more` on the system PATH
-		pagerPath, _ := exec.LookPath("pager")
-		lessPath, _ := exec.LookPath("less")
-		morePath, _ := exec.LookPath("more")
-
-		// search first for a `PAGER` envvar
-		if os.Getenv("PAGER") != "" {
-			conf.Pager = os.Getenv("PAGER")
-
-			// search for `pager`
-		} else if pagerPath != "" {
-			conf.Pager = pagerPath
-
-			// search for `less`
-		} else if lessPath != "" {
-			conf.Pager = lessPath
-
-			// search for `more`
-			//
-			// XXX: this causes issues on some Linux systems. See:
-			// https://github.com/cheat/cheat/issues/681#issuecomment-1201842334
-			//
-			// By checking for `more` last, we're hoping to at least mitigate
-			// the frequency of this occurrence, because `pager` and `less` are
-			// likely to be available on most systems on which a user is likely
-			// to have installed `cheat`.
-		} else if morePath != "" {
-			conf.Pager = morePath
-		}
-	}
 
 	return conf, nil
 }

internal/config/editor.go

@@ -0,0 +1,30 @@
+package config
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"runtime"
+)
+
+// Editor attempts to locate an editor that's appropriate for the environment.
+func Editor() (string, error) {
+
+	// default to `notepad.exe` on Windows
+	if runtime.GOOS == "windows" {
+		return "notepad", nil
+	}
+
+	// look for `nano` on the `PATH`
+	nano, _ := exec.LookPath("nano")
+
+	// search for `$VISUAL`, `$EDITOR`, and then `nano`, in that order
+	for _, editor := range []string{os.Getenv("VISUAL"), os.Getenv("EDITOR"), nano} {
+		if editor != "" {
+			return editor, nil
+		}
+	}
+
+	// return an error if no path is found
+	return "", fmt.Errorf("no editor set")
+}

internal/config/pager.go

@@ -0,0 +1,26 @@
+package config
+
+import (
+	"os"
+	"os/exec"
+)
+
+// Pager attempts to locate a pager that's appropriate for the environment.
+func Pager() string {
+
+	// if $PAGER is set, return the corresponding pager
+	if os.Getenv("PAGER") != "" {
+		return os.Getenv("PAGER")
+	}
+
+	// Otherwise, search for `pager`, `less`, and `more` on the `$PATH`. If
+	// none are found, return an empty pager.
+	for _, pager := range []string{"pager", "less", "more"} {
+		if path, err := exec.LookPath(pager); err != nil {
+			return path
+		}
+	}
+
+	// default to no pager
+	return ""
+}

internal/installer/run.go

@@ -23,6 +23,7 @@ func Run(configs string, confpath string) error {
 	// template the above paths into the default configs
 	configs = strings.Replace(configs, "COMMUNITY_PATH", community, -1)
 	configs = strings.Replace(configs, "PERSONAL_PATH", personal, -1)
+	configs = strings.Replace(configs, "PAGER_PATH", config.Pager(), -1)
 
 	// prompt the user to download the community cheatsheets
 	yes, err := Prompt(