Updated Chroma to v0.10.0

[MrGussio]

Updated Chroma to v0.10.0, and ran the commands go mod vendor and make build.

Fix #18269

[techknowlogick]

CI fail seems related.

[zeripath]

it appears that chroma has now changed its rendering significantly adding <span class=\"line\"><span class=\"cl\"> before/around lines.

[MrGussio]

it appears that chroma has now changed its rendering significantly adding <span class=\"line\"><span class=\"cl\"> before/around lines.

Yes, I noticed. Just did a commit which should fix these issues. Locally I did not have any test errors anymore, now the wait is for the CI to check them as well.

[zeripath]

I think we need to double check what if any effect this has on rendering.

I think the <span class="line"> will be being sanitized to <span> and there's no CSS for the ".cl" in dark or light.less.

Do you know what these spans are supposed to be doing?

[techknowlogick]

apologies for these conflicts.

[singuliere]

@MrGussio screenshots showing the rendering before and after the upgrade would go a long way to show it looks good. Thanks a lot for this upgrade!

[silverwind]

Need to rebase and remove the vendor files as we no longer vendor them since 84145e4.

[zeripath]

rebased and updated

[silverwind]

alecthomas/chroma@d6e61d3 appears to be related to the HTML changes.

I agree we need to be careful here that this does not break any assumptions about Chroma's HTML in our CSS. Possibly also need to update themes in case new classes are introduced.

[wxiaoguang]

And we could wait for a new release of Chroma now.

• Add mariadb aliases alecthomas/chroma#595 Many thanks to @silverwind 👍

[MrGussio]

@MrGussio screenshots showing the rendering before and after the upgrade would go a long way to show it looks good. Thanks a lot for this upgrade!

Build using the latest version of my fork:

Build of commit d7c2a29 (before the Chroma update):

It seems to have no real visual impact. Changes seem to be coming from alecthomas/chroma@d6e61d3 and alecthomas/chroma@5ed5054 as already mentioned above.

[zeripath]

It doesn't look like chroma is releasing an update soon so I think we should just approve - merge and see how things look IRL.

[silverwind]

I didn't notice any immediate regression from this but I guess the extra wrapping element is unneccary in our rendering too, but I guess it can be improved upon later.

[wxiaoguang]

A little off-topic, just to remind developers: Chroma uses a lot of abbr class names like nx, nt, cl, p.

We also wrote a lot of abbr class names like df, pr, ac, tc, sb, vm, etc. If we continue to use ambiguous abbr names globally, they would conflict and lead to strange bugs in one day.

We should stick to non-ambiguous names in future.

[silverwind]

Happy to accept a PR that refactors our helper classes to something longer.

[codecov-commenter]

Codecov Report

Merging #18270 (665f2be) into main (7427b81) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #18270      +/-   ##
==========================================
- Coverage   45.94%   45.93%   -0.02%     
==========================================
  Files         832      832              
  Lines       92092    92092              
==========================================
- Hits        42316    42306      -10     
- Misses      43022    43032      +10     
  Partials     6754     6754              

Impacted Files	Coverage Δ
modules/highlight/highlight.go	57.79% <0.00%> (-3.67%)	⬇️
modules/queue/queue_bytefifo.go	59.28% <0.00%> (-3.60%)	⬇️
modules/queue/workerpool.go	50.00% <0.00%> (-3.06%)	⬇️
modules/log/event.go	62.50% <0.00%> (+1.85%)	⬆️
modules/charset/charset.go	75.75% <0.00%> (+4.04%)	⬆️

---

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 7427b81...665f2be. Read the comment docs.