diff: print the file header on GIT_DIFF_FORMAT_PATCH_HEADER#6888
Conversation
This was a regression leading up to 1.8.
In `diff_print_patch_file` we try to avoid printing the file if we're not sure that there will be some content. This works fine if we have any later functions that might get called as part of printing. But when given the format `GIT_DIFF_FORMAT_PATCH_HEADER`, this is the only function to get called. Add the binary and hunk functions to those to be called when given this option and have them exit after printing the header. This takes care of printing the header if we would be issuing a hunk, but we skip that part.
|
Makes sense. Thanks for fixing this. Two questions - have you tested this in rugged yet? I assume so, but thought I'd ask to avoid churn after merge. Are there any advantages / disadvantages over #6887 (other than stylistic)? This one seems a bit preferable but 🤷 |
|
Yes, I've tested this against rugged as well. I think this version is preferable as it would still only show the header when we would emit a hunk. It causes a bunch more useless function calls but in the grand scheme you probably wouldn't notice it. The other version would emit the file header in cases where we wouldn't if we had asked for the whole patch. |
|
Great, I could finally update libgit2 in rugged's mainline. This would be a great candidate for backporting to the stable branch as well, as the 1.8 release is blocked on this same bug. |
2 participants
diff: print the file header on GIT_DIFF_FORMAT_PATCH_HEADER
In
diff_print_patch_filewe try to avoid printing the file if we're not surethat there will be some content. This works fine if we have any later functions
that might get called as part of printing. But when given the format
GIT_DIFF_FORMAT_PATCH_HEADER, this is the only function to get called.Add the binary and hunk functions to those to be called when given this option
and have them exit after printing the header. This takes care of printing the
header if we would be issuing a hunk, but we skip that part.
This is an alternative to #6887 which is a bit more wasteful of function calls but it shouldn't regress the issue that was originally intended to fix when introducing this regression by only printing the header if we would have printed a chunk.
I'm not positive that the hunk and binary functions are the two that should get called but I think this should be enough.