Prefer git_patch_print to git_diff_print

This has the nice side effect of allowing binary changes
to be detected.

8 files changed, 174 insertions, 30 deletions

diff --git a/LibGit2Sharp.Tests/DiffTreeToTreeFixture.cs b/LibGit2Sharp.Tests/DiffTreeToTreeFixture.cs
index 5ced715e..c1b0bcce 100644
--- a/LibGit2Sharp.Tests/DiffTreeToTreeFixture.cs
+++ b/LibGit2Sharp.Tests/DiffTreeToTreeFixture.cs
@@ -73,6 +73,71 @@ namespace LibGit2Sharp.Tests
             }
         }
 
+        static void CreateBinaryFile(string path)
+        {
+            var content = new byte[] { 0x1, 0x0, 0x2, 0x0 };
+
+            using (var binfile = File.Create(path))
+            {
+                for (int i = 0; i < 1000; i++)
+                {
+                    binfile.Write(content, 0, content.Length);
+                }
+            }
+        }
+
+        [Fact]
+        public void CanDetectABinaryChange()
+        {
+            using (var repo = new Repository(CloneStandardTestRepo()))
+            {
+                const string filename = "binfile.foo";
+                var filepath = Path.Combine(repo.Info.WorkingDirectory, filename);
+
+                CreateBinaryFile(filepath);
+
+                repo.Index.Stage(filename);
+                var commit = repo.Commit("Add binary file", Constants.Signature, Constants.Signature);
+
+                File.AppendAllText(filepath, "abcdef");
+
+                var patch = repo.Diff.Compare<Patch>(commit.Tree, DiffTargets.WorkingDirectory, new[] { filename });
+                Assert.True(patch[filename].IsBinaryComparison);
+
+                repo.Index.Stage(filename);
+                var commit2 = repo.Commit("Update binary file", Constants.Signature, Constants.Signature);
+
+                var patch2 = repo.Diff.Compare<Patch>(commit.Tree, commit2.Tree, new[] { filename });
+                Assert.True(patch2[filename].IsBinaryComparison);
+            }
+        }
+
+        [Fact]
+        public void CanDetectABinaryDeletion()
+        {
+            using (var repo = new Repository(CloneStandardTestRepo()))
+            {
+                const string filename = "binfile.foo";
+                var filepath = Path.Combine(repo.Info.WorkingDirectory, filename);
+
+                CreateBinaryFile(filepath);
+
+                repo.Index.Stage(filename);
+                var commit = repo.Commit("Add binary file", Constants.Signature, Constants.Signature);
+
+                File.Delete(filepath);
+
+                var patch = repo.Diff.Compare<Patch>(commit.Tree, DiffTargets.WorkingDirectory, new [] {filename});
+                Assert.True(patch[filename].IsBinaryComparison);
+
+                repo.Index.Remove(filename);
+                var commit2 = repo.Commit("Delete binary file", Constants.Signature, Constants.Signature);
+
+                var patch2 = repo.Diff.Compare<Patch>(commit.Tree, commit2.Tree, new[] { filename });
+                Assert.True(patch2[filename].IsBinaryComparison);
+            }
+        }
+
         /*
          * $ git diff 9fd738e..HEAD -- "1" "2/"
          * diff --git a/1/branch_file.txt b/1/branch_file.txt
diff --git a/LibGit2Sharp/Core/GitDiff.cs b/LibGit2Sharp/Core/GitDiff.cs
index d0d9bdd9..33ebcce7 100644
--- a/LibGit2Sharp/Core/GitDiff.cs
+++ b/LibGit2Sharp/Core/GitDiff.cs
@@ -200,7 +200,7 @@ namespace LibGit2Sharp.Core
     }
 
     [Flags]
-    internal enum GitDiffFileFlags
+    internal enum GitDiffFlags
     {
         GIT_DIFF_FLAG_BINARY = (1 << 0),
         GIT_DIFF_FLAG_NOT_BINARY = (1 << 1),
@@ -213,17 +213,17 @@ namespace LibGit2Sharp.Core
         public GitOid Oid;
         public IntPtr Path;
         public Int64 Size;
-        public GitDiffFileFlags Flags;
-        public ushort Mode;
+        public GitDiffFlags Flags;
+        public UInt16 Mode;
     }
 
     [StructLayout(LayoutKind.Sequential)]
     internal class GitDiffDelta
     {
         public ChangeKind Status;
-        public uint Flags;
-        public ushort Similarity;
-        public ushort NumberOfFiles;
+        public GitDiffFlags Flags;
+        public UInt16 Similarity;
+        public UInt16 NumberOfFiles;
         public GitDiffFile OldFile;
         public GitDiffFile NewFile;
     }
@@ -237,7 +237,7 @@ namespace LibGit2Sharp.Core
         public int NewLines;
         public UIntPtr HeaderLen;
 
-        [MarshalAsAttribute(UnmanagedType.ByValArray, SizeConst = 128)]
+        [MarshalAs(UnmanagedType.ByValArray, SizeConst = 128)]
         public byte[] Header;
     }
 
diff --git a/LibGit2Sharp/Core/GitDiffExtensions.cs b/LibGit2Sharp/Core/GitDiffExtensions.cs
index ae2afaf9..a885901a 100644
--- a/LibGit2Sharp/Core/GitDiffExtensions.cs
+++ b/LibGit2Sharp/Core/GitDiffExtensions.cs
@@ -6,9 +6,7 @@ namespace LibGit2Sharp.Core
     {
         public static bool IsBinary(this GitDiffDelta delta)
         {
-            //TODO Fix the interop issue on amd64 and use GitDiffDelta.Binary
-            return delta.OldFile.Flags.HasFlag(GitDiffFileFlags.GIT_DIFF_FLAG_BINARY)
-                || delta.NewFile.Flags.HasFlag(GitDiffFileFlags.GIT_DIFF_FLAG_BINARY);
+            return delta.Flags.HasFlag(GitDiffFlags.GIT_DIFF_FLAG_BINARY);
         }
     }
 }
diff --git a/LibGit2Sharp/Core/Handles/PatchSafeHandle.cs b/LibGit2Sharp/Core/Handles/PatchSafeHandle.cs
new file mode 100644
index 00000000..97c0dc9b
--- /dev/null
+++ b/LibGit2Sharp/Core/Handles/PatchSafeHandle.cs
@@ -0,0 +1,11 @@
+namespace LibGit2Sharp.Core.Handles
+{
+    internal class PatchSafeHandle : SafeHandleBase
+    {
+        protected override bool ReleaseHandleImpl()
+        {
+            Proxy.git_patch_free(handle);
+            return true;
+        }
+    }
+}
diff --git a/LibGit2Sharp/Core/NativeMethods.cs b/LibGit2Sharp/Core/NativeMethods.cs
index 6f164176..6b1accb2 100644
--- a/LibGit2Sharp/Core/NativeMethods.cs
+++ b/LibGit2Sharp/Core/NativeMethods.cs
@@ -435,19 +435,19 @@ namespace LibGit2Sharp.Core
             GitDiffOptions options);
 
         internal delegate int git_diff_file_cb(
-            GitDiffDelta delta,
+            [In] GitDiffDelta delta,
             float progress,
             IntPtr payload);
 
         internal delegate int git_diff_hunk_cb(
-            GitDiffDelta delta,
-            GitDiffHunk hunk,
+            [In] GitDiffDelta delta,
+            [In] GitDiffHunk hunk,
             IntPtr payload);
 
         internal delegate int git_diff_line_cb(
-            GitDiffDelta delta,
-            GitDiffHunk hunk,
-            GitDiffLine line,
+            [In] GitDiffDelta delta,
+            [In] GitDiffHunk hunk,
+            [In] GitDiffLine line,
             IntPtr payload);
 
         [DllImport(libgit2)]
@@ -483,6 +483,12 @@ namespace LibGit2Sharp.Core
             GitDiffFindOptions options);
 
         [DllImport(libgit2)]
+        internal static extern UIntPtr git_diff_num_deltas(DiffSafeHandle diff);
+
+        [DllImport(libgit2)]
+        internal static extern IntPtr git_diff_get_delta(DiffSafeHandle diff, UIntPtr idx);
+
+        [DllImport(libgit2)]
         internal static extern int git_graph_ahead_behind(out UIntPtr ahead, out UIntPtr behind, RepositorySafeHandle repo, ref GitOid one, ref GitOid two);
 
         [DllImport(libgit2)]
@@ -671,6 +677,15 @@ namespace LibGit2Sharp.Core
         internal static extern GitObjectType git_object_type(GitObjectSafeHandle obj);
 
         [DllImport(libgit2)]
+        internal static extern int git_patch_from_diff(out PatchSafeHandle patch, DiffSafeHandle diff, UIntPtr idx);
+
+        [DllImport(libgit2)]
+        internal static extern int git_patch_print(PatchSafeHandle patch, git_diff_line_cb print_cb, IntPtr payload);
+
+        [DllImport(libgit2)]
+        internal static extern void git_patch_free(IntPtr patch);
+
+        [DllImport(libgit2)]
         internal static extern int git_push_new(out PushSafeHandle push, RemoteSafeHandle remote);
 
         /* Push network progress notification function */
diff --git a/LibGit2Sharp/Core/Proxy.cs b/LibGit2Sharp/Core/Proxy.cs
index 5ddb6349..f16982fd 100644
--- a/LibGit2Sharp/Core/Proxy.cs
+++ b/LibGit2Sharp/Core/Proxy.cs
@@ -13,6 +13,15 @@ namespace LibGit2Sharp.Core
 {
     internal class Proxy
     {
+        private static T MarshalAs<T>(IntPtr ptr)
+        {
+            if (ptr == IntPtr.Zero)
+            {
+                return default(T);
+            }
+            return (T)Marshal.PtrToStructure(ptr, typeof(T));
+        }
+
         #region giterr_
 
         public static void giterr_set_str(GitErrorCategory error_class, Exception exception)
@@ -727,6 +736,16 @@ namespace LibGit2Sharp.Core
             }
         }
 
+        public static int git_diff_num_deltas(DiffSafeHandle diff)
+        {
+            return (int)NativeMethods.git_diff_num_deltas(diff);
+        }
+
+        public static GitDiffDelta git_diff_get_delta(DiffSafeHandle diff, int idx)
+        {
+            return MarshalAs<GitDiffDelta>(NativeMethods.git_diff_get_delta(diff, (UIntPtr) idx));
+        }
+
         #endregion
 
         #region git_graph_
@@ -1184,6 +1203,35 @@ namespace LibGit2Sharp.Core
 
         #endregion
 
+        #region git_patch_
+
+        public static void git_patch_free(IntPtr patch)
+        {
+            NativeMethods.git_patch_free(patch);
+        }
+
+        public static PatchSafeHandle git_patch_from_diff(DiffSafeHandle diff, int idx)
+        {
+            using (ThreadAffinity())
+            {
+                PatchSafeHandle handle;
+                int res = NativeMethods.git_patch_from_diff(out handle, diff, (UIntPtr) idx);
+                Ensure.ZeroResult(res);
+                return handle;
+            }
+        }
+
+        public static void git_patch_print(PatchSafeHandle patch, NativeMethods.git_diff_line_cb printCallback)
+        {
+            using (ThreadAffinity())
+            {
+                int res = NativeMethods.git_patch_print(patch, printCallback, IntPtr.Zero);
+                Ensure.ZeroResult(res);
+            }
+        }
+
+        #endregion
+
         #region git_push_
 
         public static void git_push_add_refspec(PushSafeHandle push, string pushRefSpec)
diff --git a/LibGit2Sharp/LibGit2Sharp.csproj b/LibGit2Sharp/LibGit2Sharp.csproj
index bb3948ca..92d4513b 100644
--- a/LibGit2Sharp/LibGit2Sharp.csproj
+++ b/LibGit2Sharp/LibGit2Sharp.csproj
@@ -75,6 +75,7 @@
     <Compile Include="CommitFilter.cs" />
     <Compile Include="CommitSortStrategies.cs" />
     <Compile Include="CompareOptions.cs" />
+    <Compile Include="Core\Handles\PatchSafeHandle.cs" />
     <Compile Include="FetchOptions.cs" />
     <Compile Include="RefSpec.cs" />
     <Compile Include="RefSpecCollection.cs" />
diff --git a/LibGit2Sharp/Patch.cs b/LibGit2Sharp/Patch.cs
index 9f227275..0a22f6b9 100644
--- a/LibGit2Sharp/Patch.cs
+++ b/LibGit2Sharp/Patch.cs
@@ -32,28 +32,34 @@ namespace LibGit2Sharp
 
         internal Patch(DiffSafeHandle diff)
         {
-            Proxy.git_diff_foreach(diff, FileCallback, null, null);
-
-            Proxy.git_diff_print(diff, PrintCallBack);
-        }
+            int count = Proxy.git_diff_num_deltas(diff);
+            for (int i = 0; i < count; i++)
+            {
+                using (var patch = Proxy.git_patch_from_diff(diff, i))
+                {
+                    var delta = Proxy.git_diff_get_delta(diff, i);
+                    AddFileChange(delta);
+                    Proxy.git_patch_print(patch, PrintCallBack);
+                }
 
-        private int FileCallback(GitDiffDelta delta, float progress, IntPtr payload)
-        {
-            AddFileChange(delta);
-            return 0;
+            }
         }
 
         private void AddFileChange(GitDiffDelta delta)
         {
-            var newFilePath = LaxFilePathMarshaler.FromNative(delta.NewFile.Path);
-
+            var pathPtr = delta.NewFile.Path != IntPtr.Zero ? delta.NewFile.Path : delta.OldFile.Path;
+            var newFilePath = LaxFilePathMarshaler.FromNative(pathPtr);
             changes.Add(newFilePath, new ContentChanges(delta.IsBinary()));
         }
 
         private int PrintCallBack(GitDiffDelta delta, GitDiffHunk hunk, GitDiffLine line, IntPtr payload)
         {
             string patchPart = LaxUtf8Marshaler.FromNative(line.content, (int)line.contentLen);
-            var filePath = LaxFilePathMarshaler.FromNative(delta.NewFile.Path);
+
+            // Deleted files mean no "new file" path
+
+            var pathPtr = delta.NewFile.Path != IntPtr.Zero ? delta.NewFile.Path : delta.OldFile.Path;
+            var filePath = LaxFilePathMarshaler.FromNative(pathPtr);
 
             ContentChanges currentChange = this[filePath];
             string prefix = string.Empty;
@@ -77,10 +83,10 @@ namespace LibGit2Sharp
                     break;
             }
 
-            string formatedOutput = string.Concat(prefix, patchPart);
+            string formattedOutput = string.Concat(prefix, patchPart);
 
-            fullPatchBuilder.Append(formatedOutput);
-            this[filePath].AppendToPatch(formatedOutput);
+            fullPatchBuilder.Append(formattedOutput);
+            currentChange.AppendToPatch(formattedOutput);
 
             return 0;
         }