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

Bright versions of colours output incorrectly #125

Open
unikitty37 opened this issue Feb 24, 2021 · 3 comments
Open

Bright versions of colours output incorrectly #125

unikitty37 opened this issue Feb 24, 2021 · 3 comments
Labels

Comments

@unikitty37
Copy link

Setting any colour to its bright_ variant produces incorrect output in super_diff 0.6.1.

Running this test

RSpec.describe SuperDiff do
  it 'deliberately fails' do
    expected = { foo: 1, bar: 2, baz: 3 }
    actual = { bar: 2, baz: 2, quux: 4 }

    expect(actual).to eq(expected)
  end
end

with this config

SuperDiff.configure do |config|
  config.expected_color = :red
end

changes the expected colour to red as expected:

image

If the config is changed to use :bright_red instead of :red, the diff lines are coloured correctly, but the key and the Expected section instead output right_redm instead of changing the colour:

image

I've been able to repeat this with bright_ variations of other colours such as green, black, and blue — though I haven't tried yellow, cyan, white, and magenta, I suspect it will affect them too 😁

Output from `bundle list`

Gems included by the bundle:
  * actioncable (6.1.3)
  * actionmailbox (6.1.3)
  * actionmailer (6.1.3)
  * actionpack (6.1.3)
  * actiontext (6.1.3)
  * actionview (6.1.3)
  * activejob (6.1.3)
  * activemodel (6.1.3)
  * activerecord (6.1.3)
  * activestorage (6.1.3)
  * activesupport (6.1.3)
  * addressable (2.7.0)
  * amazing_print (1.2.1)
  * ast (2.4.2)
  * attr_extras (6.2.4)
  * bcrypt (3.1.16)
  * binding_of_caller (0.8.0)
  * brakeman (5.0.0)
  * builder (3.2.4)
  * bullet (6.1.3)
  * bundler (2.0.2)
  * bundler-audit (0.8.0.rc1)
  * bundler-leak (0.2.0)
  * byebug (11.1.3)
  * claide (1.0.3)
  * claide-plugins (0.9.2)
  * coderay (1.1.3)
  * colored2 (3.1.2)
  * concurrent-ruby (1.1.8)
  * cork (0.3.0)
  * crack (0.4.5)
  * crass (1.0.6)
  * danger (8.2.2)
  * danger-rubocop (0.9.3)
  * database_consistency (0.8.13)
  * debug_inspector (0.0.3)
  * devise (4.7.3)
  * devise-jwt (0.8.1)
  * diff-lcs (1.4.4)
  * docile (1.3.2)
  * dry-auto_inject (0.7.0)
  * dry-configurable (0.12.1)
  * dry-container (0.7.2)
  * dry-core (0.5.0)
  * erubi (1.10.0)
  * et-orbi (1.2.4)
  * factory_bot (6.1.0)
  * factory_bot_rails (6.1.0)
  * faraday (1.3.0)
  * faraday-http-cache (2.2.0)
  * faraday-net_http (1.0.1)
  * ffi (1.14.2)
  * friendly_id (5.4.2)
  * fugit (1.4.2)
  * fuubar (2.5.0)
  * geocoder (1.6.5)
  * git (1.8.1)
  * gitlab (4.17.0)
  * globalid (0.4.2)
  * hashdiff (1.0.1)
  * httparty (0.18.1)
  * i18n (1.8.9)
  * icalendar (2.7.0)
  * ice_cube (0.16.3)
  * image_processing (1.12.1)
  * jbuilder (2.11.2)
  * jwt (2.2.2)
  * kramdown (2.3.0)
  * kramdown-parser-gfm (1.1.0)
  * kwalify (0.7.2)
  * launchy (2.5.0)
  * listen (3.2.1)
  * logstop (0.2.6)
  * loofah (2.9.0)
  * mail (2.7.1)
  * marcel (0.3.3)
  * method_source (1.0.0)
  * mime-types (3.3.1)
  * mime-types-data (3.2021.0212)
  * mimemagic (0.3.5)
  * mini_magick (4.11.0)
  * mini_mime (1.0.2)
  * mini_portile2 (2.5.0)
  * minitest (5.14.4)
  * mono_logger (1.1.0)
  * multi_json (1.15.0)
  * multi_xml (0.6.0)
  * multipart-post (2.1.1)
  * mustermann (1.1.1)
  * nap (1.1.0)
  * nio4r (2.5.5)
  * no_proxy_fix (0.1.2)
  * nokogiri (1.11.1)
  * octokit (4.20.0)
  * open4 (1.3.4)
  * orm_adapter (0.5.0)
  * parallel (1.20.1)
  * parser (3.0.0.0)
  * patience_diff (1.1.0)
  * pg (1.2.3)
  * pry (0.13.1)
  * pry-byebug (3.9.0)
  * pry-doc (1.1.0)
  * pry-rails (0.3.9)
  * pry-theme (1.3.0)
  * psych (3.3.0)
  * public_suffix (4.0.6)
  * puma (4.3.5)
  * pundit (2.1.0)
  * pundit-matchers (1.6.0)
  * raabro (1.4.0)
  * racc (1.5.2)
  * rack (2.2.3)
  * rack-cors (1.1.1)
  * rack-protection (2.0.8.1)
  * rack-test (1.1.0)
  * rails (6.1.3)
  * rails-dom-testing (2.0.3)
  * rails-html-sanitizer (1.3.0)
  * railties (6.1.3)
  * rainbow (3.0.0)
  * rake (13.0.3)
  * rb-fsevent (0.10.4)
  * rb-inotify (0.10.1)
  * rchardet (1.8.0)
  * redis (4.2.5)
  * redis-namespace (1.8.1)
  * reek (6.0.3)
  * regexp_parser (2.1.1)
  * rein (5.1.0)
  * responders (3.0.1)
  * resque (2.0.0)
  * rexml (3.2.4)
  * rspec-collection_matchers (1.2.0)
  * rspec-core (3.10.1)
  * rspec-expectations (3.10.1)
  * rspec-mocks (3.10.2)
  * rspec-rails (4.0.2)
  * rspec-support (3.10.2)
  * rspec_junit_formatter (0.4.1)
  * rubocop (1.10.0)
  * rubocop-ast (1.4.1)
  * rubocop-performance (1.9.2)
  * rubocop-rails (2.9.1)
  * rubocop-rspec (2.2.0)
  * rubocop-thread_safety (0.4.2)
  * ruby-progressbar (1.11.0)
  * ruby-vips (2.0.17)
  * ruby2_keywords (0.0.2)
  * rufus-scheduler (3.7.0)
  * sawyer (0.8.2)
  * sentry-raven (3.1.1)
  * simplecov (0.18.5)
  * simplecov-html (0.12.2)
  * sinatra (2.0.8.1)
  * sitemap_generator (6.1.2)
  * sprockets (4.0.2)
  * sprockets-rails (3.2.2)
  * strong_migrations (0.7.6)
  * super_diff (0.6.1)
  * terminal-table (1.6.0)
  * thor (1.1.0)
  * tilt (2.0.10)
  * trollop (1.16.2)
  * tzinfo (2.0.4)
  * unicode-display_width (2.0.0)
  * uniform_notifier (1.13.2)
  * validate_url (1.0.13)
  * vegas (0.1.11)
  * warden (1.2.9)
  * warden-jwt_auth (0.5.0)
  * webmock (3.11.2)
  * websocket-driver (0.7.3)
  * websocket-extensions (0.1.5)
  * yard (0.9.25)
  * zeitwerk (2.4.2)

@mcmire
Copy link
Collaborator

mcmire commented Feb 24, 2021

Whoops! It's possible that I haven't completely tested this. I will look at this shortly!

@mcmire
Copy link
Collaborator

mcmire commented May 19, 2021

I looked into this briefly and there's two things going on here. First, super_diff uses RSpec's color set to colorize some pieces of the output and its own color set to colorize other pieces. Second, super_diff patches RSpec so that if it looks up a color in RSpec's color set and can't find anything, it just uses the input anyway. I can't remember why I did this exactly, but that's why you're getting garbage in the output.

So I think in order to fix this, that patch probably needs to be reverted, and there needs to be one way to color text and not two. At the moment I am leaning toward constraining super_diff's colorization capabilities to match those of RSpec's, to limit the amount of custom stuff super_diff does. That means that bright colors would not be supported in super_diff. That said, if you feel like bright colors is something you really want then I could be convinced otherwise!

@unikitty37
Copy link
Author

Thanks — the main problem is that only some of the dark colours are readable on a black background on my terminal.

I could redefine the colours in iTerm 2's config, but as I'm using its 256 colour support, I'm not sure if that even works. (Support for the colours of xterm-256 would be lovely, but probably a bit much to hope for :)

That said, the main colour I was trying to change was the border colour, so it's not essential in my case, as the actual diffs are readable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants