Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix generation of the header of the patch #643

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Member

nulltoken commented Apr 25, 2012

Previously

diff --git (null)subdir.txt (null)subdir.txt
deleted file mode 100644
index e8ee89e..0000000
--- /dev/null
+++ (null)subdir.txt

With the proposed fix

diff --git a/subdir.txt b/subdir.txt
deleted file mode 100644
index e8ee89e..0000000
--- a/subdir.txt
+++ /dev/null

This looks like more git compliant

$ git diff 26a125e..735b6a2
diff --git a/subdir.txt b/subdir.txt
deleted file mode 100644
index e8ee89e..0000000
--- a/subdir.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-Is it a bird?
-Is it a plane?

This pull request fails (merged 477067dd into 19dd4e2).

Owner

vmg commented Apr 25, 2012

Looks good to me. Maybe @arrbee has something to say.

Member

arrbee commented Apr 25, 2012

Hmm, I'm not seeing this problem but I am seeing a different one. Let me see if I can track down what's going on.

Member

nulltoken commented Apr 25, 2012

@arrbee BTW, I'm not sure that the "normalizing" of the prefixes should be done where I hacked it. git_diff_list_alloc() might be a better place...

@arrbee arrbee and 1 other commented on an outdated diff Apr 25, 2012

src/diff_output.c
@@ -579,9 +585,8 @@ static int print_patch_file(void *data, git_diff_delta *delta, float progress)
if (git_buf_oom(pi->buf))
return -1;
- if (pi->print_cb(pi->cb_data, GIT_DIFF_LINE_FILE_HDR, pi->buf->ptr) < 0 ||
- delta->binary != 1)
- return -1;
+ if (delta->binary != 1)
+ return pi->print_cb(pi->cb_data, GIT_DIFF_LINE_FILE_HDR, pi->buf->ptr);
@arrbee

arrbee Apr 25, 2012

Member

You are correct that this logic is no longer right, but I think the first call to print_cb should be made even if delta->binary is true. I think this should read:

    int result;

    ...

    result = pi->print_cb(pi->cb_data, GIT_DIFF_LINE_FILE_HDR, pi->buf->ptr);
    if (result < 0)
        return result;

    if (delta->binary != 1)
        return 0;
@nulltoken

nulltoken Apr 25, 2012

Member

@arrbee Of course, you're right! Otherwise no header is being generated for binary files.

@nulltoken

nulltoken Apr 25, 2012

Member

@arrbee Fixed. I also took the liberty to replace the hardcoded a/ and b/ by DIFF_SRC_PREFIX_DEFAULT and DIFF_DST_PREFIX_DEFAULT. Does this fit you?

This pull request fails (merged 3ec5f9f into f50087c).

Member

arrbee commented Apr 25, 2012

Okay, this looks good to me.

Member

arrbee commented Apr 25, 2012

Merged as eb3d71a

@arrbee arrbee closed this Apr 25, 2012

@yorah yorah referenced this pull request in libgit2/libgit2sharp Apr 27, 2012

Closed

Diff API #136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment