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.

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

Allow on-demand loading of too large file diffs #17738

Closed
jpraet opened this issue Nov 20, 2021 · 5 comments · Fixed by #17739
Closed

Allow on-demand loading of too large file diffs #17738

jpraet opened this issue Nov 20, 2021 · 5 comments · Fixed by #17739

Comments

@jpraet
Copy link
Member

jpraet commented Nov 20, 2021

Feature Description

On gitea, file diffs are suppressed if they are too large
(app.ini MAX_GIT_DIFF_LINES)
image
or contain lines that are too long
(app.ini MAX_GIT_DIFF_LINE_CHARACTERS)
image.

It would be nice if these could be explicitly loaded on-demand anyway. Like is now supported for "Show More" if the diff contains too many files.

Screenshots

GitHub supports this:
image

zeripath added a commit to zeripath/gitea that referenced this issue Nov 20, 2021
This PR allows the loading of diffs that are suppressed because the file
is too large. It does not handle diffs of files which have lines which
are too long.

Fix go-gitea#17738

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath
Copy link
Contributor

I was already working on this...

This may not be the correct thing as we should probably really only produce a diff of a certain size.

@zeripath
Copy link
Contributor

zeripath commented Nov 21, 2021

#17739 fixes the issue with files that have too many lines - by allowing you to specifically suffer the delayed load times yourself. (As I say we really should be only loading a limited number of lines and then dynamically loading next 1000 or so lines as needed.)

Lines which are too long are a more difficult issue as they could crash the browser or even OOM gitea itself.

@silverwind
Copy link
Member

silverwind commented Nov 21, 2021

Browsers should have no issues with long wrapped lines in general.

I think we could combine the two limits (line length and line count, which are just one-dimensional limits) to just a byte count, which is a more neutral measure of size. It should apply to both diff and file view.

@zeripath
Copy link
Contributor

Browsers should have no issues with long wrapped lines in general.

Its not the length of the lines its the number of elements in that line.

I think we could combine the two limits (line length and line count, which are just one-dimensional limits) to just a byte count, which is a more neutral measure of size. It should apply to both diff and file view.

It's not as simple as that. We tokenize and diff whole lines. That's why the line length matters.

Now for very long lines we can abandon syntax highlighting these lines which will drop the tokenizing problem. Intra line diffing could still be done but it's still different so these lines would need some kind of marking.

wxiaoguang pushed a commit that referenced this issue Nov 21, 2021
* Allow Loading of Diffs that are too large

This PR allows the loading of diffs that are suppressed because the file
is too large. It does not handle diffs of files which have lines which
are too long.

Fix #17738

Signed-off-by: Andrew Thornton <[email protected]>
@silverwind
Copy link
Member

silverwind commented Nov 22, 2021

Now for very long lines we can abandon syntax highlighting these lines which will drop the tokenizing problem

IIRC, this is what GitHub does. On file with like 20k+ lines, syntax highlighting is no longer applied for performance reasons. Either that, or syntax highlighting stops after line 20k.

Chianina pushed a commit to Chianina/gitea that referenced this issue Mar 28, 2022
* Allow Loading of Diffs that are too large

This PR allows the loading of diffs that are suppressed because the file
is too large. It does not handle diffs of files which have lines which
are too long.

Fix go-gitea#17738

Signed-off-by: Andrew Thornton <[email protected]>
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants