fix: support colour output in CI via FORCE_COLOR/CLICOLOR_FORCE

README.md

@@ -144,6 +144,26 @@ This command will perform the token exchange and configure the CLI for use.
 
 See the [documentation on OpenID Connect for more information](https://oc.to/ServiceAccountOidcIdentities)
 
+### Colour output
+
+By default the CLI emits colour only when its output is attached to a terminal. CI systems such as
+GitHub Actions and GitLab CI render ANSI colour codes but do not attach a terminal, so colour is
+disabled there unless you ask for it:
+
+```shell
+export FORCE_COLOR=1   # or CLICOLOR_FORCE=1
+```
+
+The same variables turn colour off when set to `0`, even in a terminal, and the environment is
+consulted in this order:
+
+| Variable | Effect |
+|---|---|
+| `NO_COLOR` (set to any value) | Colour off. Takes precedence over everything below. |
+| `CLICOLOR_FORCE` / `FORCE_COLOR` | `0` forces colour off, any other value forces it on, regardless of whether output is a terminal. `CLICOLOR_FORCE` wins if both are set. |
+| `CLICOLOR=0` | Colour off. |
+| none of the above | Colour on only when output is attached to a terminal. |
+
 ## Overview
 
 This project aims to create a new CLI (written in Go) for communicating with the Octopus Deploy Server.

pkg/output/color.go

@@ -4,13 +4,14 @@ import (
 	"fmt"
 	"os"
 	"regexp"
+	"strings"
 
 	"github.com/mgutz/ansi"
 	"golang.org/x/term"
 )
 
 var (
-	IsColorEnabled = os.Getenv("NO_COLOR") == "" && term.IsTerminal(int(os.Stdout.Fd()))
+	IsColorEnabled = isColorEnabled()
 	magenta        = ansi.ColorFunc("magenta")
 	cyan           = ansi.ColorFunc("cyan")
 	red            = ansi.ColorFunc("red")
@@ -21,88 +22,128 @@ var (
 	dim            = ansi.ColorFunc("default+d")
 )
 
-func Blue(s string) string {
+// isColorEnabled decides whether ANSI colour codes should be emitted, following
+// the widely adopted no-color.org and bixense.com/clicolors conventions:
+//
+//   - NO_COLOR set to anything non-empty disables colour outright.
+//   - CLICOLOR_FORCE or FORCE_COLOR turns colour on even when stdout is not a
+//     terminal. CI systems such as GitHub Actions and GitLab CI render ANSI
+//     codes but do not attach a TTY, so terminal detection alone can never
+//     enable colour there. Setting either to "0" is the opposite instruction and
+//     turns colour off even when stdout is a terminal.
+//   - CLICOLOR set to "0" disables colour.
+//   - Otherwise colour is used only when stdout is a terminal.
+func isColorEnabled() bool {
+	return isColorEnabledFor(term.IsTerminal(int(os.Stdout.Fd())))
+}
+
+// isColorEnabledFor is isColorEnabled with terminal detection supplied by the
+// caller, so the decision table can be tested in both directions.
+func isColorEnabledFor(isTerminal bool) bool {
+	if os.Getenv("NO_COLOR") != "" {
+		return false
+	}
+
+	// An explicitly set force variable is an instruction in both directions, so it
+	// overrides terminal detection whichever way it points.
+	for _, name := range []string{"CLICOLOR_FORCE", "FORCE_COLOR"} {
+		if value, isSet := os.LookupEnv(name); isSet && value != "" {
+			return value != "0"
+		}
+	}
+
+	if os.Getenv("CLICOLOR") == "0" {
+		return false
+	}
+
+	return isTerminal
+}
+
+// applyColor wraps s in colorFunc, one line at a time.
+//
+// A multi-line string could be wrapped once, with a single escape at the front
+// and a single reset at the end, and a terminal would render it correctly. Log
+// viewers are less forgiving: GitHub Actions, for one, resets SGR state at every
+// line break, so only the first line of such a block is ever tinted. Emitting
+// the escape on each line renders identically in a terminal and correctly in
+// those viewers. Blank lines are left alone; there is nothing to colour, and the
+// stray escapes would be the only thing on the line.
+func applyColor(colorFunc func(string) string, s string) string {
 	if !IsColorEnabled {
 		return s
 	}
-	return blue(s)
+
+	if !strings.Contains(s, "\n") {
+		return colorFunc(s)
+	}
+
+	lines := strings.Split(s, "\n")
+	for i, line := range lines {
+		if line != "" {
+			lines[i] = colorFunc(line)
+		}
+	}
+	return strings.Join(lines, "\n")
+}
+
+func Blue(s string) string {
+	return applyColor(blue, s)
 }
 
 func Bluef(s string, args ...interface{}) string {
 	return Blue(fmt.Sprintf(s, args...))
 }
 
 func Magenta(s string) string {
-	if !IsColorEnabled {
-		return s
-	}
-	return magenta(s)
+	return applyColor(magenta, s)
 }
 
 func Magentaf(s string, args ...interface{}) string {
 	return Magenta(fmt.Sprintf(s, args...))
 }
 
 func Cyan(s string) string {
-	if !IsColorEnabled {
-		return s
-	}
-	return cyan(s)
+	return applyColor(cyan, s)
 }
 
 func Cyanf(s string, args ...interface{}) string {
 	return Cyan(fmt.Sprintf(s, args...))
 }
 
 func Red(s string) string {
-	if !IsColorEnabled {
-		return s
-	}
-	return red(s)
+	return applyColor(red, s)
 }
 
 func Redf(s string, args ...interface{}) string {
 	return Red(fmt.Sprintf(s, args...))
 }
 
 func Yellow(s string) string {
-	if !IsColorEnabled {
-		return s
-	}
-	return yellow(s)
+	return applyColor(yellow, s)
 }
 
 func Yellowf(s string, args ...interface{}) string {
 	return Yellow(fmt.Sprintf(s, args...))
 }
 
 func Green(s string) string {
-	if !IsColorEnabled {
-		return s
-	}
-	return green(s)
+	return applyColor(green, s)
 }
 
 func Greenf(s string, args ...interface{}) string {
 	return Green(fmt.Sprintf(s, args...))
 }
 
 func Bold(s string) string {
-	if !IsColorEnabled {
-		return s
-	}
-	return bold(s)
+	return applyColor(bold, s)
 }
 
 func Boldf(s string, args ...interface{}) string {
 	return Bold(fmt.Sprintf(s, args...))
 }
 
 func Dim(s string) string {
-	if !IsColorEnabled {
-		return s
-	}
-	return dim(s)
+	return applyColor(dim, s)
 }
 
 func Dimf(s string, args ...interface{}) string {

pkg/output/color_test.go

@@ -0,0 +1,176 @@
+package output
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsColorEnabled(t *testing.T) {
+	tests := []struct {
+		name       string
+		noColor    string
+		clicolor   string
+		forceColor string
+		cliColor   string
+		// expected result when stdout is, and is not, a terminal
+		expectOnTerminal    bool
+		expectNotOnTerminal bool
+	}{
+		{
+			name:             "no environment variables set: terminal detection decides",
+			expectOnTerminal: true,
+		},
+		{
+			name:                "FORCE_COLOR forces colour on",
+			forceColor:          "1",
+			expectOnTerminal:    true,
+			expectNotOnTerminal: true,
+		},
+		{
+			name:                "CLICOLOR_FORCE forces colour on",
+			clicolor:            "1",
+			expectOnTerminal:    true,
+			expectNotOnTerminal: true,
+		},
+		{
+			name:       "FORCE_COLOR of 0 forces colour off",
+			forceColor: "0",
+		},
+		{
+			name:     "CLICOLOR_FORCE of 0 forces colour off",
+			clicolor: "0",
+		},
+		{
+			name:     "CLICOLOR of 0 disables colour",
+			cliColor: "0",
+		},
+		{
+			name:             "CLICOLOR of 1 leaves terminal detection to decide",
+			cliColor:         "1",
+			expectOnTerminal: true,
+		},
+		{
+			name:       "NO_COLOR wins over FORCE_COLOR",
+			noColor:    "1",
+			forceColor: "1",
+		},
+		{
+			name:     "NO_COLOR wins over CLICOLOR_FORCE",
+			noColor:  "1",
+			clicolor: "1",
+		},
+		{
+			name:                "CLICOLOR_FORCE wins over FORCE_COLOR",
+			clicolor:            "1",
+			forceColor:          "0",
+			expectOnTerminal:    true,
+			expectNotOnTerminal: true,
+		},
+		{
+			name:                "FORCE_COLOR wins over CLICOLOR",
+			forceColor:          "1",
+			cliColor:            "0",
+			expectOnTerminal:    true,
+			expectNotOnTerminal: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Setenv("NO_COLOR", test.noColor)
+			t.Setenv("CLICOLOR_FORCE", test.clicolor)
+			t.Setenv("FORCE_COLOR", test.forceColor)
+			t.Setenv("CLICOLOR", test.cliColor)
+
+			assert.Equal(t, test.expectOnTerminal, isColorEnabledFor(true), "on a terminal")
+			assert.Equal(t, test.expectNotOnTerminal, isColorEnabledFor(false), "not on a terminal")
+		})
+	}
+}
+
+// Every exported helper must honour IsColorEnabled, otherwise NO_COLOR only
+// partially takes effect.
+func TestColorHelpersHonourIsColorEnabled(t *testing.T) {
+	helpers := map[string]struct {
+		plain     func(string) string
+		formatted func(string, ...interface{}) string
+	}{
+		"Blue":    {Blue, Bluef},
+		"Magenta": {Magenta, Magentaf},
+		"Cyan":    {Cyan, Cyanf},
+		"Red":     {Red, Redf},
+		"Yellow":  {Yellow, Yellowf},
+		"Green":   {Green, Greenf},
+		"Bold":    {Bold, Boldf},
+		"Dim":     {Dim, Dimf},
+	}
+
+	original := IsColorEnabled
+	t.Cleanup(func() { IsColorEnabled = original })
+
+	for name, helper := range helpers {
+		t.Run(name, func(t *testing.T) {
+			IsColorEnabled = false
+			assert.Equal(t, "text", helper.plain("text"))
+			assert.Equal(t, "text", helper.formatted("%s", "text"))
+
+			IsColorEnabled = true
+			assert.NotEqual(t, "text", helper.plain("text"))
+			assert.NotEqual(t, "text", helper.formatted("%s", "text"))
+		})
+	}
+}
+
+// GitHub Actions and other log viewers reset SGR state at every line break, so
+// a multi-line string needs the escape repeated on each line rather than once
+// around the whole block.
+func TestMultiLineTextIsColouredPerLine(t *testing.T) {
+	original := IsColorEnabled
+	t.Cleanup(func() { IsColorEnabled = original })
+
+	IsColorEnabled = true
+	lines := strings.Split(Cyan("one\ntwo\nthree"), "\n")
+
+	assert.Len(t, lines, 3)
+	for _, line := range lines {
+		assert.True(t, strings.HasPrefix(line, "\x1b["), "line should open with an escape: %q", line)
+		assert.True(t, strings.HasSuffix(line, "\x1b[0m"), "line should close with a reset: %q", line)
+	}
+}
+
+// A blank line has nothing to colour, so escapes around it would be the only
+// thing on the line.
+func TestBlankLinesAreNotColoured(t *testing.T) {
+	original := IsColorEnabled
+	t.Cleanup(func() { IsColorEnabled = original })
+
+	IsColorEnabled = true
+	lines := strings.Split(Cyan("one\n\ntwo\n"), "\n")
+
+	assert.Len(t, lines, 4)
+	assert.Equal(t, "", lines[1], "interior blank line should be untouched")
+	assert.Equal(t, "", lines[3], "trailing newline should not gain escapes")
+}
+
+func TestMultiLineTextIsUnchangedWhenColourDisabled(t *testing.T) {
+	original := IsColorEnabled
+	t.Cleanup(func() { IsColorEnabled = original })
+
+	IsColorEnabled = false
+	assert.Equal(t, "one\ntwo\nthree", Cyan("one\ntwo\nthree"))
+}
+
+func TestFormatDocHonoursIsColorEnabled(t *testing.T) {
+	original := IsColorEnabled
+	t.Cleanup(func() { IsColorEnabled = original })
+
+	doc := "bold(a) green(b) yellow(c) blue(d) cyan(e) magenta(f) red(g) dim(h)"
+
+	IsColorEnabled = false
+	assert.Equal(t, "a b c d e f g h", FormatDoc(doc))
+
+	IsColorEnabled = true
+	assert.NotEqual(t, "a b c d e f g h", FormatDoc(doc))
+}