Skip to content

Conversation

jas14
Copy link
Collaborator

@jas14 jas14 commented Sep 21, 2024

Problem

As described in #260, it's surprising and undesirable for RSpec to emit colorized output when that's been explicitly turned off (either via --no-color on the CLI, or by setting RSpec.configuration.color_mode = :off programmatically).

Solution

With RSpec extensions on

When this gem's RSpec extensions are loaded, and:

  • ENV["CI"] isn't set to true, and
  • SuperDiff.configuration.color_enabled wasn't explicitly set or was set to nil,

we'll use RSpec's color mode by default.

⚠️ 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.

With RSpec extensions off

When this gem's RSpec extensions are not loaded, we won't use the RSpec color configuration, even if RSpec is loaded in the environment. This prevents a different weird scenario where:

  1. RSpec just happens to be loaded, but
  2. The user loaded SuperDiff for reasons unrelated to RSpec, and consequently
  3. SuperDiff uses RSpec's color config by default.

It's hard to keep Csi.color_enabled and
SuperDiff::Configuration.color_enabled in sync. But Csi.color_enabled is
only serving Csi.colorize, which in turn only exists for
Csi.inspect_colors_in, which is apparently unused elsewhere in this
project.

Since Csi is a library private to this project, we can remove them.
When `color_enabled` has not been explicitly set and `super_diff/rspec`
has been loaded (i.e. whoever is using this gem has indicated they want
to load the RSpec extensions, not just that RSpec is available), use the
RSpec color mode if one hasn't been explicitly set already.
Copy link

📖 A new version of the docsite has been published at: https://splitwise.github.io/super_diff/branches/261/merge/e5f93d4edad4fe00762f823d6bbc4838a9d83ad6

Copy link

📖 A new version of the docsite has been published at: https://splitwise.github.io/super_diff/branches/respect-rspec-color/e5ba527f9a6c1779be1ebb40b5e9b32af7a09e6f

@jas14 jas14 marked this pull request as ready for review September 22, 2024 20:12
@jas14 jas14 merged commit eae2879 into main Sep 22, 2024
31 checks passed
@jas14 jas14 deleted the respect-rspec-color branch September 22, 2024 20:14
@dgmstuart
Copy link

Thanks so much for addressing this so quickly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants