-
Notifications
You must be signed in to change notification settings - Fork 49
Add basic support for binary diffs #46
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #46 +/- ##
==========================================
+ Coverage 28.12% 28.31% +0.18%
==========================================
Files 5 5
Lines 1152 1155 +3
==========================================
+ Hits 324 327 +3
+ Misses 784 783 -1
- Partials 44 45 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. What happened previously in this code when diffing binary files? Do you need any special settings for git for this to work?
diff/parse.go
Outdated
@@ -341,7 +341,7 @@ func (r *FileDiffReader) ReadExtendedHeaders() ([]string, error) { | |||
func handleEmpty(fd *FileDiff) (wasEmpty bool) { | |||
var err error | |||
switch { | |||
case (len(fd.Extended) == 3 || len(fd.Extended) == 4 && strings.HasPrefix(fd.Extended[3], "Binary files ")) && | |||
case (len(fd.Extended) == 3 || len(fd.Extended) == 4 && strings.HasPrefix(fd.Extended[3], "Binary files ") || len(fd.Extended) > 4 && strings.HasPrefix(fd.Extended[3], "GIT binary patch")) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like this conditional is big enough and repeated enough to be factored out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made it smaller and replaced the common len check by a variable
Previously the file paths would be empty strings, now they are properly populated |
This adds support for parsing the cases in ./diff/testdata/sample_binary_inline.diff correctly.