Skip to content

Fix git diff hunk 0 based start line #35047

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

Closed
wants to merge 1 commit into from

Conversation

lunny
Copy link
Member

@lunny lunny commented Jul 11, 2025

Fix #35040

The git diff hunk @@ -1,1 +0,4 @@ is a 0 based one which should not exist. The hunk is generated by the function CutDiffAroundLine which should be fixed in that function.

Before:

Image

After:

image

@lunny lunny added the type/bug label Jul 11, 2025
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 11, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Jul 11, 2025
@wxiaoguang
Copy link
Contributor

  1. Why not +1 to convert 0-based to 1-based?
  2. How to reproduce and test?
  3. Why not merge into Fix incorrect comment diff hunk parsing, fix github asset ID nil panic #35046 and fix together?

@wxiaoguang
Copy link
Contributor

And, isn't the bug here?

image

@lunny
Copy link
Member Author

lunny commented Jul 11, 2025

  1. Why not +1 to convert 0-based to 1-based?

It's 1-based but there is a 0 there.

  1. How to reproduce and test?

Ref #35040 , I updated the content.

  1. Why not merge into Fix a bug that the code diff hunk missing one @ #35046 and fix together?

I’m not certain this is the best fix, but the other one is clearly correct.

@wxiaoguang
Copy link
Contributor

  1. Why not +1 to convert 0-based to 1-based?

It's 1-based but there is a 0 there.

Can you look into why 0 is there?

And, isn't the bug here?

#35047 (comment)

@wxiaoguang
Copy link
Contributor

3. How to reproduce and test?

Ref #35040 , I updated the content.

How to use the "content" to reproduce and write a test?

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jul 11, 2025

The root problem is there is a bug in getDiffLineSectionInfo

image

@wxiaoguang
Copy link
Contributor

-> Fix incorrect git diff hunk parsing, fix github asset ID nil panic #35046

@wxiaoguang wxiaoguang closed this Jul 11, 2025
@lunny lunny deleted the lunny/fix_pr_review_0_idx branch July 11, 2025 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/go Pull requests that update Go code type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code review comment on pull request line number is wrong
3 participants