Merge pull request #423 from kpengboy/color-diffs

Colorize diff output

Author: mbrt

cmd/gmailctl/cmd/apply_cmd.go

@@ -51,16 +51,18 @@ func init() {
 	applyCmd.PersistentFlags().StringVarP(&applyFilename, "filename", "f", "", "configuration file")
 	applyCmd.Flags().BoolVarP(&applyYes, "yes", "y", false, "don't ask for confirmation, just apply")
 	applyCmd.Flags().BoolVarP(&applyRemoveLabels, "remove-labels", "r", false, "allow removing labels")
-	applyCmd.Flags().BoolVarP(&applySkipTests, "yolo", "", false, "skip configuration tests")
-	applyCmd.PersistentFlags().BoolVarP(&applyDebug, "debug", "", false, "print extra debugging information")
-	applyCmd.PersistentFlags().IntVarP(&applyDiffContext, "diff-context", "", papply.DefaultContextLines, "number of lines of filter diff context to show")
+	applyCmd.Flags().BoolVar(&applySkipTests, "yolo", false, "skip configuration tests")
+	applyCmd.PersistentFlags().BoolVar(&applyDebug, "debug", false, "print extra debugging information")
+	applyCmd.PersistentFlags().IntVar(&applyDiffContext, "diff-context", papply.DefaultContextLines, "number of lines of filter diff context to show")
 }
 
 func apply(path string, interactive, test bool) error {
 	if applyDiffContext < 0 {
 		return errors.New("--diff-context must be non-negative")
 	}
 
+	useColor := shouldUseColorDiff()
+
 	parseRes, err := parseConfig(path, "", test)
 	if err != nil {
 		return err
@@ -76,7 +78,7 @@ func apply(path string, interactive, test bool) error {
 		return err
 	}
 
-	diff, err := papply.Diff(parseRes.Res.GmailConfig, upstream, applyDebug, applyDiffContext)
+	diff, err := papply.Diff(parseRes.Res.GmailConfig, upstream, applyDebug, applyDiffContext, useColor)
 	if err != nil {
 		return fmt.Errorf("cannot compare upstream with local config: %w", err)
 	}

cmd/gmailctl/cmd/diff_cmd.go

@@ -40,15 +40,17 @@ func init() {
 
 	// Flags and configuration settings
 	diffCmd.PersistentFlags().StringVarP(&diffFilename, "filename", "f", "", "configuration file")
-	diffCmd.PersistentFlags().BoolVarP(&diffDebug, "debug", "", false, "print extra debugging information")
-	diffCmd.PersistentFlags().IntVarP(&diffContext, "context", "", papply.DefaultContextLines, "number of lines of filter diff context to show")
+	diffCmd.PersistentFlags().BoolVar(&diffDebug, "debug", false, "print extra debugging information")
+	diffCmd.PersistentFlags().IntVar(&diffContext, "context", papply.DefaultContextLines, "number of lines of filter diff context to show")
 }
 
 func diff(path string) error {
 	if diffContext < 0 {
 		return errors.New("--context must be non-negative")
 	}
 
+	useColor := shouldUseColorDiff()
+
 	parseRes, err := parseConfig(path, "", false)
 	if err != nil {
 		return err
@@ -64,7 +66,7 @@ func diff(path string) error {
 		return err
 	}
 
-	diff, err := papply.Diff(parseRes.Res.GmailConfig, upstream, diffDebug, diffContext)
+	diff, err := papply.Diff(parseRes.Res.GmailConfig, upstream, diffDebug, diffContext, useColor)
 	if err != nil {
 		return fmt.Errorf("cannot compare upstream with local config: %w", err)
 	}

cmd/gmailctl/cmd/edit_cmd.go

@@ -23,6 +23,8 @@ var (
 	editDiffContext int
 )
 
+var editUseColor bool
+
 var (
 	defaultEditors = []string{
 		"editor",
@@ -70,15 +72,17 @@ func init() {
 	// Flags and configuration settings
 	editCmd.PersistentFlags().StringVarP(&editFilename, "filename", "f", "", "configuration file")
 	editCmd.Flags().BoolVarP(&editSkipTests, "yolo", "", false, "skip configuration tests")
-	editCmd.PersistentFlags().BoolVarP(&editDebug, "debug", "", false, "print extra debugging information")
-	editCmd.PersistentFlags().IntVarP(&editDiffContext, "diff-context", "", papply.DefaultContextLines, "number of lines of filter diff context to show")
+	editCmd.PersistentFlags().BoolVar(&editDebug, "debug", false, "print extra debugging information")
+	editCmd.PersistentFlags().IntVar(&editDiffContext, "diff-context", papply.DefaultContextLines, "number of lines of filter diff context to show")
 }
 
 func edit(path string, test bool) error {
 	if editDiffContext < 0 {
 		return errors.New("--diff-context must be non-negative")
 	}
 
+	editUseColor = shouldUseColorDiff()
+
 	// First make sure that Gmail can be contacted, so that we don't
 	// waste the user's time editing a config file that cannot be
 	// applied now.
@@ -229,7 +233,7 @@ func applyEdited(path, originalPath string, test bool, gmailapi *api.GmailAPI) e
 		return err
 	}
 
-	diff, err := papply.Diff(parseRes.Res.GmailConfig, upstream, editDebug, editDiffContext)
+	diff, err := papply.Diff(parseRes.Res.GmailConfig, upstream, editDebug, editDiffContext, editUseColor)
 	if err != nil {
 		return errors.New("comparing upstream with local config")
 	}

cmd/gmailctl/cmd/root_cmd.go

@@ -6,10 +6,12 @@ import (
 	"os/user"
 	"path"
 
+	"github.com/mattn/go-isatty"
 	"github.com/spf13/cobra"
 )
 
 var cfgDir string
+var colorFlag string
 
 // rootCmd is the command run when executing without subcommands.
 var rootCmd = &cobra.Command{
@@ -52,6 +54,9 @@ func init() {
 	// Cobra supports persistent flags, which, if defined here,
 	// will be global for your application.
 	rootCmd.PersistentFlags().StringVar(&cfgDir, "config", "", "config directory (default is $HOME/.gmailctl)")
+	rootCmd.PersistentFlags().StringVar(&colorFlag, "color", "auto",
+		"whether to enable color output ('always', 'auto' or 'never')")
+	rootCmd.PersistentFlags().Lookup("color").NoOptDefVal = "always"
 }
 
 // initConfig reads in config file and ENV variables if set.
@@ -68,3 +73,20 @@ func initConfig() {
 	}
 	cfgDir = path.Join(usr.HomeDir, ".gmailctl")
 }
+
+// shouldUseColorDiff decides, based on the value of the color flag and other
+// factors, whether gmailctl should use color output.
+func shouldUseColorDiff() bool {
+	switch colorFlag {
+	case "never":
+		return false
+	case "auto":
+		return os.Getenv("TERM") != "dumb" &&
+			(isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd()))
+	case "always":
+		return true
+	default:
+		fatal(fmt.Errorf("--color must be 'always', 'auto' or 'never', not '%v'", colorFlag))
+		return false
+	}
+}

go.mod

@@ -5,9 +5,11 @@ go 1.23.7
 toolchain go1.24.4
 
 require (
+	github.com/fatih/color v1.18.1-0.20241008080414-2aae7c9a9c41
 	github.com/google/go-jsonnet v0.21.0
 	github.com/gorilla/mux v1.8.1
 	github.com/hashicorp/go-multierror v1.1.1
+	github.com/mattn/go-isatty v0.0.20
 	github.com/pmezard/go-difflib v1.0.0
 	github.com/spf13/cobra v1.9.1
 	github.com/stretchr/testify v1.10.0
@@ -30,6 +32,7 @@ require (
 	github.com/googleapis/gax-go/v2 v2.14.2 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
+	github.com/mattn/go-colorable v0.1.13 // indirect
 	github.com/spf13/pflag v1.0.6 // indirect
 	go.opentelemetry.io/auto/sdk v1.1.0 // indirect
 	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.60.0 // indirect

go.sum

@@ -7,6 +7,8 @@ cloud.google.com/go/compute/metadata v0.7.0/go.mod h1:j5MvL9PprKL39t166CoB1uVHfQ
 github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/fatih/color v1.18.1-0.20241008080414-2aae7c9a9c41 h1:XkQfr36qgf52W/9RfDNk/aNfBC6Cl++6uc/L+lHEKRk=
+github.com/fatih/color v1.18.1-0.20241008080414-2aae7c9a9c41/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU=
 github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg=
 github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
 github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
@@ -42,6 +44,11 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
 github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
+github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
+github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg=
+github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
+github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
+github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII=
@@ -77,6 +84,8 @@ golang.org/x/oauth2 v0.30.0 h1:dnDm7JmhM45NNpd8FDDeLhK6FwqbOf4MLCM9zb1BOHI=
 golang.org/x/oauth2 v0.30.0/go.mod h1:B++QgG3ZKulg6sRPGD/mqlHQs5rB3Ml9erfeDY7xKlU=
 golang.org/x/sync v0.14.0 h1:woo0S4Yywslg6hp4eUFjTVOyKt0RookbpAHG4c1HmhQ=
 golang.org/x/sync v0.14.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
+golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw=
 golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
 golang.org/x/text v0.25.0 h1:qVyWApTSYLk/drJRO5mDlNYskwQznZmkpV2c8q9zls4=

integration_test.go

@@ -68,7 +68,7 @@ func TestIntegration(t *testing.T) {
 			require.Nil(t, err)
 
 			// Apply the diff.
-			d, err := apply.Diff(pres.GmailConfig, upres, false, apply.DefaultContextLines)
+			d, err := apply.Diff(pres.GmailConfig, upres, false, apply.DefaultContextLines, false /* colorize */)
 			require.Nil(t, err)
 			err = apply.Apply(d, gapi, true)
 			require.Nil(t, err)
@@ -139,7 +139,7 @@ func TestIntegrationImportExport(t *testing.T) {
 			require.Nil(t, err)
 
 			// Apply the diff.
-			d, err := apply.Diff(pres.GmailConfig, upres, false, apply.DefaultContextLines)
+			d, err := apply.Diff(pres.GmailConfig, upres, false, apply.DefaultContextLines, false /* colorize */)
 			require.Nil(t, err)
 			err = apply.Apply(d, gapi, true)
 			require.Nil(t, err)
@@ -174,7 +174,7 @@ func TestIntegrationImportExport(t *testing.T) {
 }
 
 func assertEmptyDiff(t *testing.T, local, remote apply.GmailConfig) {
-	d, err := apply.Diff(local, remote, false, -1)
+	d, err := apply.Diff(local, remote, false, -1 /* contextLines */, false /* colorize */)
 	require.Nil(t, err)
 	assert.True(t, d.FiltersDiff.Empty())
 	assert.True(t, d.LabelsDiff.Empty())

internal/engine/apply/apply.go

@@ -116,20 +116,20 @@ func (d ConfigDiff) Validate() error {
 }
 
 // Diff computes the diff between local and upstream configuration.
-func Diff(local, upstream GmailConfig, debugInfo bool, contextLines int) (ConfigDiff, error) {
+func Diff(local, upstream GmailConfig, debugInfo bool, contextLines int, colorize bool) (ConfigDiff, error) {
 	res := ConfigDiff{
 		LocalConfig: local,
 	}
 	var err error
 
-	res.FiltersDiff, err = filter.Diff(upstream.Filters, local.Filters, debugInfo, contextLines)
+	res.FiltersDiff, err = filter.Diff(upstream.Filters, local.Filters, debugInfo, contextLines, colorize)
 	if err != nil {
 		return res, fmt.Errorf("cannot compute filters diff: %w", err)
 	}
 
 	if len(local.Labels) > 0 {
 		// LabelsDiff management opted-in
-		res.LabelsDiff, err = label.Diff(upstream.Labels, local.Labels)
+		res.LabelsDiff, err = label.Diff(upstream.Labels, local.Labels, colorize)
 		if err != nil {
 			return res, fmt.Errorf("cannot compute labels diff: %w", err)
 		}

internal/engine/filter/diff.go

@@ -9,17 +9,18 @@ import (
 	"github.com/pmezard/go-difflib/difflib"
 
 	"github.com/mbrt/gmailctl/internal/graph"
+	"github.com/mbrt/gmailctl/internal/reporting"
 )
 
 // Diff computes the diff between two lists of filters.
 //
 // To compute the diff, IDs are ignored, only the contents of the filters are actually considered.
-func Diff(upstream, local Filters, debugInfo bool, contextLines int) (FiltersDiff, error) {
+func Diff(upstream, local Filters, debugInfo bool, contextLines int, colorize bool) (FiltersDiff, error) {
 	// Computing the diff is very expensive, so we have to minimize the number of filters
 	// we have to analyze. To do so, we get rid of the filters that are exactly the same,
 	// by hashing them.
 	added, removed := changedFilters(upstream, local)
-	return NewMinimalFiltersDiff(added, removed, debugInfo, contextLines), nil
+	return NewMinimalFiltersDiff(added, removed, debugInfo, contextLines, colorize), nil
 }
 
 // NewMinimalFiltersDiff creates a new FiltersDiff with reordered filters, where
@@ -28,11 +29,11 @@ func Diff(upstream, local Filters, debugInfo bool, contextLines int) (FiltersDif
 // The algorithm used is a quadratic approximation to the otherwise NP-complete
 // travel salesman problem. Hopefully the number of filters is low enough to
 // make this not too slow and the approximation not too bad.
-func NewMinimalFiltersDiff(added, removed Filters, printDebugInfo bool, contextLines int) FiltersDiff {
+func NewMinimalFiltersDiff(added, removed Filters, printDebugInfo bool, contextLines int, colorize bool) FiltersDiff {
 	if len(added) > 0 && len(removed) > 0 {
 		added, removed = reorderWithHungarian(added, removed)
 	}
-	return FiltersDiff{added, removed, printDebugInfo, contextLines}
+	return FiltersDiff{added, removed, printDebugInfo, contextLines, colorize}
 }
 
 // FiltersDiff contains filters that have been added and removed locally with respect to upstream.
@@ -41,6 +42,7 @@ type FiltersDiff struct {
 	Removed        Filters
 	PrintDebugInfo bool
 	ContextLines   int
+	Colorize       bool
 }
 
 // Empty returns true if the diff is empty.
@@ -69,6 +71,10 @@ func (f FiltersDiff) String() string {
 		// We can't get a diff apparently, let's make something up here
 		return fmt.Sprintf("Removed:\n%s\nAdded:\n%s", removed, added)
 	}
+	if f.Colorize {
+		s = reporting.ColorizeDiff(s)
+	}
+
 	return s
 }