Fix parsing long lines in diffs #967
Conversation
| if err != nil { | ||
| return "", err | ||
| } | ||
| l = append(l, line...) |
There was a problem hiding this comment.
We don't need this line because the godoc says that readline consumes remaining content even if the line size reaches size limit.
// readline reads lines from bufio.Reader with size limit. It consumes // remaining content even if the line size reaches size limit. func readline(r *bufio.Reader) (string, error) {
There was a problem hiding this comment.
So it is intentional to not parse the entire diff if it has long lines?
There was a problem hiding this comment.
Would be great to figure this out, as I am happy to modify the comment if this enhancement is going to be accepted
There was a problem hiding this comment.
@haya14busa How do you think about this?
There was a problem hiding this comment.
Sry for the super later reply.
Originally, it's not important for reviewdog to read whole lines in diff because reviewdog only wants to know whether lines are added/changed/deleted to filter diagnostics.
However, we introduced diff format with #699 and reading whole lines in diff is important for suggestion feature.
I'm not sure whether it's realistic to show super-large lines as code suggestions, but we should accept this p-r.
|
Hi, @Dhertz! We merged your PR to reviewdog! 🐶 We just invited you to join the @reviewdog organization on GitHub. Thanks again! |
Fixes issue if there is a line longer than 4096 characters in a diff. Currently Reviewdog returns 4096 characters over multiple lines, this changes resolves the issue to return just a single (entire) line correctly.