Skip to content

Update chroma to v2.20.0 #35220

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sebastianertz
Copy link

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 6, 2025
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 6, 2025
@silverwind
Copy link
Member

Unit test failure is related:

 --- FAIL: TestRender_Source (0.01s)
    orgmode_test.go:97: 
        	Error Trace:	/home/runner/work/gitea/gitea/modules/markup/orgmode/orgmode_test.go:97
        	            				/home/runner/work/gitea/gitea/modules/markup/orgmode/orgmode_test.go:100
        	Error:      	Not equal: 
        	            	expected: "<div class=\"src src-go\">\n<pre><code class=\"chroma language-go\"><span class=\"c1\">// HelloWorld prints &#34;Hello World&#34;</span>\n<span class=\"kd\">func</span> <span class=\"nf\">HelloWorld</span><span class=\"p\">()</span> <span class=\"p\">{</span>\n\t<span class=\"nx\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Println</span><span class=\"p\">(</span><span class=\"s\">&#34;Hello World&#34;</span><span class=\"p\">)</span>\n<span class=\"p\">}</span></code></pre>\n</div>"
        	            	actual  : "<div class=\"src src-go\">\n<pre><code class=\"chroma language-go\"><span class=\"c1\">// HelloWorld prints &#34;Hello World&#34;</span><span class=\"w\">\n</span><span class=\"w\"></span><span class=\"kd\">func</span><span class=\"w\"> </span><span class=\"nf\">HelloWorld</span><span class=\"p\">()</span><span class=\"w\"> </span><span class=\"p\">{</span><span class=\"w\">\n</span><span class=\"w\">\t</span><span class=\"nx\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Println</span><span class=\"p\">(</span><span class=\"s\">&#34;Hello World&#34;</span><span class=\"p\">)</span><span class=\"w\">\n</span><span class=\"w\"></span><span class=\"p\">}</span></code></pre>\n</div>"

@sebastianertz
Copy link
Author

Does the unit test need to be adjusted?
I'm not familiar with the Go language. But I would be happy if Chroma received an update. Because of the new languages in Chroma.

@silverwind
Copy link
Member

Yes, apparently chrome tokenization changed in this version which causes the failure. We need to update the test and verify syntax highlighting works as expected.

@a1012112796
Copy link
Member

Unit test failure is related:

 --- FAIL: TestRender_Source (0.01s)
    orgmode_test.go:97: 
        	Error Trace:	/home/runner/work/gitea/gitea/modules/markup/orgmode/orgmode_test.go:97
        	            				/home/runner/work/gitea/gitea/modules/markup/orgmode/orgmode_test.go:100
        	Error:      	Not equal: 
        	            	expected: "<div class=\"src src-go\">\n<pre><code class=\"chroma language-go\"><span class=\"c1\">// HelloWorld prints &#34;Hello World&#34;</span>\n<span class=\"kd\">func</span> <span class=\"nf\">HelloWorld</span><span class=\"p\">()</span> <span class=\"p\">{</span>\n\t<span class=\"nx\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Println</span><span class=\"p\">(</span><span class=\"s\">&#34;Hello World&#34;</span><span class=\"p\">)</span>\n<span class=\"p\">}</span></code></pre>\n</div>"
        	            	actual  : "<div class=\"src src-go\">\n<pre><code class=\"chroma language-go\"><span class=\"c1\">// HelloWorld prints &#34;Hello World&#34;</span><span class=\"w\">\n</span><span class=\"w\"></span><span class=\"kd\">func</span><span class=\"w\"> </span><span class=\"nf\">HelloWorld</span><span class=\"p\">()</span><span class=\"w\"> </span><span class=\"p\">{</span><span class=\"w\">\n</span><span class=\"w\">\t</span><span class=\"nx\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Println</span><span class=\"p\">(</span><span class=\"s\">&#34;Hello World&#34;</span><span class=\"p\">)</span><span class=\"w\">\n</span><span class=\"w\"></span><span class=\"p\">}</span></code></pre>\n</div>"

looks that's because in alecthomas/chroma@d0ad679 , golang rulers was updated. the ui is ok in my test. so I think just update the test code is enough. @silverwind @sebastianertz

image

@silverwind
Copy link
Member

Yeah just update the failing test value assertions.

BTW I wish we would have snapshot-based testing (for example https://github.com/gkampitakis/go-snaps), then we could automate such updates with a single command (like make test-backend-update).

@wxiaoguang
Copy link
Contributor

BTW I wish we would have snapshot-based testing (for example https://github.com/gkampitakis/go-snaps), then we could automate such updates with a single command (like make test-backend-update).

It only introduces more garbage into code base. Every test should be designed & written carefully, and has a clear purpose, otherwise, there is already a lot of incomprehensible & unmaintainable code like

func TestTotal_RenderString(t *testing.T) {


For this one, it could simply test some simple code like this to confirm that "the render works".

#+begin_src c
int a;
#+end_src

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. modifies/dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants