Respect RSpec color mode when applicable (#261)

jas14 · web-flow · commit eae2879865cd · 2024-09-22T13:14:05.000-07:00
Default to RSpec color mode when SuperDIff RSpec extensions are loaded (and when `ENV["CI"]` isn't set to `true`). ⚠️ This is technically a breaking change. Before this PR, `SuperDiff.configuration.color_enabled = nil` turned colors off. As of this PR, it indicates that SuperDiff should decide based on the environment.
diff --git a/.github/workflows/super_diff.yml b/.github/workflows/super_diff.yml
@@ -143,7 +143,7 @@ jobs:
         env:
           IS_NEW_RELEASE: ${{ needs.collect-release-info.outputs.IS_NEW_RELEASE }}
           RELEASE_VERSION: ${{ needs.collect-release-info.outputs.RELEASE_VERSION }}
-          BRANCH_NAME: ${{ github.ref_name }}
+          BRANCH_NAME: ${{ github.head_ref || github.ref_name }}
           COMMIT_ID: ${{ github.event.pull_request.head.sha }}
     outputs:
       DOCSITE_RELEASE_VERSION: ${{ steps.command.outputs.DOCSITE_RELEASE_VERSION }}
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,13 @@
 
 ### Features
 
-- Add better support for Data object diffing. [#259](https://github.com/splitwise/super_diff/pull/224)
+- Add better support for Data object diffing. [#259](https://github.com/splitwise/super_diff/pull/259)
+- Fall back on RSpec color mode when `SuperDiff.configuration.color_enabled` is unspecified or nil. [#261](https://github.com/splitwise/super_diff/pull/261)
+
+### Breaking changes
+
+- Removed several `SuperDiff::Csi` methods. This will break any code that uses those parts of the `SuperDiff::Csi` (which is private in general).
+- `SuperDiff.configuration.color_enabled = nil` used to disable color output. It now allows SuperDiff to determine whether to colorize output based on the environment (namely RSpec color mode and whether stdout is a TTY).
 
 ## 0.12.1 - 2024-04-26
 
diff --git a/docs/users/customization.md b/docs/users/customization.md
@@ -14,17 +14,17 @@ end
 The following is a list of options you can set on the configuration object
 along with their defaults:
 
-| name                   | description                                                                   | default                                                     |
-| ---------------------- | ----------------------------------------------------------------------------- | ----------------------------------------------------------- |
-| `actual_color`         | The color used to display "actual" values in diffs                            | `:yellow`                                                   |
-| `border_color`         | The color used to display the border in diff keys                             | `:blue`                                                     |
-| `color_enabled`        | Whether to colorize output                                                    | `true` if `ENV["CI"]` or stdout is a TTY, `false` otherwise |
-| `diff_elision_enabled` | Whether to elide (remove) unchanged lines in diff                             | `false`                                                     |
-| `diff_elision_maximum` | How large a section of consecutive unchanged lines can be before being elided | `0`                                                         |
-| `elision_marker_color` | The color used to display the marker substituted for elided lines in a diff   | `:cyan`                                                     |
-| `expected_color`       | The color used to display "expected" values in diffs                          | `:magenta`                                                  |
-| `header_color`         | The color used to display the "Diff:" header in failure messages              | `:white`                                                    |
-| `key_enabled`          | Whether to show the key above diffs                                           | `true`                                                      |
+| name                   | description                                                                           | default    |
+| ---------------------- | ------------------------------------------------------------------------------------- | ---------- |
+| `actual_color`         | The color used to display "actual" values in diffs                                    | `:yellow`  |
+| `border_color`         | The color used to display the border in diff keys                                     | `:blue`    |
+| `color_enabled`        | Whether to colorize output, or `nil` to let SuperDiff decide based on the environment | `nil`      |
+| `diff_elision_enabled` | Whether to elide (remove) unchanged lines in diff                                     | `false`    |
+| `diff_elision_maximum` | How large a section of consecutive unchanged lines can be before being elided         | `0`        |
+| `elision_marker_color` | The color used to display the marker substituted for elided lines in a diff           | `:cyan`    |
+| `expected_color`       | The color used to display "expected" values in diffs                                  | `:magenta` |
+| `header_color`         | The color used to display the "Diff:" header in failure messages                      | `:white`   |
+| `key_enabled`          | Whether to show the key above diffs                                                   | `true`     |
 
 The following is a list of methods you can call on the configuration object:
 
diff --git a/lib/super_diff.rb b/lib/super_diff.rb
@@ -36,7 +36,6 @@ def self.const_missing(missing_const_name)
 
   def self.configure
     yield configuration
-    configuration.updated
   end
 
   def self.configuration
diff --git a/lib/super_diff/core/configuration.rb b/lib/super_diff/core/configuration.rb
@@ -23,7 +23,7 @@ class Configuration
       def initialize(options = {})
         @actual_color = :yellow
         @border_color = :blue
-        @color_enabled = color_enabled_by_default?
+        @color_enabled = nil
         @diff_elision_enabled = false
         @diff_elision_maximum = 0
         @elision_marker_color = :cyan
@@ -41,6 +41,8 @@ def initialize(options = {})
 
       def initialize_dup(original)
         super
+        @extra_diff_formatter_classes =
+          original.extra_diff_formatter_classes.dup.freeze
         @extra_differ_classes = original.extra_differ_classes.dup.freeze
         @extra_operation_tree_builder_classes =
           original.extra_operation_tree_builder_classes.dup.freeze
@@ -51,6 +53,8 @@ def initialize_dup(original)
       end
 
       def color_enabled?
+        return color_enabled_by_default? if @color_enabled.nil?
+
         @color_enabled
       end
 
@@ -71,12 +75,6 @@ def merge!(configuration_or_options)
           end
 
         options.each { |key, value| instance_variable_set("@#{key}", value) }
-
-        updated
-      end
-
-      def updated
-        SuperDiff::Csi.color_enabled = color_enabled?
       end
 
       def add_extra_diff_formatter_classes(*classes)
@@ -165,7 +163,7 @@ def to_h
         {
           actual_color: actual_color,
           border_color: border_color,
-          color_enabled: color_enabled?,
+          color_enabled: @color_enabled,
           diff_elision_enabled: diff_elision_enabled?,
           diff_elision_maximum: diff_elision_maximum,
           elision_marker_color: elision_marker_color,
@@ -185,7 +183,13 @@ def to_h
       private
 
       def color_enabled_by_default?
-        ENV["CI"] == "true" || $stdout.respond_to?(:tty?) && $stdout.tty?
+        return true if ENV["CI"] == "true"
+
+        if defined?(::SuperDiff::RSpec)
+          return ::RSpec.configuration.color_enabled?
+        end
+
+        $stdout.respond_to?(:tty?) && $stdout.tty?
       end
     end
   end
diff --git a/lib/super_diff/core/helpers.rb b/lib/super_diff/core/helpers.rb
@@ -6,7 +6,7 @@ module Helpers
       # TODO: Simplify this
       def style(*args, color_enabled: true, **opts, &block)
         klass =
-          if color_enabled && Csi.color_enabled?
+          if color_enabled && SuperDiff.configuration.color_enabled?
             Csi::ColorizedDocument
           else
             Csi::UncolorizedDocument
diff --git a/lib/super_diff/csi.rb b/lib/super_diff/csi.rb
@@ -12,48 +12,16 @@ module Csi
     autoload :TwentyFourBitColor, "super_diff/csi/twenty_four_bit_color"
     autoload :UncolorizedDocument, "super_diff/csi/uncolorized_document"
 
-    class << self
-      attr_writer :color_enabled
-    end
-
     def self.reset_sequence
       ResetSequence.new
     end
 
-    def self.color_enabled?
-      @color_enabled
-    end
-
-    def self.colorize(*args, **opts, &block)
-      if color_enabled?
-        ColorizedDocument.new(*args, **opts, &block)
-      else
-        UncolorizedDocument.new(*args, **opts, &block)
-      end
-    end
-
     def self.decolorize(text)
       text.gsub(/\e\[\d+(?:;\d+)*m(.+?)\e\[0m/, '\1')
     end
 
     def self.already_colorized?(text)
       text.match?(/\e\[\d+m/)
     end
-
-    def self.inspect_colors_in(text)
-      [FourBitColor, EightBitColor, TwentyFourBitColor].reduce(
-        text
-      ) do |str, klass|
-        klass.sub_colorized_areas_in(str) do |area, color|
-          color_block = colorize("◼︎", color.to_foreground)
-
-          layer_indicator = (color.foreground? ? "(fg)" : "(bg)")
-
-          "#{color_block} #{layer_indicator} ❮#{area}❯"
-        end
-      end
-    end
-
-    self.color_enabled = false
   end
 end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
@@ -68,6 +68,7 @@
   unless defined?(ActiveSupport)
     config.filter_run_excluding active_support: true
   end
+  config.filter_run_excluding with_superdiff_rspec: false
 
   config.order = :random
   Kernel.srand config.seed
diff --git a/spec/unit/core/configuration_no_rspec_spec.rb b/spec/unit/core/configuration_no_rspec_spec.rb
@@ -0,0 +1,58 @@
+require "delegate"
+require "super_diff"
+
+class FakeTTYDecorator < SimpleDelegator
+  def initialize(obj, is_tty:)
+    super(obj)
+    @is_tty = is_tty
+  end
+
+  def isatty = @is_tty
+  def tty? = isatty
+end
+
+RSpec.describe SuperDiff::Core::Configuration, with_superdiff_rspec: false do
+  describe "#color_enabled?" do
+    it "is true when stdout is a TTY" do
+      original_stdout = $stdout
+      color_enabled = nil
+      begin
+        $stdout = FakeTTYDecorator.new(StringIO.new, is_tty: true)
+        color_enabled = SuperDiff.configuration.color_enabled?
+      ensure
+        $stdout = original_stdout
+      end
+      expect(color_enabled).to be(true)
+    end
+
+    it "is false when stdout is not a TTY but we are in CI" do
+      original_stdout = $stdout
+      original_ci = ENV["CI"]
+      color_enabled = nil
+      begin
+        $stdout = FakeTTYDecorator.new(StringIO.new, is_tty: false)
+        ENV["CI"] = "true"
+        color_enabled = SuperDiff.configuration.color_enabled?
+      ensure
+        $stdout = original_stdout
+        ENV["CI"] = original_ci
+      end
+      expect(color_enabled).to be(true)
+    end
+
+    it "is false when stdout is not a TTY and we are not in CI" do
+      original_stdout = $stdout
+      original_ci = ENV["CI"]
+      color_enabled = nil
+      begin
+        $stdout = FakeTTYDecorator.new(StringIO.new, is_tty: false)
+        ENV["CI"] = nil
+        color_enabled = SuperDiff.configuration.color_enabled?
+      ensure
+        $stdout = original_stdout
+        ENV["CI"] = original_ci
+      end
+      expect(color_enabled).to be(false)
+    end
+  end
+end
diff --git a/spec/unit/core/configuration_spec.rb b/spec/unit/core/configuration_spec.rb
