content: Implement Pygments Monokai, for upcoming dark theme #756

chrisbobbe · 2024-06-20T19:41:51Z

For dark theme, web currently uses a mix of Pygments Monokai and Pygments Default. Mixing in Pygments Default seems accidental, so here we just use pure Pygments Monokai. Discussion.

I've included screenshots, below, of just Pygments Monokai, and of Pygments Monokai with web's adjustments. (Except for the adjustment of putting a border around err text; that's hard in Flutter.)

Compare with Pygments's own demonstration of Pygments Monokai here: https://pygments.org/styles/

(To make these screenshots, I plugged in dark-theme colors for the code-block background and message background. So just focus on the code blocks and disregard the jarring appearance of light-theme-styled elements elsewhere. When we implement dark theme #95, we'll do it coherently.)

This PR (pure Pygments Monokai)	Following web (having some Pygments Default mixed in)

Fixes: #749

chrisbobbe · 2024-06-20T19:56:23Z

Oh, and I owe another pair of screenshots; ~~coming soon~~ here they are. This PR includes a commit to change light mode's plain code-block text color (used for un-highlighted code blocks) from hsl(0deg 0% 15%) to true black, following web.

Before	After

gnprice

Thanks! These changes all LGTM. One commit-message comment below.

Can you also say a few words about how you produced the list of styles? The link in the comment is to upstream, but then the comments on individual styles are in terms of CSS, which that upstream file isn't.

Oh, and I owe another pair of screenshots; ~~coming soon~~ here they are. This PR includes a commit to change light mode's plain code-block text color (used for un-highlighted code blocks) from hsl(0deg 0% 15%) to true black, following web.

Does this have a visible effect in the screenshots? I flipped back and forth, and couldn't spot any difference at all except in the clock at the top of the screen.

gnprice · 2024-06-21T01:15:29Z

lib/widgets/code_block.dart

@@ -31,18 +32,34 @@ class CodeBlockTextStyles {
      // .err { border: 1px solid hsl(0deg 100% 50%); }
      err: const TextStyle(backgroundColor: Color(0xffe2706e)),

+      esc: null,


While we're at it, write the TODO(#754) to update the light-theme styles. (Hopefully to something that also has a style for all of `CodeBlockSpanType`'s values.)

This change is fine, but there's nothing wrong with some of the values being null — that just means those spans inherit the base styling from plain, right?

I believe the empty-string values in the upstream file:
https://github.com/pygments/pygments/blob/f64833d9d/pygments/styles/monokai.py
mean inheriting the style, so e.g. the classes w and x, and g and gr, all inherit the base style. (While e.g. cm inherits that of c.)

plain copies a variable in web, --color-markdown-code-text (starting at this PR's commit

content: Follow web in code-block default text color

). I don't think that variable or its light/dark values has anything to do with Pygments styles; I think it's mostly meant for plain un-syntax-highlighted code blocks.

From a Pygments doc on writing your own custom style, I think the "base style" that w, x, g, and gr are meant to inherit from is actually the Token style, which for Monokai is #f8f8f2. Here's from that doc:

Token is the default style inherited by all token types.

This probably explains why web gives color: #f8f8f2; for w, x, g, and gr, instead of letting them fall back to --color-markdown-code-text. In zulip/zulip@f020f9eee, the switch to Monokai, it says the CSS was generated from a CLI tool from Pygments.

My process in making this commit was to follow web (minus some overrides; discussion) and then check the result against that upstream monokai.py file. So that's why our w, x, g, and gr are 0xfff8f8f2 instead of white with 85% opacity, and I think that's correct.

So I guess having gone through all this just now, I'm now questioning some of our lines in the switch (type):

CodeBlockSpanType.text => null, // A span with type of text is always unstyled.

CodeBlockSpanType.unknown => null,

Maybe we should store the Token style in a variable and return that instead of null.

Yeah, I agree with that conclusion.

Not a blocker for merging this, though — that'd be a fine followup. I don't think this PR causes any regressions on that front.

Actually it's a bit more complicated. In the Pygments HTML, text that is meant to get the Text style seems to be represented without <span>s—which is the same way text appears in plain, un-Pygmented code blocks. For example, this:

diff --git lib/widgets/content.dart lib/widgets/content.dart index 1715d41e7..43c747232 100644 --- lib/widgets/content.dart +++ lib/widgets/content.dart @@ -37,6 +37,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> { colorCodeBlockBackground: const HSLColor.fromAHSL(0.04, 0, 0, 0).toColor(), colorDirectMentionBackground: const HSLColor.fromAHSL(0.2, 240, 0.7, 0.7).toColor(), colorGlobalTimeBackground: const HSLColor.fromAHSL(1, 0, 0, 0.93).toColor(), + colorGlobalTimeBorder: const HSLColor.fromAHSL(1, 0, 0, 0.8).toColor(), colorMathBlockBorder: const HSLColor.fromAHSL(0.15, 240, 0.8, 0.5).toColor(), colorMessageMediaContainerBackground: const Color.fromRGBO(0, 0, 0, 0.03), colorThematicBreak: const HSLColor.fromAHSL(1, 0, 0, .87).toColor(), @@ -61,6 +62,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> { colorCodeBlockBackground: const HSLColor.fromAHSL(0.04, 0, 0, 1).toColor(), colorDirectMentionBackground: const HSLColor.fromAHSL(0.25, 240, 0.52, 0.6).toColor(), colorGlobalTimeBackground: const HSLColor.fromAHSL(0.2, 0, 0, 0).toColor(), + colorGlobalTimeBorder: const HSLColor.fromAHSL(0.4, 0, 0, 0).toColor(), colorMathBlockBorder: const HSLColor.fromAHSL(1, 240, 0.4, 0.4).toColor(), colorMessageMediaContainerBackground: const HSLColor.fromAHSL(0.03, 0, 0, 1).toColor(), colorThematicBreak: const HSLColor.fromAHSL(1, 0, 0, .87).toColor().withOpacity(0.2),

The diff-untouched lines are just text-node children of the <code>, not wrapped in spans. (They start with a .w span with a single space, not sure why.)

<p><span class="user-mention" data-user-id="13313">@Chris Bobbe</span></p> <p><span class="user-mention" data-user-id="17327">@Chris Bobbe (Test Account)</span></p><p><span class="user-mention" data-user-id="17327">@Chris Bobbe (Test Account)</span></p><div class="spoiler-block"><div class="spoiler-header"> <p>regular <strong>bold</strong></p> </div><div class="spoiler-content" aria-hidden="true"> <p>content</p> </div></div><h1><del><strong>bold</strong></del></h1><div class="spoiler-block"><div class="spoiler-header"> <p><em>italic <strong>bold</strong></em></p> </div><div class="spoiler-content" aria-hidden="true"> <p>content</p> </div></div><h1><span class="user-mention silent" data-user-id="13313">Chris Bobbe</span></h1><div class="spoiler-block"><div class="spoiler-header"> <h1>h1: regular <strong>bold</strong></h1> </div><div class="spoiler-content" aria-hidden="true"> <p>content</p> </div></div><div class="spoiler-block"><div class="spoiler-header"> <h1>h1</h1> </div><div class="spoiler-content" aria-hidden="true"> <p>content</p> </div></div><div class="codehilite" data-code-language="Dart"><pre><span></span><code><span class="kd">class</span><span class="w"> </span><span class="nc">ContentExample</span><span class="w"> </span><span class="p">{</span> </code></pre></div><div class="codehilite" data-code-language="Diff"><pre><span></span><code><span class="gh">diff --git lib/widgets/content.dart lib/widgets/content.dart</span> <span class="gh">index 1715d41e7..43c747232 100644</span> <span class="gd">--- lib/widgets/content.dart</span> <span class="gi">+++ lib/widgets/content.dart</span> <span class="gu">@@ -37,6 +37,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {</span> <span class="w"> </span> colorCodeBlockBackground: const HSLColor.fromAHSL(0.04, 0, 0, 0).toColor(), <span class="w"> </span> colorDirectMentionBackground: const HSLColor.fromAHSL(0.2, 240, 0.7, 0.7).toColor(), <span class="w"> </span> colorGlobalTimeBackground: const HSLColor.fromAHSL(1, 0, 0, 0.93).toColor(), <span class="gi">+ colorGlobalTimeBorder: const HSLColor.fromAHSL(1, 0, 0, 0.8).toColor(),</span> <span class="w"> </span> colorMathBlockBorder: const HSLColor.fromAHSL(0.15, 240, 0.8, 0.5).toColor(), <span class="w"> </span> colorMessageMediaContainerBackground: const Color.fromRGBO(0, 0, 0, 0.03), <span class="w"> </span> colorThematicBreak: const HSLColor.fromAHSL(1, 0, 0, .87).toColor(), <span class="gu">@@ -61,6 +62,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {</span> <span class="w"> </span> colorCodeBlockBackground: const HSLColor.fromAHSL(0.04, 0, 0, 1).toColor(), <span class="w"> </span> colorDirectMentionBackground: const HSLColor.fromAHSL(0.25, 240, 0.52, 0.6).toColor(), <span class="w"> </span> colorGlobalTimeBackground: const HSLColor.fromAHSL(0.2, 0, 0, 0).toColor(), <span class="gi">+ colorGlobalTimeBorder: const HSLColor.fromAHSL(0.4, 0, 0, 0).toColor(),</span> <span class="w"> </span> colorMathBlockBorder: const HSLColor.fromAHSL(1, 240, 0.4, 0.4).toColor(), <span class="w"> </span> colorMessageMediaContainerBackground: const HSLColor.fromAHSL(0.03, 0, 0, 1).toColor(), <span class="w"> </span> colorThematicBreak: const HSLColor.fromAHSL(1, 0, 0, .87).toColor().withOpacity(0.2), </code></pre></div>

That's the same way "foo" appears in an unstyled code block:

foo

<div class="codehilite"><pre><span></span><code>foo </code></pre></div>

So when we see text like that, we'll want to apply Text / Token style if it's a syntax-highlighted block (and I guess ditto for unknown tokens), and Zulip's own --color-markdown-code-text otherwise.

Web doesn't seem to handle this wrinkle. With a color meter, I can see the diff-untouched lines in the highlighted block are the same color as "foo" in the unhighlighted block. Link to those messages: https://chat.zulip.org/#narrow/stream/7-test-here/topic/content/near/1841644

I think we can check if a code block is syntax-highlighted by looking for a data-code-language in the codehilite.zulip-code-block div.

Actually, you know what, it's probably --color-markdown-code-text that we should ignore, and just use the Pygments Text / Token color wherever we were using that. That'll be way simpler in code.

chrisbobbe · 2024-06-21T05:12:02Z

Oh, and I owe another pair of screenshots; ~~coming soon~~ here they are. This PR includes a commit to change light mode's plain code-block text color (used for un-highlighted code blocks) from hsl(0deg 0% 15%) to true black, following web.

Does this have a visible effect in the screenshots? I flipped back and forth, and couldn't spot any difference at all except in the clock at the top of the screen.

It's very subtle. The plain, unstyled code block text is now true black, instead of hsl(0deg 0% 15%) i.e. ContentTheme.textStylePlainParagraph.color. Do you have something like Mac's "Digital Color Meter" to see the difference? That's how I checked that there definitely was a change, but I'm also able to see a small difference visually on my monitor.

chrisbobbe · 2024-06-21T05:17:33Z

Can you also say a few words about how you produced the list of styles?

I looked at web's %dark-theme block in web/styles/pygments.css, bulk-formatted it to make nice single-line comments like // .ss { color: #e6db74; }, copy-pasted those into this file, and translated the commented CSS into Flutter TextStyles.

(Then I started applying the "non-color Zulip customizations" from pygments.css before realizing we didn't want those, as mentioned, and I removed what I had added.)

gnprice · 2024-06-26T02:01:51Z

It's very subtle. The plain, unstyled code block text is now true black, instead of hsl(0deg 0% 15%) i.e. ContentTheme.textStylePlainParagraph.color.

Oh wow, OK — yeah, if I bring my face right up a few inches from the screen and then flip back and forth, I do see the difference.

gnprice · 2024-06-26T02:06:31Z

I looked at web's %dark-theme block in web/styles/pygments.css, bulk-formatted it to make nice single-line comments like // .ss { color: #e6db74; }, copy-pasted those into this file, and translated the commented CSS into Flutter TextStyles.

Cool, makes sense. I think a few words saying these came from web/styles/pygments.css like that would be helpful.

Maybe just have this comment:

  // Pygments Monokai:
  //   https://github.com/pygments/pygments/blob/f64833d9d/pygments/styles/monokai.py
  factory CodeBlockTextStyles.dark(BuildContext context) {

mention that, in addition to the upstream Pygments link (that the current revision has) for whence the colors were ultimately derived.

chrisbobbe · 2024-06-26T07:25:17Z

Thanks for the review! Revision pushed, and I've made a note about the followup you mentioned: #756 (comment)

We're about to add a "dark", for zulip#749 / zulip#95 dark theme.

Web uses black instead of hsl(0deg 0% 15%), the default message text color.

The dark-theme styles, which we'll add soon, have something for all of `CodeBlockSpanType`'s values. While we're at it, write the TODO(zulip#754) to update the light-theme styles. (Hopefully to something that also has a style for all of `CodeBlockSpanType`'s values.)

We might reasonably have declared them alphabetized by the first element in the "packages" list ('Noto Color Emoji', 'Source Code Pro', etc.; the first argument passed to LicenseEntryWithLineBreaks). But I didn't think that would be best in a future where we add something with more than one item in that "packages" list.

For dark theme, web currently uses a mix of Pygments Monokai and Pygments Default. Mixing in Pygments Default seems accidental, so here we just use pure Pygments Monokai. Discussion: https://chat.zulip.org/#narrow/stream/431-redesign-project/topic/code.20span.20colors/near/1832283 LICENSE and AUTHORS files downloaded from: https://github.com/pygments/pygments/blob/f64833d9d/LICENSE https://github.com/pygments/pygments/blob/f64833d9d/AUTHORS (I've included AUTHORS because LICENSE refers to it.) Fixes: zulip#749 Related: zulip#95

chrisbobbe · 2024-06-26T07:44:39Z

Just resolved the conflicts from #759

gnprice · 2024-06-26T21:23:33Z

Thanks! Looks good; merging.

chrisbobbe added a-content Parsing and rendering Zulip HTML content, notably message contents a-design Visual and UX design labels Jun 20, 2024

chrisbobbe force-pushed the pr-dark-theme-pygments-monokai branch from 3dd63af to f289473 Compare June 20, 2024 19:43

gnprice reviewed Jun 21, 2024

View reviewed changes

gnprice mentioned this pull request Jun 26, 2024

Make StreamColorSwatches.lerp and CodeBlockTextStyles.lerp static #759

Merged

chrisbobbe force-pushed the pr-dark-theme-pygments-monokai branch from f289473 to af6add9 Compare June 26, 2024 07:22

chrisbobbe added 5 commits June 26, 2024 00:29

content [nfc]: Name CodeBlockTextStyles's factory constructor "light"

d955039

We're about to add a "dark", for zulip#749 / zulip#95 dark theme.

content: Follow web in code-block default text color

299c76b

Web uses black instead of hsl(0deg 0% 15%), the default message text color.

chrisbobbe force-pushed the pr-dark-theme-pygments-monokai branch from af6add9 to 3aaebdb Compare June 26, 2024 07:43

gnprice merged commit 3aaebdb into zulip:main Jun 26, 2024
1 check passed

chrisbobbe deleted the pr-dark-theme-pygments-monokai branch June 26, 2024 22:00

chrisbobbe mentioned this pull request Jun 26, 2024

content [nfc]: Finish writing down dark-theme color variants for all content #767

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

content: Implement Pygments Monokai, for upcoming dark theme #756

content: Implement Pygments Monokai, for upcoming dark theme #756

chrisbobbe commented Jun 20, 2024 •

edited

Loading

chrisbobbe commented Jun 20, 2024 •

edited

Loading

gnprice left a comment

gnprice Jun 21, 2024

chrisbobbe Jun 21, 2024 •

edited

Loading

chrisbobbe Jun 21, 2024 •

edited

Loading

gnprice Jun 26, 2024

gnprice Jun 26, 2024

chrisbobbe Jun 28, 2024

chrisbobbe Jun 28, 2024

chrisbobbe Jun 29, 2024

chrisbobbe commented Jun 21, 2024

chrisbobbe commented Jun 21, 2024 •

edited

Loading

gnprice commented Jun 26, 2024

gnprice commented Jun 26, 2024

chrisbobbe commented Jun 26, 2024

chrisbobbe commented Jun 26, 2024

gnprice commented Jun 26, 2024

content: Implement Pygments Monokai, for upcoming dark theme #756

content: Implement Pygments Monokai, for upcoming dark theme #756

Conversation

chrisbobbe commented Jun 20, 2024 • edited Loading

chrisbobbe commented Jun 20, 2024 • edited Loading

gnprice left a comment

Choose a reason for hiding this comment

gnprice Jun 21, 2024

Choose a reason for hiding this comment

chrisbobbe Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

chrisbobbe Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

gnprice Jun 26, 2024

Choose a reason for hiding this comment

gnprice Jun 26, 2024

Choose a reason for hiding this comment

chrisbobbe Jun 28, 2024

Choose a reason for hiding this comment

chrisbobbe Jun 28, 2024

Choose a reason for hiding this comment

chrisbobbe Jun 29, 2024

Choose a reason for hiding this comment

chrisbobbe commented Jun 21, 2024

chrisbobbe commented Jun 21, 2024 • edited Loading

gnprice commented Jun 26, 2024

gnprice commented Jun 26, 2024

chrisbobbe commented Jun 26, 2024

chrisbobbe commented Jun 26, 2024

gnprice commented Jun 26, 2024

chrisbobbe commented Jun 20, 2024 •

edited

Loading

chrisbobbe commented Jun 20, 2024 •

edited

Loading

chrisbobbe Jun 21, 2024 •

edited

Loading

chrisbobbe Jun 21, 2024 •

edited

Loading

chrisbobbe commented Jun 21, 2024 •

edited

Loading