Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

diff: print the file header on GIT_DIFF_FORMAT_PATCH_HEADER#6888

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ethomson merged 2 commits intomainfromcmn/print-header-alternative
Sep 25, 2024

Conversation

carlosmn
Copy link
Member

diff: print the file header on GIT_DIFF_FORMAT_PATCH_HEADER

Indiff_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.

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.

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 surethat there will be some content. This works fine if we have any later functionsthat 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 optionand have them exit after printing the header. This takes care of printing theheader if we would be issuing a hunk, but we skip that part.
@ethomson
Copy link
Member

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 🤷

@carlosmn
Copy link
MemberAuthor

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.

ethomson reacted with thumbs up emoji

@ethomsonethomson merged commit15668ba intomainSep 25, 2024
19 checks passed
@carlosmncarlosmn deleted the cmn/print-header-alternative branchSeptember 25, 2024 07:39
@carlosmn
Copy link
MemberAuthor

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.

ethomson reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@carlosmn@ethomson

[8]ページ先頭

©2009-2025 Movatter.jp