search

sourcegraph / go-diff

Unified diff parser and printer for Go
https://sourcegraph.com/github.com/sourcegraph/go-diff
Other
426 stars 48 forks source link

Detect file header, when parsing Hunk #53

Closed sofiia-tesliuk closed 4 years ago

sofiia-tesliuk commented 4 years ago

The version before was failing to identify next file header (without extended lines before it), while was parsing Hunk. Hunk reader identify line as a part of the hunk, if it starts with `,+or-. Because of this, file header starting with--- `, was also identified as part of the hunk.

keegancsmith commented 4 years ago

Thanks!

mrnugget commented 4 years ago

Heads up: I think this commit broke some behaviour when parsing hunk lines that do contain -- at the beginning.

Example diff:

diff --git a/foobar.sql b/foobar.sql
index 55140ba..236cd59 100644
--- a/foobar.sql
+++ b/foobar.sql
@@ -1,4 +1,3 @@
 select 1;
--- this is my query
 select 2;
 select 3;

In SQL -- starts a comment and this patch deletes a comment.

If I try to parse this with a new MultiFileDiffReader I run into a nil-panic, because we don't handle an error correctly. But the underlying cause for the error is the addition here.

I'm currently investigating to see whether we can make that detect-file-header check more robust (maybe we need to read ahead and check whether the next line starts with +++).