#308 bug in diff web view with lines starting with minus characters

Closed
opened 1 week ago by sh2d · 21 comments
sh2d commented 1 week ago

In Lua, single line comments start with two minus characters --. Codeberg’s diff web view seems to have problems showing removed Lua comment lines that start at the first column. Added comment lines or comment lines starting with indentation seem not to be affected.

Recipe:

$ git clone https://codeberg.org/sh2d/padrinoma.git
$ git log -p fb32cf2 lua-modules/pdnm_nl_manipulation.lua

The latter command shows correctly:

diff --git a/lua-modules/pdnm_nl_manipulation.lua b/lua-modules/pdnm_nl_manipulation.lua
index 1f485fd..3e73c04 100644
--- a/lua-modules/pdnm_nl_manipulation.lua
+++ b/lua-modules/pdnm_nl_manipulation.lua
@@ -59,7 +59,7 @@
 
 
 
--- Table for module identification (see below).
+-- Table for module identification.

Watching the same commit at Codeberg, fb32cf2f69 (scroll down to see file lua-modules/pdnm_nl_manipulation.lua) the removed comment line is missing (ln. 62).

Guesswork: Problematic lines in the diff start with multiple minus characters -. One minus character being diff’s removed line indicator and two introducing a Lua comment.

In Lua, single line comments start with two minus characters `--`. Codeberg's diff web view seems to have problems showing _removed_ Lua comment lines that start at the first column. Added comment lines or comment lines starting with indentation seem not to be affected. Recipe: ``` sh $ git clone https://codeberg.org/sh2d/padrinoma.git $ git log -p fb32cf2 lua-modules/pdnm_nl_manipulation.lua ``` The latter command shows correctly: ``` text diff --git a/lua-modules/pdnm_nl_manipulation.lua b/lua-modules/pdnm_nl_manipulation.lua index 1f485fd..3e73c04 100644 --- a/lua-modules/pdnm_nl_manipulation.lua +++ b/lua-modules/pdnm_nl_manipulation.lua @@ -59,7 +59,7 @@ --- Table for module identification (see below). +-- Table for module identification. ``` Watching the same commit at Codeberg, https://codeberg.org/sh2d/padrinoma/commit/fb32cf2f6908e3645aca1590ae4c56ad05f17cae (scroll down to see file `lua-modules/pdnm_nl_manipulation.lua`) the removed comment line is missing (ln. 62). Guesswork: Problematic lines in the diff start with multiple minus characters `-`. One minus character being diff's removed line indicator and two introducing a Lua comment.
sh2d changed title from bug in diff web view with Lua comments to bug in diff web view with lines starting with minus characters 1 week ago
hw added the
bug
label 1 week ago
hw added the
gitea-related issue
label 1 week ago
6543 commented 1 week ago
Collaborator

@sh2d did you create an issue upstream?

@sh2d did you create an issue upstream?
6543 commented 1 week ago
Collaborator

I think this is a regex issue in modules/git

I think this is a regex issue in **modules/git**
sh2d commented 1 week ago
Poster

@6543 Not yet. I’m just browsing the tracker. Hell, 1.300 open issues? Is that a software I wanna use? Quiet disappointing. Will add one more issue after I finished browsing.

@6543 Not yet. I'm just browsing the tracker. Hell, 1.300 open issues? Is that a software I wanna use? Quiet disappointing. Will add one more issue after I finished browsing.
hw commented 1 week ago
Owner

:) yes. Check the rate of closed issues too ;)

:) yes. Check the rate of closed issues too ;)
sh2d commented 1 week ago
Poster

Could anybody shed some light on how to find out the Gitea version Codeberg is running?

Could anybody shed some light on how to find out the Gitea version Codeberg is running?
lhinderberger commented 1 week ago
Collaborator

@sh2d It’s explained at https://docs.codeberg.org/faq/ - currently it’s 1.12.5+20-g7494b24c6

@sh2d It's explained at https://docs.codeberg.org/faq/ - currently it's `1.12.5+20-g7494b24c6`
lhinderberger commented 1 week ago
Collaborator

By the way, you’re right - Gitea is a bit of a bug-fest. But at the moment, it seems like it’s the best alternative available for a project like Codeberg. And while the issue tracker seems to be overflowing, PRs seem to get processed quite quickly :)

By the way, you're right - Gitea is a bit of a bug-fest. But at the moment, it seems like it's the best alternative available for a project like Codeberg. And while the issue tracker seems to be overflowing, PRs seem to get processed quite quickly :)
sh2d commented 1 week ago
Poster

@lhinderberger Ah, thank you! About the number of Gitea bugs, as @hw hinted to, last month saw more bugs closed than opened at least.

@lhinderberger Ah, thank you! About the number of Gitea bugs, as @hw hinted to, last month saw more bugs closed than opened at least.
6543 commented 1 week ago
Collaborator

issues with bug lable: 138 open | 1394 closed 😅 - gitea get a lot of feature requests too

issues with bug lable: `138 open | 1394 closed` :sweat_smile: - gitea get **a lot** of feature requests too
6543 commented 1 week ago
Collaborator

