Display issue in commit diff

[fnetX]

• Gitea version (or commit ref): try.gitea.io or codeberg.org
• Operating system: Debian GNU/Linux 10 (Buster)
• Yes, @6543 could reproduce the bug at https://try.gitea.io/6543/vturn_server/commit/8d672a398f22ea465edd42a6d9de7cd2e88c7ab5

Description

First reported on https://codeberg.org/Codeberg/Community/issues/374

I found a display issue in this commit diff view: https://codeberg.org/eventbike/vturn_server/commit/8d672a398f22ea465edd42a6d9de7cd2e88c7ab5 (main.v, line 27)

Displayed line:
run(dban class="o">)
Actual file content:
run(db)

HTML code at this is point:

<td class="lines-code lines-code-new halfwidth add-code"><span class="mono wrap">
<span class="n">run</span><span class="added-code"><span class="o">(
</span><span class="n">d</span>b</span>
<sp<span class="added-code">an class="o</sp<span></span>"&gt;)</td>

Screenshots

Thank you a lot.

[bagasme]

@fnetX i see that for your case as noted by your screenshot, the wrong color for brackets at run().

[fnetX]

You mean the brackets have the wrong colour? Yes, but that's because the whole HTML is messed up here.

[zeripath]

diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go
index cd7b2273c..813260f3b 100644
--- a/services/gitdiff/gitdiff_test.go
+++ b/services/gitdiff/gitdiff_test.go
@@ -15,6 +15,7 @@ import (
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/highlight"
 	"code.gitea.io/gitea/modules/setting"
 	dmp "github.com/sergi/go-diff/diffmatchpatch"
 	"github.com/stretchr/testify/assert"
@@ -101,6 +102,15 @@ func TestDiffToHTML(t *testing.T) {
 	}, DiffLineAdd))
 }
 
+func TestDiffToHTML_14231(t *testing.T) {
+	setting.Cfg = ini.Empty()
+
+	diffRecord := diffMatchPatch.DiffMain(highlight.Code("main.v", "		run()\n"), highlight.Code("main.v", "		run(db)\n"), true)
+	diffRecord = diffMatchPatch.DiffCleanupEfficiency(diffRecord)
+	assertEqual(t, `		<span class="n">run</span><span class="added-code"><span class="o">(</span><span class="n">db</span><span class="o">)</span></span>`,
+		diffToHTML("main.v", diffRecord, DiffLineAdd))
+}
+
 func TestParsePatch_singlefile(t *testing.T) {
 	type testcase struct {
 		name        string

will add a testcase to work with

[zeripath]

the diffRecord is:

 [
    {Equal 		<span class="n">run</span><span class="}
    {Insert o">(</span><span class="n">d}
    {Equal b}
    {Insert </span><s}
    {Equal p}
    {Insert an class="o}
    {Equal ">}
    {Delete (} {Equal )</span>}
]

[zeripath]

I honestly think the regexp approach cannot work here.

Record 2 here is an attribute in deleted case and is text in added case.

[zeripath]

In my previous reverted pr I attempted to do the kind of necessary fixup.

Now, we have several options:

1. Abandon highlighting of diffs.
2. Do the kind of fixup I initially suggested - but correctly.
3. Tokenize the output of chroma highlight and use a token aware diff or block diff. We'd need to copy the code from the current diff library and adjust it to make the work in tokens.
4. Improve the regexp but add some kind of fixup.

[zeripath]

thinking on here - I think the fixup technique is the correct answer.

Intention of the proposed algo - No Equals/Insert/Delete block starts or ends within an element, entity or even within an unclosed span. (EDIT: this last case needs a bit more thought)

1. If an equals tag and normal state ends with an incomplete element or entity - split off the incomplete bit and add it to insert & delete stacks. Set state for both insert/delete to in entity in element, in element, in entity or normal, and move to next tag.
2. If an insert tag - prepend insert stack and clear stack. Check end of tag and split off incomplete bit adding it to insert stack. Set state for insert to in entity in element, in element, in entity or normal, and move to next tag.
3. If a delete tag - prepend delete stack and clear stack. Check end of tag and split off incomplete bit adding it to delete stack. Set state for delete to in entity in element, in element, in entity or normal, and move to next tag.
4. If an equals tag (not normal state) then this has to be removed at least until the end of element/entity and its content added to both the insert and delete stack. If there is a complete element/entity then insert and delete tags should be inserted before the Equal. Do the rest as per the end of the stack.
5. Merge insert/delete tags.

I think that would do it.

[fnetX]

@zeripath Thank you!

[zeripath]

@fnetX Sorry it's taken so long. It was unfortunately a non-trivial bug.

[zeripath]

I should also add I was significantly delayed by breaking my left hand over the Christmas period.