Disable color when not in terminal

* Move IsTerminal to internal logging package, to keep it general but not expose it as part of public API

Signed-off-by: David Freilich <dfreilich@vmware.com>

cmd/cmd.go

@@ -11,6 +11,7 @@ import (
 	"github.com/buildpacks/pack/internal/commands"
 	"github.com/buildpacks/pack/internal/config"
 	imagewriter "github.com/buildpacks/pack/internal/inspectimage/writer"
+	ilogging "github.com/buildpacks/pack/internal/logging"
 	"github.com/buildpacks/pack/logging"
 )
 
@@ -41,9 +42,13 @@ func NewPackCommand(logger ConfigurableLogger) (*cobra.Command, error) {
 		Short: "CLI for building apps using Cloud Native Buildpacks",
 		PersistentPreRun: func(cmd *cobra.Command, args []string) {
 			if fs := cmd.Flags(); fs != nil {
-				if flag, err := fs.GetBool("no-color"); err == nil {
+				if flag, err := fs.GetBool("no-color"); err == nil && flag {
 					color.Disable(flag)
 				}
+
+				_, canDisplayColor := ilogging.IsTerminal(logging.GetWriterForLevel(logger, logging.InfoLevel))
+				color.Disable(!canDisplayColor)
+
 				if flag, err := fs.GetBool("quiet"); err == nil {
 					logger.WantQuiet(flag)
 				}

internal/image/fetcher.go

@@ -7,8 +7,6 @@ import (
 	"io"
 	"strings"
 
-	"github.com/buildpacks/pack/config"
-
 	"github.com/buildpacks/imgutil"
 	"github.com/buildpacks/imgutil/local"
 	"github.com/buildpacks/imgutil/remote"
@@ -18,8 +16,9 @@ import (
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/pkg/errors"
-	"golang.org/x/crypto/ssh/terminal"
 
+	"github.com/buildpacks/pack/config"
+	ilogging "github.com/buildpacks/pack/internal/logging"
 	"github.com/buildpacks/pack/internal/style"
 	"github.com/buildpacks/pack/logging"
 )
@@ -99,7 +98,7 @@ func (f *Fetcher) pullImage(ctx context.Context, imageID string) error {
 	}
 
 	writer := logging.GetWriterForLevel(f.logger, logging.InfoLevel)
-	termFd, isTerm := isTerminal(writer)
+	termFd, isTerm := ilogging.IsTerminal(writer)
 
 	err = jsonmessage.DisplayJSONMessagesStream(rc, &colorizedWriter{writer}, termFd, isTerm, nil)
 	if err != nil {
@@ -109,20 +108,6 @@ func (f *Fetcher) pullImage(ctx context.Context, imageID string) error {
 	return rc.Close()
 }
 
-func isTerminal(w io.Writer) (uintptr, bool) {
-	type descriptor interface {
-		Fd() uintptr
-	}
-
-	if f, ok := w.(descriptor); ok {
-		termFd := f.Fd()
-		isTerm := terminal.IsTerminal(int(termFd))
-		return termFd, isTerm
-	}
-
-	return 0, false
-}
-
 func registryAuth(ref string) (string, error) {
 	_, a, err := auth.ReferenceForRepoName(authn.DefaultKeychain, ref)
 	if err != nil {

internal/logging/log_writer.go

@@ -3,7 +3,6 @@ package logging
 import (
 	"fmt"
 	"io"
-	"os"
 	"regexp"
 	"sync"
 	"time"
@@ -57,15 +56,18 @@ func (tw *LogWriter) Fd() uintptr {
 	tw.Lock()
 	defer tw.Unlock()
 
-	file, ok := tw.out.(*os.File)
-	if ok {
+	if file, ok := tw.out.(hasDescriptor); ok {
 		return file.Fd()
 	}
 
-	return 0
+	return invalidFileDescriptor
 }
 
 // Remove all ANSI color information.
 func stripColor(b []byte) []byte {
 	return colorCodeMatcher.ReplaceAll(b, []byte(""))
 }
+
+type hasDescriptor interface {
+	Fd() uintptr
+}

internal/logging/log_writer_test.go

@@ -69,7 +69,7 @@ func testLogWriter(t *testing.T, when spec.G, it spec.S) {
 			it("returns 0", func() {
 				var out *bytes.Buffer
 				writer = ilogging.NewLogWriter(out, clockFunc, true)
-				h.AssertEq(t, int(writer.Fd()), 0)
+				h.AssertEq(t, int(writer.Fd()), -1)
 			})
 		})
 	})

internal/logging/logging.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/apex/log"
+	"golang.org/x/crypto/ssh/terminal"
 
 	"github.com/buildpacks/pack/internal/style"
 	"github.com/buildpacks/pack/logging"
@@ -23,7 +24,8 @@ const (
 	// log level to use when debug is true
 	verboseLevel = log.DebugLevel
 	// time format the out logging uses
-	timeFmt = "2006/01/02 15:04:05.000000"
+	timeFmt               = "2006/01/02 15:04:05.000000"
+	invalidFileDescriptor = ^(uintptr(0))
 )
 
 // LogWithWriters is a logger used with the pack CLI, allowing users to print logs for various levels, including Info, Debug and Error
@@ -123,6 +125,17 @@ func (lw *LogWithWriters) IsVerbose() bool {
 	return lw.Level == log.DebugLevel
 }
 
+// IsTerminal returns whether a writer is a terminal
+func IsTerminal(w io.Writer) (uintptr, bool) {
+	if f, ok := w.(hasDescriptor); ok {
+		termFd := f.Fd()
+		isTerm := terminal.IsTerminal(int(termFd))
+		return termFd, isTerm
+	}
+
+	return invalidFileDescriptor, false
+}
+
 func formatLevel(ll log.Level) string {
 	switch ll {
 	case log.ErrorLevel:

internal/logging/logging_test.go

@@ -222,6 +222,13 @@ func testLogWithWriters(t *testing.T, when spec.G, it spec.S) {
 		expected := "\n"
 		h.AssertEq(t, fOut(), expected)
 	})
+
+	when("IsTerminal", func() {
+		it("returns false for a pipe", func() {
+			_, isTerm := IsTerminal(logger.WriterForLevel(logging.InfoLevel))
+			h.AssertFalse(t, isTerm)
+		})
+	})
 }
 
 func assertLogWriterHasOut(t *testing.T, writer io.Writer, out io.Writer) {