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

Implement 16 named ANSI colors #259

Merged
merged 4 commits into from
Apr 7, 2016
Merged

Conversation

mgeier
Copy link
Contributor

@mgeier mgeier commented Feb 28, 2016

... instead of 8. See #245.

For this to work, the notebook CSS has to be updated:

.ansi-black-fg { color: #3E424D; }
.ansi-black-bg { background-color: #3E424D; }
.ansi-black-intense-fg { color: #282C36; }
.ansi-black-intense-bg { background-color: #282C36; }
.ansi-red-fg { color: #E75C58; }
.ansi-red-bg { background-color: #E75C58; }
.ansi-red-intense-fg { color: #B22B31; }
.ansi-red-intense-bg { background-color: #B22B31; }
.ansi-green-fg { color: #00A250; }
.ansi-green-bg { background-color: #00A250; }
.ansi-green-intense-fg { color: #007427; }
.ansi-green-intense-bg { background-color: #007427; }
.ansi-yellow-fg { color: #DDB62B; }
.ansi-yellow-bg { background-color: #DDB62B; }
.ansi-yellow-intense-fg { color: #B27D12; }
.ansi-yellow-intense-bg { background-color: #B27D12; }
.ansi-blue-fg { color: #208FFB; }
.ansi-blue-bg { background-color: #208FFB; }
.ansi-blue-intense-fg { color: #0065CA; }
.ansi-blue-intense-bg { background-color: #0065CA; }
.ansi-magenta-fg { color: #D160C4; }
.ansi-magenta-bg { background-color: #D160C4; }
.ansi-magenta-intense-fg { color: #A03196; }
.ansi-magenta-intense-bg { background-color: #A03196; }
.ansi-cyan-fg { color: #60C6C8; }
.ansi-cyan-bg { background-color: #60C6C8; }
.ansi-cyan-intense-fg { color: #258F8F; }
.ansi-cyan-intense-bg { background-color: #258F8F; }
.ansi-white-fg { color: #C5C1B4; }
.ansi-white-bg { background-color: #C5C1B4; }
.ansi-white-intense-fg { color: #A1A6B2; }
.ansi-white-intense-bg { background-color: #A1A6B2; }

.ansi-bold { font-weight: bold; }

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

I haven't changed the tests yet, I'd like to get some feedback before doing that.

Any comments?

@takluyver
Copy link
Member

In that case, it will also need a new release of the notebook, and nbconvert will have to pull in the CSS from that new version. So I guess the notebook is the thing to focus on first.

I have no objection to differentiating the bright versions of colours.

@Carreau
Copy link
Member

Carreau commented Mar 1, 2016

I have no objection to differentiating the bright versions of colours.

Neither do I.

@minrk
Copy link
Member

minrk commented Mar 8, 2016

To avoid relying on a notebook release, we can add an additional CSS file to this repo, for any classes needed here that are not (or not yet) defined or available there.

@mgeier
Copy link
Contributor Author

mgeier commented Mar 10, 2016

Thanks for the feedback!

I'll try to hack this into the notebook in the next few days/weeks, and if successful, I'll come back here afterwards.

@mgeier
Copy link
Contributor Author

mgeier commented Mar 18, 2016

I made a PR for the Notebook: jupyter/notebook#1230

@mgeier
Copy link
Contributor Author

mgeier commented Mar 26, 2016

OK, jupyter/notebook#1230 has been merged.

I've rebased this PR and added tests (b28f2bf). I've also added support for the escape codes 90-97 and 100-107 (7b799e0).

I've not added the CSS definitions. Is it foreseeable when the next Notebook CSS release will be?

@mgeier
Copy link
Contributor Author

mgeier commented Mar 26, 2016

Another thing: I renamed a few colors in the LaTeX template to make clear that those are ANSI colors, but I don't know if the old color names were used anywhere else in a different context.

  • unused in nbconvert (but probably used elsewhere?):

    orange, myteal, gray, lightgray (was defined twice before this PR!), mediumgray, inputbackground, outputbackground, traceback

  • used in nbconvert:

    darkorange (used for linkcolor), darkgreen (used for citecolor)

  • colors which I deleted in this PR:

    red, green, brown, blue (this breaks urlcolor!), purple, cyan, (one occurence of) lightgray, darkgray, lightred, lightgreen, lightblue, lightpurple

How should I handle this?

One suggestion that comes to my mind:

  1. delete all color definitions mentioned above
  2. define only the actually used colors with "semantic" names, i.e. urlcolor, linkcolor, citecolor instead of blue, darkorange, darkgreen, respectively

@minrk
Copy link
Member

minrk commented Mar 27, 2016

I've not added the CSS definitions. Is it foreseeable when the next Notebook CSS release will be?

notebook-5.0 will not be very soon, and we should be releasing nbconvert very soon (it's frankly overdue for a release by months). So if this should get into a minor release, it has to include a copy of the CSS definitions.

@mgeier
Copy link
Contributor Author

mgeier commented Mar 28, 2016

OK, I added the temporary CSS definitions: 692594c

I'm still waiting for instructions regarding #259 (comment).

@minrk
Copy link
Member

minrk commented Apr 5, 2016

@mgeier sorry for not replying. Semantic names sounds good to me, if you want to add that.

@mgeier
Copy link
Contributor Author

mgeier commented Apr 7, 2016

I've added the discussed color definitions and removed the unused colors.
I also rebased this PR for your merging convenience!

@minrk minrk added this to the 4.2 milestone Apr 7, 2016
@minrk minrk merged commit f59ef7c into jupyter:master Apr 7, 2016
@minrk
Copy link
Member

minrk commented Apr 7, 2016

Great, thanks!

@mgeier mgeier deleted the 16-ansi-colors branch April 8, 2016 08:14
@mgeier
Copy link
Contributor Author

mgeier commented Apr 8, 2016

Thanks for merging! I'm looking forward to the new release!

@mgeier mgeier mentioned this pull request Apr 17, 2016
@KristofferC
Copy link

Sorry if this is the wrong place to ask, but is this supposed to work in notebooks as well?

image

@takluyver
Copy link
Member

Yes (jupyter/notebook#1230), but it's not yet in a released version.

@KristofferC
Copy link

Thank you for the reference!

@mgeier
Copy link
Contributor Author

mgeier commented Sep 22, 2016

@KristofferC Just a little side note: What you call "Light Red" in your example, will actually become very close to the old "Red". The new "Red" will be brighter, which looks fainter and weaker on a white background. That's the reason why I called the colors 9-16 "intense" instead of "light" or "bright".

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.

5 participants