Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect RSpec color mode when applicable #261

Merged
merged 6 commits into from
Sep 22, 2024
Merged

Respect RSpec color mode when applicable #261

merged 6 commits into from
Sep 22, 2024

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