@sh2d gita’s maintainer are now aware of this ... so keep clam and hope we find time to fix it 😓

@sh2d gita's maintainer are now aware of this ... so keep clam and hope we find time to fix it 😓
sh2d commented 6 days ago
Poster

One more observation, lines with leading plus sign characters are also affected. You can find a test repo at https://codeberg-test.org/sh2d/plus

$ git show -1
commit dd03689a37ab1adfb7fcb122693b54339d2c819f (HEAD -> master, origin/master)
Author: Stephan Hennig <sh2d@posteo.net>
Date:   Tue Oct 13 18:25:27 2020 +0200

    Change leading -- to ++

diff --git a/test.txt b/test.txt
index c7ddeba..076580a 100644
--- a/test.txt
+++ b/test.txt
@@ -1,4 +1,4 @@
 A test file.
--- Starting with minus sign characters.
+++ Starting with plus sign characters.
 Another line.
-Trailing line.
+Trailing long line.

Web view of commit dd03689a37 shows none of the removed/added lines containing the text “Starting with ...", but the change of the trailing line only.

One more observation, lines with leading plus sign characters are also affected. You can find a test repo at https://codeberg-test.org/sh2d/plus ``` text $ git show -1 commit dd03689a37ab1adfb7fcb122693b54339d2c819f (HEAD -> master, origin/master) Author: Stephan Hennig <sh2d@posteo.net> Date: Tue Oct 13 18:25:27 2020 +0200 Change leading -- to ++ diff --git a/test.txt b/test.txt index c7ddeba..076580a 100644 --- a/test.txt +++ b/test.txt @@ -1,4 +1,4 @@ A test file. --- Starting with minus sign characters. +++ Starting with plus sign characters. Another line. -Trailing line. +Trailing long line. ``` Web view of commit https://codeberg-test.org/sh2d/plus/commit/dd03689a37ab1adfb7fcb122693b54339d2c819f shows none of the removed/added lines containing the text "Starting with ...", but the change of the trailing line only.
zeripath commented 6 days ago

OK the cause of this is the incorrect parsing of the patch files in services/gitdiff/gitdiff.go:449:ParsePatch

It’s just wrong.

OK the cause of this is the incorrect parsing of the patch files in `services/gitdiff/gitdiff.go:449:ParsePatch` It's just wrong.
6543 commented 4 days ago
Collaborator
@zeripath rocks! -> https://github.com/go-gitea/gitea/pull/13157
hw commented 4 days ago
Owner

hehe :)

hehe :)
ashimokawa commented 4 days ago
Owner

Cool, will watch the PR discussion, seems a bit too early to cherry pick, but the fix is near :)

Cool, will watch the PR discussion, seems a bit too early to cherry pick, but the fix is near :)
ashimokawa commented 3 days ago
Owner

Deployed!

@sh2d

Should be fixed

Deployed! @sh2d Should be fixed
ashimokawa commented 3 days ago
Owner

EDIT: Sorry it has not been merged in 1.13 yet, only 1.12 and master, codeberg-test.org runs 1.13-rc1+

@zeripath

This still looks broken though:

dd03689a37

The patch stats are ok now (2 lines changed) but the diff still misses one line afaics...

**EDIT:** Sorry it has not been merged in 1.13 yet, only 1.12 and master, codeberg-test.org runs 1.13-rc1+ @zeripath ~~This still looks broken though:~~ https://codeberg-test.org/sh2d/plus/commit/dd03689a37ab1adfb7fcb122693b54339d2c819f ~~The patch stats are ok now (2 lines changed) but the diff still misses one line afaics...~~
sh2d closed this issue 3 days ago
sh2d reopened this issue 3 days ago
sh2d commented 3 days ago
Poster

Sorry, hit the wrong button.

Sorry, hit the wrong button.
ashimokawa commented 3 days ago
Owner

@sh2d
It should be fixed on codeberg.org just not on codeberg-test.org, can you confirm?

@sh2d It should be fixed on codeberg.org just not on codeberg-test.org, can you confirm?
ashimokawa commented 3 days ago
Owner

The fix is now also deployed on codeberg-test.org!

I think this can be closed. @sh2d

The fix is now also deployed on codeberg-test.org! I think this can be closed. @sh2d
sh2d commented 3 days ago
Poster

I can confirm that I cannot reproduce the issue anymore. Another test commit 0bdd9bbef5 looks fine, too (cf. file examples/luatex/break-ligatures/pdnm_break-ligatures.lua).

The thing is, I didn’t wanted to close the issue myself, but was actually looking for a “discard” button. Closing the issue now.

I can confirm that I cannot reproduce the issue anymore. Another test commit https://codeberg.org/sh2d/padrinoma/commit/0bdd9bbef59b421d7727c279ef3ada7082cb8742 looks fine, too (cf. file examples/luatex/break-ligatures/pdnm_break-ligatures.lua). The thing is, I didn't wanted to close the issue myself, but was actually looking for a "discard" button. Closing the issue now.
sh2d closed this issue 3 days ago
Sign in to join this conversation.
No Milestone
No Assignees
6 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.