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

Consider tweaking colors to draw eye to differences #14

Open
lread opened this issue Sep 1, 2019 · 12 comments
Open

Consider tweaking colors to draw eye to differences #14

lread opened this issue Sep 1, 2019 · 12 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@lread
Copy link
Contributor

lread commented Sep 1, 2019

Thanks so much for the kaocha, I am really enjoying using it!

Some of my integration tests include the comparison of largish data structures. It can take my eyeballs a while to find the diff between expected and actual in these data structures.

I do sometimes use the search feature in my terminal to search for + and - and that can help and I was thinking focusing only on the differences, as #13 suggests, would help.

But I am also thinking perhaps a different choice of colors might help. I like the choice of colors chosen for differences, but I find that the red can take a while to spot because it is already used in non-diff syntax highlighting.

My initial thoughts are:

  1. change the red to something else in default syntax highlighting, maybe blue.
  2. take advantage of background colors to really draw the eye to changes. We might even be able to keep the normal syntax highlighting foreground colors with this scheme.

I am happy to give some color schemes a whirl for review, then a PR with changes, if there is interest in this idea.

@lread
Copy link
Contributor Author

lread commented Sep 5, 2019

Chatted on Slack with @plexus about this one. Summary:

  1. Go ahead with some mockups so we can get a feel for what this might look like
  2. Bear in mind that some people will not like the change, so in addition to any new coloring we'll still need to support the current coloring through some sort of config (analysis to come if we decide to proceed) - default would very likely be current coloring.

@lread
Copy link
Contributor Author

lread commented Sep 5, 2019

Ok, I played around with some mockups.

Everybody likely has different terminal color schemes and fonts setup. I did my testing on macOS using iTerm2 using the "Roboto Mono for Powerline" font with the "Solarized Dark" color preset for for dark background testing and "Solarized Light" for light background testing. Note that the Solarized themes do compensate for yellow looking terrible on a light background.

README screenshot
Here is the example presented on the deep-diff README...
image
It looks like whoever took this screenshot was using a similar terminal color scheme similar to the one I am using for my tests.

Option A) baseline
If I run README example against current code, I get:
image
This closely matches the README screenshot.

On a light background:
image

I won't explore it more unless requested, but if a terminal color scheme is chosen that does not compensate for yellow, you'll see something like this on a light background:
image

Option B) Option A + blue delimiter characters
image
To me, this maybe a bit better, my eye is drawn to the red of the change.

Option C) Option A + background colors for insertions/deletions
image
Ok. no.

Option D) Option C + with black foreground for insertions/deletions
image
To me, this is great, I see my insertions and deletions clearly.

Option E) Option D + Option B
And one last variation that combines option B and D
image
To me, Option E is slightly better than Option D as the distracting red is gone.

Let's see what this candidate looks like on a light background:
image
Looks good to me.

I will follow up with another message exploring how this looks with a larger diff.

@lread
Copy link
Contributor Author

lread commented Sep 5, 2019

Where coloring changes really shine are for larger diffs. Let's compare the current highlighting of a larger diff to Option E highlighting.

Option A) larger diff
image

Option E) larger diff
image

@plexus
Copy link
Member

plexus commented Sep 6, 2019

I'd be curious if puget allows rgb colors, so we can come up with a sensible scheme that isn't dependent on people's terminal configuration.

@lread
Copy link
Contributor Author

lread commented Sep 6, 2019

Good question, it looks like puget might support 8-bit colors via :fg-256 and :bg-256 https://github.com/greglook/puget/blob/d438931f35454cd2eb5afc10f458ae54dfcb4285/src/puget/color/ansi.clj#L28

I’ll try them out.

@lread
Copy link
Contributor Author

lread commented Sep 6, 2019

Hiya @plexus, this git issue is more interesting than I originally thought it would be!

I am thinking ANSI RGB colors might not be a great idea for one solid reason and one subjective one.

solid reason: circleci does not seem render them
I found a few scripts that test out terminal capabilities with regards to color.
Here's output from color-spaces.pl on macOS iTerm2:
image
And here's the output from the same script when viewed from circleci's web interface:
image

subjective reason: 16-color ANSI option is themable
Maybe it is not important for all people who use kaocha to see the very same colors. People choose color themes in their terminals to get consistent coloring across all 16-color ANSI terminal apps they use. Like you said red and green have specific meanings, but maybe I like my red to be rust and you like yours to be indigo. Consistent terminal colors with 16-ANSI-color Vim themes gives a good overview of the concept.

I think this also puts configuration of color to address any accessibility issues with vision in the right place.

options
I see our options as:

  1. do nothing, solve through How to skip printing values same in both structures? #13
  2. add an option to allow kaocha users to change the color palette to whatever puget compatible colors they like. This might include defining and selecting color themes.
  3. add an option to allow kaocha users to invert the color for differences only.

All I really need to address my issue is to have an option to invert colors for differences. Circleci does seem to render background colors just fine:
image
So I am leaning toward option 3, but I am looking forward to learning what you think.

@magnars
Copy link

magnars commented Sep 10, 2019

I'll just add a little point here: Allowing configurability to this, will let color blind people (red/green for me) to use colors that are more noticably different for them.

@lread
Copy link
Contributor Author

lread commented Sep 10, 2019

Thanks @magnars, your input is much appreciated.

I have been focusing on the terminal and its typical configurability with regards to color, (or lack thereof when presented by circleci), but there is also REPL output when used from an editor and maybe other scenarios I have not thought of.

@plexus
Copy link
Member

plexus commented Sep 11, 2019

I would still be a fan of 2., making it configurable. I also prefer rbg colors over the 16 terminal colors because they are absolute. We can set a foreground and background and be sure it renders the same everywhere. There is such a wide range of color schemes out there that whatever we do will be unreadable for some.

If it's configurable at the deep-diff level then kaocha can always detect that CI=true and switch to something more basic.

@magnars that is a really great point. It's very unfortunate that the two colors that are so commonly used culturally to denote some kind of contrast (red/green) are also the most common colors that colorblind people have trouble with. what other colors would you use to convey added/deleted that aren't so problematic?

@magnars
Copy link

magnars commented Sep 11, 2019 via email

@lread
Copy link
Contributor Author

lread commented Sep 11, 2019

Thanks for the reply @plexus!

I still think there is strong merit to a 16 color palette being themeable.

That said, option 2 does not prohibit using the 16 color palette for those that see the benefit. I’ll proceed with testing out support for RGB colors with puget.

@lread
Copy link
Contributor Author

lread commented Sep 23, 2019

I was poking around a bit with puget. I confirm that it does support 8-bit and 24-bit ANSI schemes via:fg-256 and :bg-256.

8-bit scheme is expressed via [:fg-256 5 n] where n is between 0 and 255. We can combine foreground and background, for example, like so: [:fg-256 5 226 :bg-256 5 56].

24-bit scheme is expressed via [:fg-256 2 r g b] where r g b are each between 0 and 255. Foreground and background can be combined, for example: [:fg-256 2 205 236 255 :bg-256 2 110 22 188].

These all show nicely in iTerm2 on macOS. They did not, however, show properly in a cider repl session in spacemacs. Caveats on 8-bit and 24-bit scheme support would be included kaocha docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
Status: Candidate
Development

No branches or pull requests

3 participants