Remove GOPASS_NOCOLOR (gopasspw#1937)

Fixes gopasspw#1936

RELEASE_NOTES=[CLEANUP] Remove GOPASS_NOCOLOR in favor of NO_COLOR

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>

docs/config.md

@@ -15,7 +15,6 @@ Some configuration options are only available through setting environment variab
 | `GOPASS_UMASK`          | `octal`  | Set to any valid umask to mask bits of files created by gopass                                               |
 | `GOPASS_GPG_OPTS`       | `string` | Add any extra arguments, e.g. `--armor` you want to pass to GPG on every invocation                          |
 | `GOPASS_EXTERNAL_PWGEN` | `string` | Use an external password generator. See [Features](features.md#using-custom-password-generators) for details |
-| `GOPASS_NOCOLOR`        | `bool`   | Set to true to disable colored output                                                                        |
 | `GOPASS_CHARACTER_SET`  | `bool`   | Set to any non-empty value to restrict the characters used in generated passwords                            |
 | `GOPASS_CONFIG`         | `string` | Set this to the absolute path to the configuration file                                                      |
 | `GOPASS_HOMEDIR`        | `string` | Set this to the absolute path of the directory containing the `.config/` tree                                |
@@ -33,6 +32,7 @@ Variables not exclusively used by gopass
 | `PAGER`                | `string` | the pager program used for `gopass list`. See [Features](features.md#auto-pager) for details           |
 | `GIT_AUTHOR_NAME`      | `string` | name of the author, used by the rcs backend to create a commit                                         |
 | `GIT_AUTHOR_EMAIL`     | `string` | email of the author, used by the rcs backend to create a commit                                        |
+| `NO_COLOR`             | `bool`   | disable color output. See [no-color.org](https://no-color.org) for more information.
 
 ## Configuration Options
 

docs/features.md

@@ -444,7 +444,7 @@ Running `gopass [generate|insert] foo/bar` on an existing entry `foo/bar` will o
 
 ### Disabling Colors
 
-Disabling colors is as simple as setting `GOPASS_NOCOLOR` to `true`.
+Disabling colors is as simple as setting `NO_COLOR` to `true`. See [no-color.org](https://no-color.org) for more information.
 
 ### Password Templates
 

internal/action/config_test.go

@@ -42,7 +42,6 @@ func TestConfig(t *testing.T) {
 autoimport: true
 cliptimeout: 45
 exportkeys: true
-nocolor: false
 nopager: false
 notifications: true
 parsing: true
@@ -83,7 +82,6 @@ parsing: true
 autoimport: true
 cliptimeout: 45
 exportkeys: true
-nocolor: false
 nopager: true
 notifications: true
 parsing: true
@@ -119,7 +117,6 @@ parsing: true
 autoimport
 cliptimeout
 exportkeys
-nocolor
 nopager
 notifications
 parsing

internal/config/config.go

@@ -21,7 +21,6 @@ type Config struct {
 	AutoImport    bool              `yaml:"autoimport"`    // import missing public keys w/o asking
 	ClipTimeout   int               `yaml:"cliptimeout"`   // clear clipboard after seconds
 	ExportKeys    bool              `yaml:"exportkeys"`    // automatically export public keys of all recipients
-	NoColor       bool              `yaml:"nocolor"`       // do not use color when outputing text
 	NoPager       bool              `yaml:"nopager"`       // do not invoke a pager to display long lists
 	Notifications bool              `yaml:"notifications"` // enable desktop notifications
 	Parsing       bool              `yaml:"parsing"`       // allows to switch off all output parsing

internal/config/config_test.go

@@ -21,7 +21,7 @@ func TestNewConfig(t *testing.T) {
 
 	cfg := config.New()
 	cs := cfg.String()
-	assert.Contains(t, cs, `&config.Config{AutoClip:false, AutoImport:true, ClipTimeout:45, ExportKeys:true, NoColor:false, NoPager:false, Notifications:true,`)
+	assert.Contains(t, cs, `&config.Config{AutoClip:false, AutoImport:true, ClipTimeout:45, ExportKeys:true, NoPager:false, Notifications:true,`)
 	assert.Contains(t, cs, `SafeContent:false, Mounts:map[string]string{},`)
 
 	cfg = &config.Config{
@@ -30,7 +30,7 @@ func TestNewConfig(t *testing.T) {
 	cfg.Mounts["foo"] = ""
 	cfg.Mounts["bar"] = ""
 	cs = cfg.String()
-	assert.Contains(t, cs, `&config.Config{AutoClip:false, AutoImport:false, ClipTimeout:0, ExportKeys:false, NoColor:false, NoPager:false, Notifications:false,`)
+	assert.Contains(t, cs, `&config.Config{AutoClip:false, AutoImport:false, ClipTimeout:0, ExportKeys:false, NoPager:false, Notifications:false,`)
 	assert.Contains(t, cs, `SafeContent:false, Mounts:map[string]string{"bar":"", "foo":""},`)
 }
 

internal/config/io.go

@@ -110,6 +110,7 @@ func decode(buf []byte, relaxed bool) (*Config, error) {
 	cfgs := []configer{
 		// most recent config must come first
 		mostRecent,
+		&Pre1127{},
 		&Pre1102{},
 		&Pre193{
 			Root: &Pre193StoreConfig{},

internal/config/io_test.go

@@ -39,7 +39,6 @@ mounts:
 				AutoImport:    false,
 				ClipTimeout:   45,
 				ExportKeys:    true,
-				NoColor:       false,
 				NoPager:       false,
 				Notifications: true,
 				Parsing:       true,
@@ -69,7 +68,6 @@ mounts:
 				AutoImport:    false,
 				ClipTimeout:   45,
 				ExportKeys:    true,
-				NoColor:       false,
 				NoPager:       false,
 				Notifications: true,
 				Parsing:       true,
@@ -135,7 +133,6 @@ mounts:
 				AutoImport:    false,
 				ClipTimeout:   45,
 				ExportKeys:    false,
-				NoColor:       false,
 				NoPager:       false,
 				Notifications: false,
 				Parsing:       true,
@@ -182,7 +179,6 @@ version: 1.4.0`,
 				AutoImport:    false,
 				ClipTimeout:   45,
 				ExportKeys:    false,
-				NoColor:       false,
 				NoPager:       false,
 				Notifications: false,
 				Parsing:       true,
@@ -213,7 +209,6 @@ version: "1.3.0"`,
 				AutoImport:    true,
 				ClipTimeout:   45,
 				ExportKeys:    false,
-				NoColor:       false,
 				NoPager:       false,
 				Notifications: false,
 				Parsing:       true,
@@ -252,7 +247,6 @@ version: "1.2.0"`,
 				AutoImport:    true,
 				ClipTimeout:   45,
 				ExportKeys:    false,
-				NoColor:       false,
 				NoPager:       false,
 				Notifications: false,
 				Parsing:       true,
@@ -289,7 +283,6 @@ version: 1.1.0`,
 				AutoImport:    false,
 				ClipTimeout:   45,
 				ExportKeys:    false,
-				NoColor:       false,
 				NoPager:       false,
 				Notifications: false,
 				Parsing:       true,
@@ -324,7 +317,6 @@ version: "1.0.0"`,
 				AutoImport:    false,
 				ClipTimeout:   45,
 				ExportKeys:    false,
-				NoColor:       false,
 				NoPager:       false,
 				Notifications: false,
 				Parsing:       true,

internal/config/legacy.go

@@ -5,6 +5,51 @@ import (
 	"strings"
 )
 
+// Pre1127 is a pre-1.12.7 config
+type Pre1127 struct {
+	AutoClip      bool              `yaml:"autoclip"`      // decide whether passwords are automatically copied or not
+	AutoImport    bool              `yaml:"autoimport"`    // import missing public keys w/o asking
+	ClipTimeout   int               `yaml:"cliptimeout"`   // clear clipboard after seconds
+	ExportKeys    bool              `yaml:"exportkeys"`    // automatically export public keys of all recipients
+	NoColor       bool              `yaml:"nocolor"`       // do not use color when outputing text
+	NoPager       bool              `yaml:"nopager"`       // do not invoke a pager to display long lists
+	Notifications bool              `yaml:"notifications"` // enable desktop notifications
+	Parsing       bool              `yaml:"parsing"`       // allows to switch off all output parsing
+	Path          string            `yaml:"path"`
+	SafeContent   bool              `yaml:"safecontent"` // avoid showing passwords in terminal
+	Mounts        map[string]string `yaml:"mounts"`
+
+	ConfigPath string `yaml:"-"`
+
+	// Catches all undefined files and must be empty after parsing
+	XXX map[string]interface{} `yaml:",inline"`
+}
+
+// Config converts the Pre1127 config to the current config struct
+func (c *Pre1127) Config() *Config {
+	cfg := &Config{
+		AutoClip:      c.AutoClip,
+		AutoImport:    c.AutoImport,
+		ClipTimeout:   c.ClipTimeout,
+		ExportKeys:    c.ExportKeys,
+		NoPager:       c.NoPager,
+		Notifications: c.Notifications,
+		Parsing:       true,
+		Path:          c.Path,
+		SafeContent:   c.SafeContent,
+		Mounts:        make(map[string]string, len(c.Mounts)),
+	}
+	for k, v := range c.Mounts {
+		cfg.Mounts[k] = v
+	}
+	return cfg
+}
+
+// CheckOverflow implements configer
+func (c *Pre1127) CheckOverflow() error {
+	return checkOverflow(c.XXX)
+}
+
 // Pre1102 is a pre-1.10.2 config
 type Pre1102 struct {
 	AutoClip      bool              `yaml:"autoclip"`      // decide whether passwords are automatically copied or not
@@ -35,7 +80,6 @@ func (c *Pre1102) Config() *Config {
 		AutoImport:    c.AutoImport,
 		ClipTimeout:   c.ClipTimeout,
 		ExportKeys:    c.ExportKeys,
-		NoColor:       c.NoColor,
 		NoPager:       c.NoPager,
 		Notifications: c.Notifications,
 		Parsing:       true,
@@ -90,7 +134,6 @@ func (c *Pre193) Config() *Config {
 		AutoClip:      c.Root.AutoClip,
 		AutoImport:    c.Root.AutoImport,
 		ClipTimeout:   c.Root.ClipTimeout,
-		NoColor:       c.Root.NoColor,
 		NoPager:       c.Root.NoPager,
 		Notifications: c.Root.Notifications,
 		Parsing:       true,
@@ -153,7 +196,6 @@ func (c *Pre182) Config() *Config {
 		AutoClip:      c.Root.AutoClip,
 		AutoImport:    c.Root.AutoImport,
 		ClipTimeout:   c.Root.ClipTimeout,
-		NoColor:       c.Root.NoColor,
 		NoPager:       c.Root.NoPager,
 		Notifications: c.Root.Notifications,
 		Parsing:       true,

main.go

@@ -243,22 +243,9 @@ func initContext(ctx context.Context, cfg *config.Config) context.Context {
 		ctx = leaf.WithCheckRecipients(ctx, false)
 	}
 
-	// need this override for our integration tests
-	if nc := os.Getenv("GOPASS_NOCOLOR"); nc == "true" || ctxutil.IsNoColor(ctx) {
-		color.NoColor = true
-		ctx = ctxutil.WithColor(ctx, false)
-	}
-
-	// support for no-color.org
-	if nc := os.Getenv("NO_COLOR"); nc != "" {
-		color.NoColor = true
-		ctx = ctxutil.WithColor(ctx, false)
-	}
-
 	// only emit color codes when stdout is a terminal
 	if !isatty.IsTerminal(os.Stdout.Fd()) {
 		color.NoColor = true
-		ctx = ctxutil.WithColor(ctx, false)
 		ctx = ctxutil.WithTerminal(ctx, false)
 		ctx = ctxutil.WithInteractive(ctx, false)
 	}
@@ -274,7 +261,6 @@ func initContext(ctx context.Context, cfg *config.Config) context.Context {
 	// disable this for all terms on this platform
 	if runtime.GOOS == "windows" {
 		color.NoColor = true
-		ctx = ctxutil.WithColor(ctx, false)
 	}
 
 	return ctx

main_test.go

@@ -161,11 +161,4 @@ func TestInitContext(t *testing.T) {
 
 	ctx = initContext(ctx, cfg)
 	assert.Equal(t, true, gpg.IsAlwaysTrust(ctx))
-
-	assert.NoError(t, os.Setenv("GOPASS_DEBUG", "true"))
-	ctx = initContext(ctx, cfg)
-
-	assert.NoError(t, os.Setenv("GOPASS_NOCOLOR", "true"))
-	ctx = initContext(ctx, cfg)
-	assert.Equal(t, false, ctxutil.IsColor(ctx))
 }

pkg/ctxutil/ctxutil.go

@@ -12,15 +12,13 @@ import (
 type contextKey int
 
 const (
-	ctxKeyColor contextKey = iota
-	ctxKeyTerminal
+	ctxKeyTerminal contextKey = iota
 	ctxKeyInteractive
 	ctxKeyStdin
 	ctxKeyNoPager
 	ctxKeyShowSafeContent
 	ctxKeyGitCommit
 	ctxKeyAlwaysYes
-	ctxKeyNoColor
 	ctxKeyVerbose
 	ctxKeyNotifications
 	ctxKeyProgressCallback
@@ -51,21 +49,6 @@ func WithGlobalFlags(c *cli.Context) context.Context {
 // ProgressCallback is a callback for updateing progress
 type ProgressCallback func()
 
-// WithColor returns a context with an explicit value for color
-func WithColor(ctx context.Context, color bool) context.Context {
-	return context.WithValue(ctx, ctxKeyColor, color)
-}
-
-// HasColor returns true if a value for Color has been set in this context
-func HasColor(ctx context.Context) bool {
-	return hasBool(ctx, ctxKeyColor)
-}
-
-// IsColor returns the value of color or the default (true)
-func IsColor(ctx context.Context) bool {
-	return is(ctx, ctxKeyColor, true)
-}
-
 // WithTerminal returns a context with an explicit value for terminal
 func WithTerminal(ctx context.Context, isTerm bool) context.Context {
 	return context.WithValue(ctx, ctxKeyTerminal, isTerm)
@@ -204,26 +187,6 @@ func IsGitCommit(ctx context.Context) bool {
 	return is(ctx, ctxKeyGitCommit, true)
 }
 
-// WithNoColor returns a context with the value for ask for more set
-func WithNoColor(ctx context.Context, bv bool) context.Context {
-	return context.WithValue(ctx, ctxKeyNoColor, bv)
-}
-
-// HasNoColor returns true if a value for NoColor has been set in this context
-func HasNoColor(ctx context.Context) bool {
-	_, ok := ctx.Value(ctxKeyNoColor).(bool)
-	return ok
-}
-
-// IsNoColor returns the value of ask for more or the default (false)
-func IsNoColor(ctx context.Context) bool {
-	bv, ok := ctx.Value(ctxKeyNoColor).(bool)
-	if !ok {
-		return false
-	}
-	return bv
-}
-
 // WithAlwaysYes returns a context with the value of always yes set
 func WithAlwaysYes(ctx context.Context, bv bool) context.Context {
 	return context.WithValue(ctx, ctxKeyAlwaysYes, bv)

pkg/ctxutil/ctxutil_test.go

@@ -9,14 +9,6 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
-func TestColor(t *testing.T) {
-	ctx := context.Background()
-
-	assert.Equal(t, true, IsColor(ctx))
-	assert.Equal(t, true, IsColor(WithColor(ctx, true)))
-	assert.Equal(t, false, IsColor(WithColor(ctx, false)))
-}
-
 func TestTerminal(t *testing.T) {
 	ctx := context.Background()
 
@@ -73,14 +65,6 @@ func TestAlwaysYes(t *testing.T) {
 	assert.Equal(t, false, IsAlwaysYes(WithAlwaysYes(ctx, false)))
 }
 
-func TestNoColor(t *testing.T) {
-	ctx := context.Background()
-
-	assert.Equal(t, false, IsNoColor(ctx))
-	assert.Equal(t, true, IsNoColor(WithNoColor(ctx, true)))
-	assert.Equal(t, false, IsNoColor(WithNoColor(ctx, false)))
-}
-
 func TestVerbose(t *testing.T) {
 	ctx := context.Background()
 
@@ -139,15 +123,13 @@ func TestCommitMessage(t *testing.T) {
 
 func TestComposite(t *testing.T) {
 	ctx := context.Background()
-	ctx = WithColor(ctx, false)
 	ctx = WithTerminal(ctx, false)
 	ctx = WithInteractive(ctx, false)
 	ctx = WithStdin(ctx, true)
 	ctx = WithNoPager(ctx, true)
 	ctx = WithShowSafeContent(ctx, true)
 	ctx = WithGitCommit(ctx, false)
 	ctx = WithAlwaysYes(ctx, true)
-	ctx = WithNoColor(ctx, true)
 	ctx = WithVerbose(ctx, true)
 	ctx = WithNotifications(ctx, true)
 	ctx = WithExportKeys(ctx, false)
@@ -158,9 +140,6 @@ func TestComposite(t *testing.T) {
 	ctx = WithForce(ctx, true)
 	ctx = WithGitInit(ctx, false)
 
-	assert.Equal(t, false, IsColor(ctx))
-	assert.Equal(t, true, HasColor(ctx))
-
 	assert.Equal(t, false, IsTerminal(ctx))
 	assert.Equal(t, true, HasTerminal(ctx))
 
@@ -182,9 +161,6 @@ func TestComposite(t *testing.T) {
 	assert.Equal(t, true, IsAlwaysYes(ctx))
 	assert.Equal(t, true, HasAlwaysYes(ctx))
 
-	assert.Equal(t, true, IsNoColor(ctx))
-	assert.Equal(t, true, HasNoColor(ctx))
-
 	assert.Equal(t, true, IsVerbose(ctx))
 	assert.Equal(t, true, HasVerbose(ctx))