👌 IMPROVE: Update ANSI CSS colors

[saulshanabrook]

Currently, when displaying terminal output in Jupyter Book, the default colors are a bit hard to read.

For example, this is some CLI output using Rich's default color encoding:

The yellow in particular is very light.

This is the same page, after changing the yellow manually to the new color:

I copied all of the colors from Jupyter Notebook. You can see there it's much more legible:

I noticed that Jupyter Notebook itself changed their colors in 2017: jupyter/notebook#3286

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

[chrisjsewell]

cheers!

[codecov]

Codecov Report

Merging #384 (587343d) into master (d639806) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #384   +/-   ##
=======================================
  Coverage   80.44%   80.44%           
=======================================
  Files          21       21           
  Lines        2132     2132           
=======================================
  Hits         1715     1715           
  Misses        417      417           

Flag	Coverage Δ
pytests	80.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d639806...587343d. Read the comment docs.

[chrisjsewell]

note the failures in the tests are just because of ipython updating to v8. I'll make another PR to pin that to v7 for now

[saulshanabrook]

Thank you! I hope I copied all the colors properly, might be good to double check just to make sure...

[chrisjsewell]

Looks good to me: https://myst-nb--384.org.readthedocs.build/en/384/use/formatting_outputs.html#ansi-outputs

maybe if you also want to give a look

[saulshanabrook]

Thanks for looking, yeah I just doubled check I copied things correctly, all seems good.

[choldgraf]

A quick comparison from the preview:

Old:

New:

I think that overall it is certainly easier to read - though the "short demo" snippet that precedes the table is harder to read with the new ANSI colors. Though unless those colors were chosen because they need to be legible, maybe we should just pick different colors? The table is probably the most important reference to use

[saulshanabrook]

Thanks for running the comparison! I am open to any colors, these are at least readable on a lighter background, which is the main thing that was important to me for the example.

It seems like these were originally added in mgeier/notebook@7bee6c5 by @mgeier (jupyter/notebook#1230)

[chrisjsewell]

Actually, if we are going to do this "properly", then we should probably change them to CSS variables and implement dark mode switching 😉

[saulshanabrook]

It looks like the original choice was (jupyter/nbconvert#259):

I chose the colors as a mix of http://www.xcolors.net/dl/baskerville-ivorylight and http://www.xcolors.net/dl/euphrasia.

Actually, if we are going to do this "properly", then we should probably change them to CSS variables and implement dark mode switching 😉

Sounds good long term! But having a visible yellow would be great short term... (i.e. I don't feel up to doing it properly!)

[choldgraf]

Agree that CSS variables are a better long-term solution, this seems like a nice improvement to me though and the colors in general seem reasonable. It'd be nice if that first example used a different set of ANSI colors so it was more readable, but I don't think it needs to block the PR

[mgeier]

There seems to have been a bit of confusion with the "intense" color variants, because I guess those are not implemented yet?

For comparison, this is how it is supposed to look like: https://nbsphinx.readthedocs.io/en/latest/code-cells.html#ANSI-Colors

Actually, if we are going to do this "properly", then we should probably change them to CSS variables and implement dark mode switching 😉

The idea is that red is red, regardless of the background color.
I don't know whether that works in practice, but that's the idea.

However, there are two special CSS classes: ansi-default-inverse-fg and ansi-default-inverse-bg, which are supposed to be changed when changing the background color.

For example, this is where they are set in JupyterLab: https://github.com/jupyterlab/jupyterlab/blob/ea1529b6683adb692b1c9908f04d0a2cead44a9f/packages/rendermime/style/base.css#L164-L168

It's of course also possible to switch all the colors when switching from light to dark mode, but I was hoping to be able to avoid this.

[saulshanabrook]

@mgeier I used the second color in each definition as the background color, maybe that was incorrect? Some of them matched our existing background colors, so I thought it seemed OK.

[mgeier]

I used the second color in each definition as the background color, maybe that was incorrect?

Well, "incorrect" is a strong word. The second color is supposed to be the "intense" one.
Sometimes, that's called "bright", but I changed it to "intense", since "bright" makes only sense on a dark background.

You can of course use any colors you want, nothing is "incorrect", but if you want to match JupyterLab and the Classic Notebook (and nbsphinx for that matter), you should use the first color for "normal" and the second for "intense" (which isn't implemented yet AFAICT).

[saulshanabrook]

Thanks for the clarification. I updated the background colors to be those from the npshinx ANSI colors page you posted.

[chrisjsewell]

Hey @saulshanabrook, if you want to fix the merge conflicts, then we can revisit this

[saulshanabrook]

@chrisjsewell just merged!

[chrisjsewell]

cheers!

[welcome]

Congrats on your first merged pull request in this project! 🎉


Thank you for contributing, we are very proud of you! ❤️