From f68900627999331e439e4afd27a37126d2efa89d Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Sat, 21 Feb 2026 15:59:42 +0900 Subject: [PATCH] change cli option / config name validate better --- config/config.go | 32 +++++++++++++++++++- config/config_test.go | 68 +++++++++++++++++++++++++++++++++++++++++++ options.go | 3 +- peco.go | 7 ++++- peco_test.go | 13 +++++++-- 5 files changed, 117 insertions(+), 6 deletions(-) diff --git a/config/config.go b/config/config.go index 7cc8177..c44a564 100644 --- a/config/config.go +++ b/config/config.go @@ -34,6 +34,36 @@ func (o *OnCancelBehavior) UnmarshalText(b []byte) error { return nil } +// ColorMode specifies how peco handles ANSI color codes in input. +type ColorMode string + +const ( + ColorModeAuto ColorMode = "auto" + ColorModeNone ColorMode = "none" +) + +func (c *ColorMode) unmarshal(s string) error { + switch s { + case "", "auto": + *c = ColorModeAuto + case "none": + *c = ColorModeNone + default: + return fmt.Errorf("invalid Color value %q: must be %q or %q", s, ColorModeAuto, ColorModeNone) + } + return nil +} + +// UnmarshalText implements encoding.TextUnmarshaler (used by JSON/YAML decoders). +func (c *ColorMode) UnmarshalText(b []byte) error { + return c.unmarshal(string(b)) +} + +// UnmarshalFlag implements go-flags Unmarshaler (used by CLI flag parsing). +func (c *ColorMode) UnmarshalFlag(s string) error { + return c.unmarshal(s) +} + // Config holds all the data that can be configured in the // external configuration file type Config struct { @@ -54,7 +84,7 @@ type Config struct { FilterBufSize int `json:"FilterBufSize" yaml:"FilterBufSize"` FuzzyLongestSort bool `json:"FuzzyLongestSort" yaml:"FuzzyLongestSort"` SuppressStatusMsg bool `json:"SuppressStatusMsg" yaml:"SuppressStatusMsg"` - ANSI bool `json:"ANSI" yaml:"ANSI"` + Color ColorMode `json:"Color" yaml:"Color"` // If this is true, then the prefix for single key jump mode // is displayed by default. diff --git a/config/config_test.go b/config/config_test.go index 47ccf6d..d083db4 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -267,6 +267,74 @@ func TestOnCancelBehavior(t *testing.T) { }) } +func TestColorMode(t *testing.T) { + t.Run("valid values via JSON", func(t *testing.T) { + for _, tc := range []struct { + input string + expected ColorMode + }{ + {`{"Color":"auto"}`, ColorModeAuto}, + {`{"Color":"none"}`, ColorModeNone}, + {`{}`, ""}, // absent key stays at zero value; default applied later in ApplyConfig + } { + var cfg Config + require.NoError(t, cfg.Init()) + require.NoError(t, json.Unmarshal([]byte(tc.input), &cfg)) + require.Equal(t, tc.expected, cfg.Color) + } + }) + + t.Run("valid values via YAML", func(t *testing.T) { + for _, tc := range []struct { + input string + expected ColorMode + }{ + {"Color: auto", ColorModeAuto}, + {"Color: none", ColorModeNone}, + } { + var cfg Config + require.NoError(t, cfg.Init()) + require.NoError(t, yaml.Unmarshal([]byte(tc.input), &cfg)) + require.Equal(t, tc.expected, cfg.Color) + } + }) + + t.Run("invalid value via JSON", func(t *testing.T) { + var cfg Config + require.NoError(t, cfg.Init()) + err := json.Unmarshal([]byte(`{"Color":"bogus"}`), &cfg) + require.Error(t, err) + require.Contains(t, err.Error(), "bogus") + }) + + t.Run("invalid value via YAML", func(t *testing.T) { + var cfg Config + require.NoError(t, cfg.Init()) + err := yaml.Unmarshal([]byte("Color: bogus"), &cfg) + require.Error(t, err) + require.Contains(t, err.Error(), "bogus") + }) + + t.Run("UnmarshalFlag valid values", func(t *testing.T) { + var c ColorMode + require.NoError(t, c.UnmarshalFlag("auto")) + require.Equal(t, ColorModeAuto, c) + + require.NoError(t, c.UnmarshalFlag("none")) + require.Equal(t, ColorModeNone, c) + + require.NoError(t, c.UnmarshalFlag("")) + require.Equal(t, ColorModeAuto, c) + }) + + t.Run("UnmarshalFlag invalid value", func(t *testing.T) { + var c ColorMode + err := c.UnmarshalFlag("bogus") + require.Error(t, err) + require.Contains(t, err.Error(), "bogus") + }) +} + func TestReadFilenameYAML(t *testing.T) { dir := t.TempDir() yamlFile := filepath.Join(dir, "config.yaml") diff --git a/options.go b/options.go index cda3309..6e15fee 100644 --- a/options.go +++ b/options.go @@ -31,7 +31,7 @@ type CLIOptions struct { OptSelectionPrefix string `long:"selection-prefix" description:"use a prefix instead of changing line color to indicate currently selected lines.\ndefault is to use colors. This option is experimental"` OptExec string `long:"exec" description:"execute command instead of finishing/terminating peco.\nPlease note that this command will receive selected line(s) from stdin,\nand will be executed via '/bin/sh -c' or 'cmd /c'"` OptPrintQuery bool `long:"print-query" description:"print out the current query as first line of output"` - OptColor string `long:"color" description:"color mode: 'auto' (default, parse ANSI codes) or 'none' (disable)" default:"auto"` + OptColor config.ColorMode `long:"color" description:"color mode: 'auto' (default, parse ANSI codes) or 'none' (disable)" default:"auto"` OptHeight string `long:"height" description:"display height in lines or percentage (e.g. '10', '50%')"` } @@ -58,6 +58,7 @@ func (options CLIOptions) Validate() error { return errors.New("unknown layout: '" + options.OptLayout + "'") } } + return nil } diff --git a/peco.go b/peco.go index 879de5a..6c008fa 100644 --- a/peco.go +++ b/peco.go @@ -754,7 +754,12 @@ func (p *Peco) ApplyConfig(opts CLIOptions) error { } p.enableSep = opts.OptEnableNullSep - p.enableANSI = opts.OptColor != "none" + switch opts.OptColor { + case config.ColorModeNone: + p.enableANSI = false + default: + p.enableANSI = true + } if i := opts.OptInitialIndex; i >= 0 { p.Location().SetLineNumber(i) diff --git a/peco_test.go b/peco_test.go index 8425145..882603d 100644 --- a/peco_test.go +++ b/peco_test.go @@ -384,7 +384,7 @@ func TestApplyConfig(t *testing.T) { opts.OptSelectionPrefix = ">" opts.OptPrintQuery = true opts.OptExec = "cat" - opts.OptColor = "auto" + opts.OptColor = config.ColorModeAuto opts.OptHeight = "20" p := newPeco() @@ -517,14 +517,21 @@ func TestApplyConfig(t *testing.T) { // --color=none → enableANSI is false p2 := newPeco() - require.NoError(t, p2.ApplyConfig(CLIOptions{OptColor: "none"}), "p.ApplyConfig should succeed") + require.NoError(t, p2.ApplyConfig(CLIOptions{OptColor: config.ColorModeNone}), "p.ApplyConfig should succeed") require.False(t, p2.enableANSI, "p.enableANSI should be false when OptColor is 'none'") // --color=auto → enableANSI is true p3 := newPeco() - require.NoError(t, p3.ApplyConfig(CLIOptions{OptColor: "auto"}), "p.ApplyConfig should succeed") + require.NoError(t, p3.ApplyConfig(CLIOptions{OptColor: config.ColorModeAuto}), "p.ApplyConfig should succeed") require.True(t, p3.enableANSI, "p.enableANSI should be true when OptColor is 'auto'") }) + + t.Run("Invalid --color value is rejected", func(t *testing.T) { + var c config.ColorMode + err := c.UnmarshalFlag("bogus") + require.Error(t, err, "invalid --color value should be rejected") + require.Contains(t, err.Error(), "bogus") + }) } // While this issue is labeled for Issue363, it tests against 376 as well.