Fix diff skipping lines #13154

zeripath · 2020-10-15T05:34:18Z

zeripath commented

2020-10-15 05:34:18 +00:00

ParsePatch previously just skipped all lines that start with "+++ " or "--- "
and makes no attempt to see these lines in context.

This PR rewrites ParsePatch to pay attention to context and position
within a patch, ensuring that --- and +++ are only skipped if
appropriate.

This PR also fixes several issues with incomplete files.

Fix https://codeberg.org/Codeberg/Community/issues/308
Fix #13153

Signed-off-by: Andrew Thornton art27@cantab.net

ParsePatch previously just skipped all lines that start with "+++ " or "--- " and makes no attempt to see these lines in context. This PR rewrites ParsePatch to pay attention to context and position within a patch, ensuring that --- and +++ are only skipped if appropriate. This PR also fixes several issues with incomplete files. Fix https://codeberg.org/Codeberg/Community/issues/308 Fix #13153 Signed-off-by: Andrew Thornton <art27@cantab.net>

👍 1 👀 1

techknowlogick reviewed 2020-10-15 05:52:02 +00:00

techknowlogick left a comment

First, thank you so much for this PR! I've been afraid of our diff parsing, and so I'm especially appreciative that you took this on. Second, would you hate me if I asked for tests, specifically ones for diffs with files that have lines that start with ++ and --?

First, thank you so much for this PR! I've been afraid of our diff parsing, and so I'm especially appreciative that you took this on. Second, would you hate me if I asked for tests, specifically ones for diffs with files that have lines that start with `++` and `--`?

zeripath reviewed 2020-10-15 06:06:08 +00:00

services/gitdiff/gitdiff.go

zeripath commented

2020-10-15 06:06:07 +00:00

	// - we might want to consider detecting encoding while parsing but...

```suggestion // - we might want to consider detecting encoding while parsing but... ```

techknowlogick reviewed 2020-10-15 06:22:02 +00:00

services/gitdiff/gitdiff.go

techknowlogick commented

2020-10-15 06:22:02 +00:00

I've attempted to trace through this loop to search for an err that is previously unhandled such that this conditional would be needed. Although it is an unreasonable hour for me to be awake, let alone doing PR reviews, so I am likely missing something, and should return in the morning with fresh eyes.