Skip to content

enhance(diff): add plain view for long line files - #39202

Draft
SergioBenitez wants to merge 1 commit into
go-gitea:mainfrom
SergioBenitez:enhance/plain-long-line-diffs
Draft

enhance(diff): add plain view for long line files#39202
SergioBenitez wants to merge 1 commit into
go-gitea:mainfrom
SergioBenitez:enhance/plain-long-line-diffs

Conversation

@SergioBenitez

Copy link
Copy Markdown
Contributor

Add a "View plain diff" button when a file contains lines that exceed the configured rendering limit. The button opens the complete Git patch for that file as plain text in a new tab, without syntax highlighting.

The link adds plain=true to existing diff URLs. The commit, comparison, and pull-request handlers then stream the selected patch as text/plain, reusing the existing files parameters to identify the file, including both paths of a rename. Plain requests validate that selection and pass the names to Git literally (using --literal-pathspecs) so they cannot expand to unrelated files.

The existing rendered-diff limits and repository access checks remain unchanged. Tests were added to exercise the new behavior.

Screenshot 2026-09-01 at 3 03 12 PM

Add a "View plain diff" button when a file contains lines that exceed
the configured rendering limit. The button opens the complete Git patch
for that file as plain text in a new tab, without syntax highlighting.

The link adds `plain=true` to existing diff URLs. The commit,
comparison, and pull-request handlers then stream the selected patch as
text/plain, reusing the existing files parameters to identify the file,
including both paths of a rename. Plain requests validate that selection
and pass the names to Git literally (using `--literal-pathspecs`) so
they cannot expand to unrelated files.

The existing rendered-diff limits and repository access checks remain
unchanged. Tests were added to exercise the new behavior.
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 1, 2026
@github-actions github-actions Bot added the type/enhancement An improvement of existing functionality label Sep 1, 2026
@wxiaoguang

Copy link
Copy Markdown
Contributor

Why it needs to introduce more workarounds? Can you increase your MAX_GIT_DIFF_LINE_CHARACTERS?

@wxiaoguang
wxiaoguang marked this pull request as draft September 1, 2026 13:28
@SergioBenitez

Copy link
Copy Markdown
Contributor Author

It's not really a work-around. As long as there's a limit, there will be cases in which you can't currently view the diff at all. And removing the limit entirely would mean being susceptible to DOS attacks, since the parser (ParsePatch) allocates a buffer equal to the limit for every diff request.

@SergioBenitez
SergioBenitez marked this pull request as ready for review September 1, 2026 14:13
@wxiaoguang

wxiaoguang commented Sep 1, 2026

Copy link
Copy Markdown
Contributor
  • Why you need it?

  • Why can't you change MAX_GIT_DIFF_LINE_CHARACTERS, 10000 or 20000?

  • Are human really able to review a diff line which is longer than 5000 or 10000 chars or would anyone really write such a long line in real world?

@wxiaoguang
wxiaoguang marked this pull request as draft September 4, 2026 08:20
@wxiaoguang wxiaoguang added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Sep 4, 2026
@SergioBenitez

Copy link
Copy Markdown
Contributor Author
  • Why you need it?

I'd like to be able to view any kind of diff in the browser via gitea. This PR makes that possible.

  • Why can't you change MAX_GIT_DIFF_LINE_CHARACTERS, 10000 or 20000?

You can, but this has usability and performance implications that this PR does not. Doing what you suggest means 1) figuring what the max line size will ever be on my server, or changing it if a new max line length appears, and 2) paying the memory cost associated with that max line setting even when it's extremely rare. The latter is because gitdiff.go does this:

	// OK let's set a reasonable buffer size.
	// This should be at least the size of maxLineCharacters or 4096 whichever is larger.
	readerSize := max(maxLineCharacters, 4096)

This PR has neither of those drawbacks: you can keep the max line length to something reasonable while, in the rare case that you do get some absurdly long line length, still being able to view the diff.

  • Are human really able to review a diff line which is longer than 5000 or 10000 chars or would anyone really write such a long line in real world?

Maybe? Either the plain diff makes the change really obvious, in which case yes, or it doesn't, so you open it up in a better tool that helps you review it. But for either case to happen, you need some way to know what the diff is at all. and this PR give you that. Otherwise, there are just diffs that gitea gives you absolutely no way to view at all, without pulling whatever the diff refers to locally.

@wxiaoguang

Copy link
Copy Markdown
Contributor
  • Are human really able to review a diff line which is longer than 5000 or 10000 chars or would anyone really write such a long line in real world?

Maybe? Either the plain diff makes the change really obvious, in which case yes, or it doesn't, so you open it up in a better tool that helps you review it. But for either case to happen, you need some way to know what the diff is at all. and this PR give you that. Otherwise, there are just diffs that gitea gives you absolutely no way to view at all, without pulling whatever the diff refers to locally.

Do you have a real world use case? For example: what kind of file has more than 10000 or 20000 chars in one line to diff and need to be reviewed? At least, it need to clarify the use case and leave comments for it.

@SergioBenitez

Copy link
Copy Markdown
Contributor Author

This happened when I had a file with one minified line (javascript) embedded in otherwise normal changes. I wanted to review the other changes at a glance before dropping in to an editor that could un-minify the line.

@wxiaoguang

wxiaoguang commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

This happened when I had a file with one minified line (javascript) embedded in otherwise normal changes. I wanted to review the other changes at a glance before dropping in to an editor that could un-minify the line.

Then such diff result usually is useless because a huge line is changed.

You can simply click the "View file" menu item to open the file and have a full view (review).

So I still don't think it is a real world use case or it's worth to make the code base and logic more complicated than it should be.

Details image

@wxiaoguang

wxiaoguang commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

I wanted to review the other changes at a glance before dropping in to an editor that could un-minify the line.

If you'd like to review other short lines, the best UI/UX is to render all the short lines, only ignore the long line.

Then end users can see the diff result at first glance, no need to click the "load" button.

So the render logic can be changed to this:

  • If a line is too long, only ignore that line , and mark it as "too long (incomplete)"
  • Still render other short lines.

Haven't looked into details, I guess it is feasible and brings better user experience.

@SergioBenitez

Copy link
Copy Markdown
Contributor Author

I agree, that would be nicer. I did consider it, but it seemed like a must harder change than the one proposed here. Still, I'm happy to investigate if that's your preference.

@wxiaoguang

wxiaoguang commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

I agree, that would be nicer. I did consider it, but it seemed like a must harder change than the one proposed here. Still, I'm happy to investigate if that's your preference.

Thank you. TBH I guess it is easier than "reload the raw diff". I also found that IsIncompleteLineTooLong was never tested, so it's also a good chance to remove it, and only mark long lines as incomplete (per line).

The new logic can be covered by a unit test in services/gitdiff/gitdiff_test.go. The UI part only needs to render some extra tags for the incomplete lines.

I will also take a look later.

@wxiaoguang wxiaoguang removed the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Sep 5, 2026
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. type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants