Display specific message if diff is not displayed because of too long line

[Loutro]

If file diff incomplete because one line is too long, display specific message

Fix #7184

[codecov-commenter]

Codecov Report

Merging #15611 (b356a52) into master (1cd3017) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #15611      +/-   ##
==========================================
- Coverage   43.89%   43.82%   -0.07%     
==========================================
  Files         678      678              
  Lines       81878    81881       +3     
==========================================
- Hits        35941    35886      -55     
- Misses      40087    40153      +66     
+ Partials     5850     5842       -8     

Impacted Files	Coverage Δ
services/gitdiff/gitdiff.go	73.76% <100.00%> (+1.52%)	⬆️
modules/notification/ui/ui.go	58.97% <0.00%> (-6.84%)	⬇️
models/issue_comment.go	45.97% <0.00%> (-6.61%)	⬇️
modules/notification/mail/mail.go	33.67% <0.00%> (-5.11%)	⬇️
modules/notification/base/null.go	76.31% <0.00%> (-2.64%)	⬇️
modules/notification/notification.go	85.12% <0.00%> (-2.48%)	⬇️
modules/process/manager.go	72.83% <0.00%> (-2.47%)	⬇️
modules/queue/workerpool.go	51.64% <0.00%> (-1.10%)	⬇️
models/notification.go	65.18% <0.00%> (-0.89%)	⬇️
services/pull/pull.go	42.92% <0.00%> (-0.46%)	⬇️
... and 6 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cd3017...b356a52. Read the comment docs.

[silverwind]

Tested, did not work for me with this file because I think there is a off-by-one error in the code. So the code initializes a reader

readerSize := maxLineCharacters
input := bufio.NewReaderSize(reader, readerSize)

This reader is passed to parseHunks which then reads a chunk of maxLineCharacters characters and I think the if condition below can probably never be true because len(line) will at most be equal to maxLineCharacters.

lineBytes, isFragment, err = input.ReadLine()
line := string(lineBytes)

if len(line) > maxLineCharacters {

I think we need to use >=.

cc: @zeripath

[Loutro]

Tested, did not work for me with this file because I think there is a off-by-one error in the code. So the code initializes a reader

readerSize := maxLineCharacters
input := bufio.NewReaderSize(reader, readerSize)

This reader is passed to parseHunks which then reads a chunk of maxLineCharacters characters and I think the if condition below can probably never be true because len(line) will at most be equal to maxLineCharacters.

lineBytes, isFragment, err = input.ReadLine()
line := string(lineBytes)

if len(line) > maxLineCharacters {

I think we need to use >=.

cc: @zeripath

Thanks for the test.
Can we go with something like that:

readerSize := maxLineCharacters
	if readerSize < 4096 {
		readerSize = 4096
	} else {
		readerSize++
	}

If the line's length is superior to maxLineCharacters we get the new message.
But if length is equal, we display the file.

[zeripath]

agh sorry I had thought that the OP had tried this so just looked at the proposed PR and thought it looked alright.

[zeripath]

Right we want to look at how too long a line could cause IsIncomplete set. So the easiest thing to do is to look at all the places where IsIncomplete is set and see why that would occur:

gitea/services/gitdiff/gitdiff.go

Lines 713 to 714 in ca0460b

	if len(diff.Files) >= maxFiles {
	diff.IsIncomplete = true

This is due to maxFiles - so this one is fine.

gitea/services/gitdiff/gitdiff.go

Lines 935 to 944 in ca0460b

	for {
	for isFragment {
	curFile.IsIncomplete = true
	_, isFragment, err = input.ReadLine()
	if err != nil {
	// Now by the definition of ReadLine this cannot be io.EOF
	err = fmt.Errorf("Unable to ReadLine: %v", err)
	return
	}
	}

This one is due to a line that is so long that it is larger than the buffer can buffer. So this is one that needs to have the IncompleteLine flag set.

gitea/services/gitdiff/gitdiff.go

Lines 961 to 964 in ca0460b

	if curFileLinesCount >= maxLines {
	curFile.IsIncomplete = true
	continue
	}

gitea/services/gitdiff/gitdiff.go

Lines 997 to 1000 in ca0460b

	if curFileLinesCount >= maxLines {
	curFile.IsIncomplete = true
	continue
	}

gitea/services/gitdiff/gitdiff.go

Lines 1012 to 1015 in ca0460b

	if curFileLinesCount >= maxLines {
	curFile.IsIncomplete = true
	continue
	}

gitea/services/gitdiff/gitdiff.go

Lines 1027 to 1029 in ca0460b

	if curFileLinesCount >= maxLines {
	curFile.IsIncomplete = true
	continue

gitea/services/gitdiff/gitdiff.go

Lines 1042 to 1045 in ca0460b

	curFileLinesCount++
	if curFileLinesCount >= maxLines {
	curFile.IsIncomplete = true
	continue

these are too many lines - so is fine.

gitea/services/gitdiff/gitdiff.go

Lines 1063 to 1073 in ca0460b

	if isFragment {
	curFile.IsIncomplete = true
	for isFragment {
	lineBytes, isFragment, err = input.ReadLine()
	if err != nil {
	// Now by the definition of ReadLine this cannot be io.EOF
	err = fmt.Errorf("Unable to ReadLine: %v", err)
	return
	}
	}
	}

This one is due to a line that is so long that it is larger than the buffer can buffer. So this is one that needs to have the IncompleteLine flag set.

gitea/services/gitdiff/gitdiff.go

Lines 1074 to 1077 in ca0460b

	if len(line) > maxLineCharacters {
	curFile.IsIncomplete = true
	line = line[:maxLineCharacters]
	}

This is a line that although less than the buffer is still too long for the maxLineCharacters setting so it needs the flag.

@Loutro there are therefore two other places you need to set this flag - which I have set in 263f33c

[zeripath]

I've just pushed those two missing cases up to your branch.

@silverwind please feel free to test again.

[Loutro]

@zeripath , If i understand well, The cases you added are useful if you are in error because you will never go on this check:
if len(line) > maxLineCharacters
But i suppose an error could occur for many things, not only a buffer problem. if it's why we want here it's ok, or we could provide a specific message to the user ("an error occured patati patata").

If there is no error in reading the file, the solution i provided works.

	if readerSize < 4096 {
		readerSize = 4096
	} else {
		readerSize++
	}

[silverwind]

Current solution works for me now with my test case.

[zeripath]

@Loutro I don't think you need that change - I don't understand how it is supposed to help.

[Loutro]

@Loutro I don't think you need that change - I don't understand how it is supposed to help.

it help because on the condition below, your len(line) is maxLineCharacters + 1 if line too long.
if len(line) > maxLineCharacters {

Has i previously said, If the line's length is superior to maxLineCharacters we get the new message.
But if length is equal to maxLineCharacters (and that the important part) , we display the file.

I tested it and it worked for the test case of @silverwind.

But your solution seems ok, and perhap's seems cleaner to you.