Fix broken highlight styles

[wxiaoguang]

Close #22348

This PR uses Chroma's style=github

[silverwind]

Can you make the background on dark the same color as the line numbers, like it is on light, probably by just not setting it? In that screenshot, it looks very out of place to the rest of the site.

[silverwind]

I think syntax theme should not set .chroma background at all, that is the job of the gitea theme, not the syntax theme.

[wxiaoguang]

I think syntax theme should not set .chroma background at all, that is the job of the gitea theme, not the syntax theme.

a4db933

[silverwind]

Also I would object to having font-weight and font-style being set at all in the theme. I perceive this as ugly, and Github also no longer does this so I would suggest dropping these declarations.

[wxiaoguang]

Also I would object to having font-weight and font-style being set at all in the theme. I perceive this as ugly, and Github also no longer does this so I would suggest dropping these declarations.

This PR is only for fixing the broken chroma styles. Feel free to propose new PRs or replace this one.

[wxiaoguang]

Or please help to propose a complete document about how to follow/import Chroma's style correctly (eg: https://github.com/go-gitea/gitea/pull/23148/files#diff-9a774436362a015fd2af28b3169b5ecc05f8f0f74d150abc8bf9a40fe2589f79R1-R10). Then future developers could know what to do.

[silverwind]

You are completely replacing the dark theme with another one and are adding these unwanted font styles from the chroma bundled themes. I do agree we can probably switch the dark color scheme to something more closely matching GitHub, thought.

These Chroma-bundled themes are not suitable for us without more adjustments, like these font rules. I would also remove this comment on the generation, it's far from a simply copy-paste and needs more refinement.

[wxiaoguang]

7dcee32 could do the work, by overwriting .chroma * { font styles }

[wxiaoguang]

These Chroma-bundled themes are not suitable for us without more adjustments, like these font rules. I would also remove this comment on the generation, it's far from a simply copy-paste and needs more refinement.

But #22348 has been open for a long time, maybe everyone is waiting for you to fine tune the theme since the last modification was also contributed by you. I guess nobody else knows how to get these styles.

[wxiaoguang]

7dcee32 could do the work, by overwriting .chroma * { font styles }

Actually, I think the bold is just fine and done by purpose, some keywords are just "black + bold". Forcing font-weight: normal also makes the UI looks strange.

I would suggest to:

1. Follow a major Chroma style, do not change too much
2. Write a clear document about how to maintain these styles

[codecov-commenter]

Codecov Report

Merging #23148 (7dcee32) into main (33e556e) will decrease coverage by 0.02%.
The diff coverage is 45.16%.

❗ Current head 7dcee32 differs from pull request most recent head 4d32020. Consider uploading reports for the commit 4d32020 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main   #23148      +/-   ##
==========================================
- Coverage   47.44%   47.42%   -0.02%     
==========================================
  Files        1140     1140              
  Lines      150804   150990     +186     
==========================================
+ Hits        71549    71610      +61     
- Misses      70788    70904     +116     
- Partials     8467     8476       +9     

Impacted Files	Coverage Δ
models/actions/run.go	1.73% <0.00%> (+0.02%)	⬆️
models/actions/run_list.go	0.00% <0.00%> (ø)
models/actions/status.go	22.85% <0.00%> (-1.39%)	⬇️
models/auth/oauth2.go	60.52% <ø> (ø)
models/auth/twofactor.go	19.73% <ø> (ø)
models/db/engine.go	44.80% <ø> (ø)
models/issues/label.go	55.95% <0.00%> (ø)
models/unittest/testdb.go	12.92% <0.00%> (ø)
modules/auth/password/hash/pbkdf2.go	69.69% <ø> (ø)
modules/avatar/hash.go	100.00% <ø> (ø)
... and 51 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[wxiaoguang]

@silverwind

Could you take a look at https://swapoff.org/chroma/playground/ and choose 2 best styles (light/dark) and just use them? I do not think it's worth to fine tune everything on our side. These styles are contributed by Chroma users and have been tested and used heavily, it could be better than our home-made styles. And using these style will reduce maintaining work, there will be no bug in the future even if Chroma changes its output again.

[silverwind]

I do not like the chroma styles for various reasons, one being the bold/italic. I think we can generate our own dark/light styles based on GitHub's colors available via https://github.com/primer/primitives. All that's left to do is convert the prettylights primitives to chroma.

dark:

    --color-prettylights-syntax-comment: #8b949e;
    --color-prettylights-syntax-constant: #79c0ff;
    --color-prettylights-syntax-entity: #d2a8ff;
    --color-prettylights-syntax-storage-modifier-import: #c9d1d9;
    --color-prettylights-syntax-entity-tag: #7ee787;
    --color-prettylights-syntax-keyword: #ff7b72;
    --color-prettylights-syntax-string: #a5d6ff;
    --color-prettylights-syntax-variable: #ffa657;
    --color-prettylights-syntax-brackethighlighter-unmatched: #f85149;
    --color-prettylights-syntax-invalid-illegal-text: #f0f6fc;
    --color-prettylights-syntax-invalid-illegal-bg: #8e1519;
    --color-prettylights-syntax-carriage-return-text: #f0f6fc;
    --color-prettylights-syntax-carriage-return-bg: #b62324;
    --color-prettylights-syntax-string-regexp: #7ee787;
    --color-prettylights-syntax-markup-list: #f2cc60;
    --color-prettylights-syntax-markup-heading: #1f6feb;
    --color-prettylights-syntax-markup-italic: #c9d1d9;
    --color-prettylights-syntax-markup-bold: #c9d1d9;
    --color-prettylights-syntax-markup-deleted-text: #ffdcd7;
    --color-prettylights-syntax-markup-deleted-bg: #67060c;
    --color-prettylights-syntax-markup-inserted-text: #aff5b4;
    --color-prettylights-syntax-markup-inserted-bg: #033a16;
    --color-prettylights-syntax-markup-changed-text: #ffdfb6;
    --color-prettylights-syntax-markup-changed-bg: #5a1e02;
    --color-prettylights-syntax-markup-ignored-text: #c9d1d9;
    --color-prettylights-syntax-markup-ignored-bg: #1158c7;
    --color-prettylights-syntax-meta-diff-range: #d2a8ff;
    --color-prettylights-syntax-brackethighlighter-angle: #8b949e;
    --color-prettylights-syntax-sublimelinter-gutter-mark: #484f58;
    --color-prettylights-syntax-constant-other-reference-link: #a5d6ff;

light:

    --color-prettylights-syntax-comment: #6e7781;
    --color-prettylights-syntax-constant: #0550ae;
    --color-prettylights-syntax-entity: #8250df;
    --color-prettylights-syntax-storage-modifier-import: #24292f;
    --color-prettylights-syntax-entity-tag: #116329;
    --color-prettylights-syntax-keyword: #cf222e;
    --color-prettylights-syntax-string: #0a3069;
    --color-prettylights-syntax-variable: #953800;
    --color-prettylights-syntax-brackethighlighter-unmatched: #82071e;
    --color-prettylights-syntax-invalid-illegal-text: #f6f8fa;
    --color-prettylights-syntax-invalid-illegal-bg: #82071e;
    --color-prettylights-syntax-carriage-return-text: #f6f8fa;
    --color-prettylights-syntax-carriage-return-bg: #cf222e;
    --color-prettylights-syntax-string-regexp: #116329;
    --color-prettylights-syntax-markup-list: #3b2300;
    --color-prettylights-syntax-markup-heading: #0550ae;
    --color-prettylights-syntax-markup-italic: #24292f;
    --color-prettylights-syntax-markup-bold: #24292f;
    --color-prettylights-syntax-markup-deleted-text: #82071e;
    --color-prettylights-syntax-markup-deleted-bg: #ffebe9;
    --color-prettylights-syntax-markup-inserted-text: #116329;
    --color-prettylights-syntax-markup-inserted-bg: #dafbe1;
    --color-prettylights-syntax-markup-changed-text: #953800;
    --color-prettylights-syntax-markup-changed-bg: #ffd8b5;
    --color-prettylights-syntax-markup-ignored-text: #eaeef2;
    --color-prettylights-syntax-markup-ignored-bg: #0550ae;
    --color-prettylights-syntax-meta-diff-range: #8250df;
    --color-prettylights-syntax-brackethighlighter-angle: #57606a;
    --color-prettylights-syntax-sublimelinter-gutter-mark: #8c959f;
    --color-prettylights-syntax-constant-other-reference-link: #0a3069;

I think we can also take the opporturnity to expose first-party variables, e.g. --color-code-comment to themes, which could then possibly map to chroma, codemirror and monaco colors.

[wxiaoguang]

I do not like the chroma styles for various reasons, one being the bold/italic.

There are many Chroma styles without bold/italic, and the bold/italic styles could be fixed manually by adding a few new lines, without changing the original output too much.

All that's left to do is convert the prettylights primitives to chroma.

It's not a 1v1 mapping to Chroma. If Chroma changes its output in the future (like this time), people have to learn to do this again. I don't like to maintain the color mapping between Chroma and an external color system. But if most people like that mapping and have implemented it and have a document, then I would have no objection.

[mrsdizzie]

I only worked on the light theme aspect and can't speak to anything related to the dark theme or the linked issue --- which seems like a real issue to be fixed.

For what it is worth these Chroma styles were considered and intentionally not used as-is when adding Chroma syntax highlighting because I (and others) found them to be both "not that nice looking" and to not reflect what Github looked like at the time. It doesn't look like they have changed at all since then. I know looks are an opinion, but just as a background that this option was considered and rejected previously because it didn't seem good enough for Gitea users.

The light Chroma github style was used as an initial baseline, some parts were updated base on https://github.com/primer/github-syntax-theme-generator and other things were probably added manually to match what Github was actually doing based on looking at lots of examples.

"Fix broken highlight styles" seems like a misleading PR title here as the linked issue is related to an aspect of the dark theme (?) but this changes the light theme as well and isn't just a fix to current styles. The title and description should be changed to reflect what is really happening including before and after images which I think would show that this update feels like a visual regression at least in the light theme.

[wxiaoguang]

@mrsdizzie I basically agree with you. My opinions are:

• I do not think we need to strictly follow GitHub, although it's also good to follow if it doesn't make maintainability more difficult.
• If there is a "more or less" nice theme from Chroma, we could just use it, and we just need to do some fine tunes.
• If we want to follow GitHub strictly and replace all colors, there should be a very clear document about what to do. Otherwise if the styles get broken again, nobody knows what to do next time.
• Why that bug is open for long time? Maybe nobody knows how to do.
• If I understand correctly, the "light theme" might be also broken somewhat. According to my last review, I can see that the styles doesn't match the Chroma generated ones. I can confirm that some of the missing styles are not used, while I can not understand whether some others are used or not. That's why I planned to re-generated and re-sync the styles from Chroma outputs.

[wxiaoguang]

The alternative PR for fully customized styles : Fix broken Chroma CSS styles #23174