Skip to content

Conversation

@jas14
Copy link
Collaborator

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.
@github-actions
Copy link

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

@github-actions
Copy link

@jas14jas14 marked this pull request as ready for review September 22, 2024 20:12
@jas14jas14 merged commit eae2879 into mainSep 22, 2024
@jas14jas14 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 freeto 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

@jas14@dgmstuart