From 20282b189a8d639f43222d59e9b1374e9f69b0a0 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Thu, 30 Nov 2023 17:34:45 +0200 Subject: [PATCH] prevent---color-with-non-text-formats (#449) --- BREAKING-CHANGES.md | 4 ++-- CHANGELOG.md | 6 +++--- internal/breaking_changes.go | 2 +- internal/changelog.go | 2 +- internal/handlers.go | 16 ++++++++++++++++ internal/run_test.go | 10 ++++++++++ 6 files changed, 33 insertions(+), 7 deletions(-) diff --git a/BREAKING-CHANGES.md b/BREAKING-CHANGES.md index e6122761..e39b9d9a 100644 --- a/BREAKING-CHANGES.md +++ b/BREAKING-CHANGES.md @@ -29,9 +29,9 @@ You can specify the `--format` flag to output breaking changes in other formats: An additional format `singleline` displays each breaking change on a single line, this can be useful to prepare [ignore files](#ignoring-specific-breaking-changes) ### Color -When outputting breaking changes to a Unix terminal, oasdiff automatically adds colors with ANSI color codes. +When outputting breaking changes to a Unix terminal, oasdiff automatically adds colors with ANSI color escape sequences. If output is piped into another process or redirected to a file, oasdiff disables color. -To control color manually, use the `color` flag with `always` or `never`. +To control color manually, use the `--color` flag with `always` or `never`. ### API Stability Levels When a new API is introduced, you may want to allow developers to change its behavior without triggering a breaking change error. diff --git a/CHANGELOG.md b/CHANGELOG.md index ad6934b8..c0b8bf8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,12 +23,12 @@ oasdiff checks ### Output Formats By default, changes are displayed as human-readable text with [color](#color). You can specify the `--format` flag to output changes in other formats: `json`, `yaml`, `html`, `githubactions` or `junit`. -An additional format `singleline` displays each change on a single line, this can be useful to prepare [ignore files](#ignoring-specific-breaking-changes) +An additional format `singleline` displays each change on a single line, this can be useful to prepare [ignore files](BREAKING-CHANGES.md#ignoring-specific-breaking-changes) ### Color -When outputting changes to a Unix terminal, oasdiff automatically adds colors with ANSI color codes. +When outputting changes to a Unix terminal, oasdiff automatically adds colors with ANSI color escape sequences. If output is piped into another process or redirected to a file, oasdiff disables color. -To control color manually, use the `color` flag with `always` or `never`. +To control color manually, use the `--color` flag with `always` or `never`. ### Customizing the Changelog If you encounter a change that isn't logged by oasdiff you may add a [custom check](CUSTOMIZING-CHECKS.md). diff --git a/internal/breaking_changes.go b/internal/breaking_changes.go index 78134849..d3f791fe 100644 --- a/internal/breaking_changes.go +++ b/internal/breaking_changes.go @@ -42,7 +42,7 @@ func getBreakingChangesCmd() *cobra.Command { cmd.PersistentFlags().VarP(newEnumSliceValue(checker.GetOptionalChecks(), nil, &flags.includeChecks), "include-checks", "i", "comma-separated list of optional checks (run 'oasdiff checks --required false' to see options)") cmd.PersistentFlags().IntVarP(&flags.deprecationDaysBeta, "deprecation-days-beta", "", checker.BetaDeprecationDays, "min days required between deprecating a beta resource and removing it") cmd.PersistentFlags().IntVarP(&flags.deprecationDaysStable, "deprecation-days-stable", "", checker.StableDeprecationDays, "min days required between deprecating a stable resource and removing it") - enumWithOptions(&cmd, newEnumValue([]string{"auto", "always", "never"}, "auto", &flags.color), "color", "", "when to output colored escape sequences") + enumWithOptions(&cmd, newEnumValue([]string{"auto", "always", "never"}, "auto", &flags.color), "color", "", "when to colorize textual output") return &cmd } diff --git a/internal/changelog.go b/internal/changelog.go index c7443775..b625c96e 100644 --- a/internal/changelog.go +++ b/internal/changelog.go @@ -43,7 +43,7 @@ func getChangelogCmd() *cobra.Command { cmd.PersistentFlags().VarP(newEnumSliceValue(checker.GetOptionalChecks(), nil, &flags.includeChecks), "include-checks", "i", "comma-separated list of optional checks (run 'oasdiff checks --required false' to see options)") cmd.PersistentFlags().IntVarP(&flags.deprecationDaysBeta, "deprecation-days-beta", "", checker.BetaDeprecationDays, "min days required between deprecating a beta resource and removing it") cmd.PersistentFlags().IntVarP(&flags.deprecationDaysStable, "deprecation-days-stable", "", checker.StableDeprecationDays, "min days required between deprecating a stable resource and removing it") - enumWithOptions(&cmd, newEnumValue([]string{"auto", "always", "never"}, "auto", &flags.color), "color", "", "when to output colored escape sequences") + enumWithOptions(&cmd, newEnumValue([]string{"auto", "always", "never"}, "auto", &flags.color), "color", "", "when to colorize textual output") return &cmd } diff --git a/internal/handlers.go b/internal/handlers.go index d67d3eec..2297193d 100644 --- a/internal/handlers.go +++ b/internal/handlers.go @@ -25,6 +25,9 @@ func getParseArgs(flags Flags) cobra.PositionalArgs { return errors.New("can't read from stdin in composed mode") } } + if err := checkColor(cmd); err != nil { + return err + } return nil } @@ -54,3 +57,16 @@ func getRun(flags Flags, runner runner) cobra.PositionalArgs { return nil } } + +func checkColor(cmd *cobra.Command) error { + + if colorPassed := cmd.Flags().Changed("color"); !colorPassed { + return nil + } + + if format, _ := cmd.Flags().GetString("format"); format == "text" || format == "singleline" { + return nil + } + + return errors.New(`--color flag is only relevant with 'text' or 'singleline' formats`) +} diff --git a/internal/run_test.go b/internal/run_test.go index bba4c91c..c093eca0 100644 --- a/internal/run_test.go +++ b/internal/run_test.go @@ -278,3 +278,13 @@ func Test_FlattenInvalid(t *testing.T) { func Test_Checks(t *testing.T) { require.Zero(t, internal.Run(cmdToArgs("oasdiff checks -l ru"), io.Discard, io.Discard)) } + +func Test_Color(t *testing.T) { + require.Zero(t, internal.Run(cmdToArgs("oasdiff breaking ../data/allof/simple.yaml ../data/allof/revision.yaml --color always"), io.Discard, io.Discard)) +} + +func Test_ColorWithNonTextFormat(t *testing.T) { + var stderr bytes.Buffer + require.Equal(t, 100, internal.Run(cmdToArgs("oasdiff changelog ../data/allof/simple.yaml ../data/allof/revision.yaml -f yaml --color always"), io.Discard, &stderr)) + require.Equal(t, "Error: --color flag is only relevant with 'text' or 'singleline' formats\n", stderr.String()) +}