fix: Improve support for pager (#503)

Author: ankitpokhrel

internal/view/issues.go

@@ -36,7 +36,7 @@ type IssueList struct {
 
 // Render renders the view.
 func (l *IssueList) Render() error {
-	if l.Display.Plain {
+	if tui.IsDumbTerminal() || l.Display.Plain {
 		w := tabwriter.NewWriter(os.Stdout, 0, tabWidth, 1, '\t', 0)
 		return l.renderPlain(w)
 	}
@@ -147,7 +147,7 @@ func (l *IssueList) data() tui.TableData {
 	return data
 }
 
-func (IssueList) assignColumns(columns []string, issue *jira.Issue) []string {
+func (*IssueList) assignColumns(columns []string, issue *jira.Issue) []string {
 	var bucket []string
 
 	for _, column := range columns {

internal/view/sprint.go

@@ -76,7 +76,7 @@ func (sl *SprintList) Render() error {
 
 // RenderInTable renders the list in table view.
 func (sl *SprintList) RenderInTable() error {
-	if sl.Display.Plain {
+	if tui.IsDumbTerminal() || sl.Display.Plain {
 		w := tabwriter.NewWriter(os.Stdout, 0, tabWidth, 1, '\t', 0)
 		return sl.renderPlain(w)
 	}

pkg/tui/helper.go

@@ -8,6 +8,7 @@ import (
 	"runtime"
 	"strings"
 
+	"github.com/cli/safeexec"
 	"github.com/gdamore/tcell/v2"
 
 	"github.com/ankitpokhrel/jira-cli/pkg/tui/primitive"
@@ -55,35 +56,66 @@ func getInfoModal() *primitive.Modal {
 		SetBorderColor(tcell.ColorDefault)
 }
 
+// IsDumbTerminal checks TERM environment variable and returns true if it is set to dumb.
+//
+// Dumb terminal indicates terminal with limited capability. It may not provide support
+// for special character sequences, e.g., no handling of ANSI escape sequences.
+func IsDumbTerminal() bool {
+	term := strings.ToLower(os.Getenv("TERM"))
+	return term == "" || term == "dumb"
+}
+
 // GetPager returns configured pager.
 func GetPager() string {
 	if runtime.GOOS == "windows" {
 		return ""
 	}
+	if IsDumbTerminal() {
+		return "cat"
+	}
 	pager := os.Getenv("PAGER")
-	if pager == "" && cmdExists("less") {
-		pager = "less -r"
+	if pager == "" {
+		pager = "less"
 	}
 	return pager
 }
 
 // PagerOut outputs to configured pager if possible.
 func PagerOut(out string) error {
-	pager := GetPager()
-	if pager == "" {
+	pagerCmd := GetPager()
+	if pagerCmd == "" {
 		_, err := fmt.Print(out)
 		return err
 	}
-	pa := strings.Split(pager, " ")
-	cmd := exec.Command(pa[0], pa[1:]...)
+
+	pa := strings.Split(pagerCmd, " ")
+	pager, pagerArgs := pa[0], pa[1:]
+	if err := cmdExists(pager); err != nil {
+		return err
+	}
+
+	pagerEnv := os.Environ()
+	for i := len(pagerEnv) - 1; i >= 0; i-- {
+		if strings.HasPrefix(pagerEnv[i], "PAGER=") {
+			pagerEnv = append(pagerEnv[0:i], pagerEnv[i+1:]...)
+		}
+	}
+	if _, ok := os.LookupEnv("LESS"); !ok {
+		pagerEnv = append(pagerEnv, "LESS=R")
+	}
+
+	cmd := exec.Command(pager, pagerArgs...)
+	cmd.Env = pagerEnv
 	cmd.Stdin = strings.NewReader(out)
 	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+
 	return cmd.Run()
 }
 
-func cmdExists(cmd string) bool {
-	_, err := exec.LookPath(cmd)
-	return err == nil
+func cmdExists(cmd string) error {
+	_, err := safeexec.LookPath(cmd)
+	return err
 }
 
 func customTUIStyle(style TableStyle) tcell.Style {

pkg/tui/helper_test.go

@@ -89,13 +89,60 @@ func TestSplitText(t *testing.T) {
 func TestGetPager(t *testing.T) {
 	t.Parallel()
 
+	term := os.Getenv("TERM")
 	pager := os.Getenv("PAGER")
 
-	_ = os.Setenv("PAGER", "")
-	assert.Equal(t, "less -r", GetPager())
+	// TERM is xterm, PAGER is set.
+	{
+		_ = os.Setenv("TERM", "xterm")
 
-	_ = os.Setenv("PAGER", "more")
-	assert.Equal(t, "more", GetPager())
+		_ = os.Setenv("PAGER", "")
+		assert.Equal(t, "less", GetPager())
 
+		_ = os.Setenv("PAGER", "more")
+		assert.Equal(t, "more", GetPager())
+
+		_ = os.Setenv("PAGER", pager)
+	}
+
+	// TERM is set, PAGER is unset.
+	{
+		_ = os.Unsetenv("PAGER")
+
+		_ = os.Setenv("TERM", "dumb")
+		assert.Equal(t, "cat", GetPager())
+
+		_ = os.Setenv("TERM", "")
+		assert.Equal(t, "cat", GetPager())
+
+		_ = os.Setenv("TERM", "xterm")
+		assert.Equal(t, "less", GetPager())
+
+		_ = os.Setenv("TERM", term)
+	}
+
+	// TERM gets precendence if both PAGER and TERM are set.
+	{
+		_ = os.Setenv("PAGER", "")
+		_ = os.Setenv("TERM", "")
+		assert.Equal(t, "cat", GetPager())
+
+		_ = os.Setenv("PAGER", "more")
+		_ = os.Setenv("TERM", "dumb")
+		assert.Equal(t, "cat", GetPager())
+
+		_ = os.Setenv("PAGER", "more")
+		_ = os.Setenv("TERM", "xterm")
+		assert.Equal(t, "more", GetPager())
+	}
+
+	// TERM and PAGER are not set.
+	{
+		_ = os.Unsetenv("PAGER")
+		_ = os.Unsetenv("TERM")
+		assert.Equal(t, "cat", GetPager())
+	}
+
+	_ = os.Setenv("TERM", term)
 	_ = os.Setenv("PAGER", pager)
 }