Skip to content
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.

Sign up for GitHub

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

Jump to bottom

pull requests: wrong line numbers in discussions #17875

Closed
rnwgnr opened this issue Dec 1, 2021 · 3 comments · Fixed by #18029
Closed

pull requests: wrong line numbers in discussions #17875

rnwgnr opened this issue Dec 1, 2021 · 3 comments · Fixed by #18029
Labels
type/bug
Milestone
1.16.0

Comments

@rnwgnr
Copy link

rnwgnr commented Dec 1, 2021 •
edited
Loading

Gitea Version

1.16.0+dev-630-g042cac5fe

Git Version

No response

Operating System

No response

How are you running Gitea?

tested on try.gitea.io with a migrated repo

Database

No response

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Description

demo: https://try.gitea.io/murks/Vereinsdokumente/pulls/7

I've created a comment for line 23 in a PR. While everything is fine in the "Files Changed" tab, the text block in the conversation view is off-by-one which brings the comments out of context.

Screenshots

Files Changed tab:
image

Conversation tab with wrong text section:
image
@lunny
Copy link
Member

lunny commented Dec 2, 2021

So which line is the right one?

@rnwgnr
Copy link
Author

rnwgnr commented Dec 2, 2021

Sorry, i've forget to add that information:
The comment has been created for line 23.

If you look at the screen of the Conversation tab you'll notice that the line numbers are adjusted to line 23, but the text is off-by-one. ## §4 Umgang mit der SARS-Cov-2-Pandemie should be line 21 (as shown in the Files Changed tab), but is shown as line 22 in the Conversation view.

@jpraet
Copy link
Member

jpraet commented Dec 3, 2021

Possibly some corner case in CutDiffAroundLine (https://github.com/go-gitea/gitea/blob/main/modules/git/diff.go#L135)?
I suspect it may have something to do with the \ No newline at end of file.
https://try.gitea.io/murks/Vereinsdokumente/pulls/7.diff

/cc @zeripath

@lunny lunny added this to the 1.16.0 milestone Dec 5, 2021
zeripath added a commit to zeripath/gitea that referenced this issue Dec 19, 2021
@zeripath
Prevent off-by-one error on comments on newly appended lines
01abb91 
There was a bug in CutDiffAroundLine whereby if a file without a terminal new line
has a patch which appends lines to it and a comment is placed on one of those lines
the comment diff will be a line out of place.

This fixes CutDiffAroundLine to simply ignore the missing terminal newline - however,
we should really improve this rendering to add a marker to say that there was a
previously missing terminal newline.

Fix go-gitea#17875

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this issue Dec 19, 2021
Merged
zeripath added a commit to zeripath/gitea that referenced this issue Dec 19, 2021
@zeripath
Prevent off-by-one error on comments on newly appended lines (go-gite…
8c0bbd2 
…a#18029)

Backport go-gitea#18029

There was a bug in CutDiffAroundLine whereby if a file without a terminal new line
has a patch which appends lines to it and a comment is placed on one of those lines
the comment diff will be a line out of place.

This fixes CutDiffAroundLine to simply ignore the missing terminal newline - however,
we should really improve this rendering to add a marker to say that there was a
previously missing terminal newline.

Fix go-gitea#17875

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this issue Dec 19, 2021
Merged
zeripath added a commit that referenced this issue Dec 20, 2021
@zeripath
Prevent off-by-one error on comments on newly appended lines (#18029)
fb5f779 
* Prevent off-by-one error on comments on newly appended lines

There was a bug in CutDiffAroundLine whereby if a file without a terminal new line
has a patch which appends lines to it and a comment is placed on one of those lines
the comment diff will be a line out of place.

This fixes CutDiffAroundLine to simply ignore the missing terminal newline - however,
we should really improve this rendering to add a marker to say that there was a
previously missing terminal newline.

Fix #17875

Signed-off-by: Andrew Thornton <art27@cantab.net>
techknowlogick pushed a commit that referenced this issue Dec 20, 2021
@zeripath @KN4CK3R @lunny
Prevent off-by-one error on comments on newly appended lines (#18029) (…
148a417 
…#18035)

* Prevent off-by-one error on comments on newly appended lines (#18029)

Backport #18029

There was a bug in CutDiffAroundLine whereby if a file without a terminal new line
has a patch which appends lines to it and a comment is placed on one of those lines
the comment diff will be a line out of place.

This fixes CutDiffAroundLine to simply ignore the missing terminal newline - however,
we should really improve this rendering to add a marker to say that there was a
previously missing terminal newline.

Fix #17875

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Apply suggestions from code review

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Chianina pushed a commit to Chianina/gitea that referenced this issue Mar 28, 2022
@zeripath
Prevent off-by-one error on comments on newly appended lines (go-gite…
a54273d 
…a#18029)

* Prevent off-by-one error on comments on newly appended lines

There was a bug in CutDiffAroundLine whereby if a file without a terminal new line
has a patch which appends lines to it and a comment is placed on one of those lines
the comment diff will be a line out of place.

This fixes CutDiffAroundLine to simply ignore the missing terminal newline - however,
we should really improve this rendering to add a marker to say that there was a
previously missing terminal newline.

Fix go-gitea#17875

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
type/bug
Projects
None yet
Milestone
1.16.0
Development

Successfully merging a pull request may close this issue.

Prevent off-by-one error on comments on newly appended lines zeripath/gitea
4 participants
@lunny @silverwind @jpraet @rnwgnr