Report conflicted binary files and modify/delete conflicts as unmerged in Patch()

GitIntegration.Test/Fixtures/patch-unmerged-binary-and-deleted.txt

@@ -0,0 +1,4 @@
+diff --cc bin.dat
+index 1323b0a,012c31b..0000000
+Binary files differ
+* Unmerged path d.txt

GitIntegration.Test/Integration/GitPatchRoundTripTests.cs

@@ -20,6 +20,7 @@
 {
 	private static readonly GitAuthorName AuthorName = "Fixture Author".As<GitAuthorName>();
 	private static readonly GitAuthorEmail AuthorEmail = "[email protected]".As<GitAuthorEmail>();
+	private static readonly string[] ExpectedUnmergedPaths = ["bin.dat", "d.txt"];
 
 	[TestMethod]
 	public async Task StagingOneHunkLeavesTheOtherUnstagedAsync()
@@ -377,6 +378,70 @@
 		return init.Repository;
 	}
 
+	[TestMethod]
+	public async Task PatchAndDiffAgreeOnUnmergedPathsAfterABinaryAndAModifyDeleteConflictAsync()
+	{
+		await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false);
+
+		using TemporaryRepository repository = new();
+		GitClient client = IntegrationGitFixture.CreateClient();
+		GitRepository seeded = await SeedAsync(client, repository, []).ConfigureAwait(false);
+
+		// Both sides change bin.dat, which git cannot merge as text, and one side deletes d.txt
+		// while the other modifies it. Neither conflict produces a @@@ hunk.
+		repository.WriteFile("bin.dat", "\0\u0001base");
+		repository.WriteFile("d.txt", "1\n2\n3\n");
+		await CommitAllAsync(seeded).ConfigureAwait(false);
+
+		await RunGitAsync(seeded, "checkout", "-q", "-b", "other").ConfigureAwait(false);
+		repository.WriteFile("bin.dat", "\0\u0001other");
+		repository.WriteFile("d.txt", "1\n2\n3\n4\n");
+		await CommitAllAsync(seeded).ConfigureAwait(false);
+
+		await RunGitAsync(seeded, "checkout", "-q", "main").ConfigureAwait(false);
+		repository.WriteFile("bin.dat", "\0\u0001main");
+		repository.DeleteFile("d.txt");
+		await CommitAllAsync(seeded).ConfigureAwait(false);
+
+		// merge is out of scope for this library, so the fixture runs it directly. It is expected
+		// to fail, leaving both paths unmerged.
+		GitProcessResult merged = await seeded.ProcessRunner!.RunAsync(
+			new GitProcessRequest { Arguments = ["-C", repository.RootPath, "merge", "--no-edit", "other"] },
+			TestContext.CancellationTokenSource.Token).ConfigureAwait(false);
+
+		Assert.IsFalse(merged.Success, "the merge was expected to conflict but succeeded");
+
+		GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false);
+
+		IReadOnlyList<GitDiffEntry> diff = await opened.Diff().ExecuteAsync().ConfigureAwait(false);
+		GitPatch patch = await opened.Patch().ExecuteAsync().ConfigureAwait(false);
+
+		string[] diffUnmerged = [.. diff
+			.Where(entry => entry.Kind == GitChangeKind.Unmerged)
+			.Select(entry => entry.Path.WeakString)
+			.Order(StringComparer.Ordinal)];
+		string[] patchUnmerged = [.. patch.Files
+			.Where(file => file.Kind == GitChangeKind.Unmerged && file.IsConflicted)
+			.Select(file => file.Path.WeakString)
+			.Order(StringComparer.Ordinal)];
+
+		CollectionAssert.AreEqual(ExpectedUnmergedPaths, diffUnmerged, "the fixture must leave both conflicts");
+		CollectionAssert.AreEqual(diffUnmerged, patchUnmerged);
+		Assert.IsTrue(patch.Files.All(file => file.Hunks.Count == 0));
+	}
+
+	/// <summary>Runs a git command this library has no verb for, failing the test if git fails.</summary>
+	/// <param name="repository">The repository to run in.</param>
+	/// <param name="arguments">The git arguments, after <c>-C &lt;root&gt;</c>.</param>
+	private async Task RunGitAsync(GitRepository repository, params string[] arguments)
+	{
+		GitProcessResult result = await repository.ProcessRunner!.RunAsync(
+			new GitProcessRequest { Arguments = ["-C", repository.LocalPath!.WeakString, .. arguments] },
+			TestContext.CancellationTokenSource.Token).ConfigureAwait(false);
+
+		Assert.IsTrue(result.Success, $"git {string.Join(' ', arguments)} failed");
+	}
+
 	/// <summary>Stages everything in the working tree and commits it.</summary>
 	/// <param name="repository">The repository to commit in.</param>
 	private async Task CommitAllAsync(GitRepository repository)

GitIntegration.Test/Parsing/GitPatchParserTests.cs

@@ -206,6 +206,40 @@
 		Assert.AreEqual(0, file.Hunks.Count);
 	}
 
+	[TestMethod]
+	public void FlagsAConflictedBinaryFileAndAModifyDeleteConflict()
+	{
+		// Captured with git 2.43.0 on Linux after a merge left bin.dat changed on both sides (UU)
+		// and d.txt deleted on one side and modified on the other (DU). Neither has a @@@ line.
+		GitPatch patch = GitPatchParser.Parse(Fixture("patch-unmerged-binary-and-deleted.txt"));
+
+		Assert.AreEqual(2, patch.Files.Count);
+
+		GitFilePatch binary = patch.Files[0];
+		Assert.AreEqual("bin.dat", binary.Path.WeakString);
+		Assert.AreEqual(GitChangeKind.Unmerged, binary.Kind);
+		Assert.IsTrue(binary.IsConflicted);
+		Assert.AreEqual(0, binary.Hunks.Count);
+
+		GitFilePatch deleted = patch.Files[1];
+		Assert.AreEqual("d.txt", deleted.Path.WeakString);
+		Assert.AreEqual(GitChangeKind.Unmerged, deleted.Kind);
+		Assert.IsTrue(deleted.IsConflicted);
+		Assert.AreEqual(0, deleted.Hunks.Count);
+	}
+
+	[TestMethod]
+	public void FlagsALongFormCombinedHeaderAsConflicted()
+	{
+		const string output = "diff --combined f.txt\nindex 305b879,1ebef9c..0000000\nBinary files differ\n";
+
+		GitFilePatch file = GitPatchParser.Parse(output).Files.Single();
+
+		Assert.AreEqual("f.txt", file.Path.WeakString);
+		Assert.AreEqual(GitChangeKind.Unmerged, file.Kind);
+		Assert.IsTrue(file.IsConflicted);
+	}
+
 	[TestMethod]
 	public void ParsesCarriageReturnContentWithoutStrippingIt()
 	{

GitIntegration/Parsing/GitPatchParser.cs

@@ -22,6 +22,8 @@
 {
 	private const string GitHeaderPrefix = "diff --git ";
 	private const string CombinedHeaderPrefix = "diff --cc ";
+	private const string LongCombinedHeaderPrefix = "diff --combined ";
+	private const string UnmergedPathPrefix = "* Unmerged path ";
 	private const string BSidePathMarker = " b/";
 	private const string RenameFromPrefix = "rename from ";
 	private const string RenameToPrefix = "rename to ";
@@ -36,7 +38,10 @@
 	/// Parses the text a diff-producing command wrote to standard output.
 	/// </summary>
 	/// <param name="output">Everything git wrote to standard output.</param>
-	/// <returns>The patch, with one file per <c>diff --git</c> or <c>diff --cc</c> block found.</returns>
+	/// <returns>
+	/// The patch, with one file per <c>diff --git</c>, <c>diff --cc</c> or <c>diff --combined</c>
+	/// block found, and one per <c>* Unmerged path</c> line.
+	/// </returns>
 	/// <exception cref="GitParseException">A header or a hunk was malformed.</exception>
 	public static GitPatch Parse(string output)
 	{
@@ -48,7 +53,19 @@
 		int index = 0;
 		while (index < lines.Count)
 		{
-			if (!IsFileStart(Line(output, lines, index)))
+			string line = Line(output, lines, index);
+
+			if (line.StartsWith(UnmergedPathPrefix, StringComparison.Ordinal))
+			{
+				// A modify/delete conflict has no content to diff, so git names the path on this
+				// one line instead of starting a file block for it. Diff() reports the same path
+				// as Unmerged, and leaving it out here would hide the conflict entirely.
+				files.Add(UnmergedPath(output, lines, index, line));
+				index++;
+				continue;
+			}
+
+			if (!IsFileStart(line))
 			{
 				index++;
 				continue;
@@ -105,9 +122,25 @@
 
 	private static bool IsFileStart(string line) =>
 		line.StartsWith(GitHeaderPrefix, StringComparison.Ordinal) ||
-		line.StartsWith(CombinedHeaderPrefix, StringComparison.Ordinal);
+		IsCombinedStart(line);
+
+	private static bool IsCombinedStart(string line) =>
+		line.StartsWith(CombinedHeaderPrefix, StringComparison.Ordinal) ||
+		line.StartsWith(LongCombinedHeaderPrefix, StringComparison.Ordinal);
+
+	private static GitFilePatch UnmergedPath(string output, List<(int Start, int End)> lines, int index, string line) =>
+		new()
+		{
+			Path = GitParseValues.ToRelativeFilePath(UnquotePath(line[UnmergedPathPrefix.Length..], line)),
+			OriginalPath = null,
+			Kind = GitChangeKind.Unmerged,
+			IsBinary = false,
+			IsConflicted = true,
+			Header = output[lines[index].Start..RegionEnd(output, lines, index + 1)],
+			Hunks = [],
+		};
 
 	private static GitFilePatch ParseFile(string output, List<(int Start, int End)> lines, ref int index)
 	{
 		int fileStart = lines[index].Start;
 		string firstLine = Line(output, lines, index);
@@ -116,7 +149,11 @@
 		RelativeFilePath? originalPath = null;
 		GitChangeKind kind = GitChangeKind.Modified;
 		bool isBinary = false;
-		bool isConflicted = false;
+
+		// Git writes the combined format only for an unmerged path, and only some of those carry
+		// a @@@ line: a conflicted binary file stops at "Binary files differ". The header alone
+		// decides, so every such path reports the Unmerged that GitDiffParser gives it.
+		bool isConflicted = IsCombinedStart(firstLine);
 
 		index++;
 
@@ -125,6 +162,7 @@
 			string line = Line(output, lines, index);
 
 			if (IsFileStart(line) ||
+				line.StartsWith(UnmergedPathPrefix, StringComparison.Ordinal) ||
 				line.StartsWith(ConflictHunkPrefix, StringComparison.Ordinal) ||
 				line.StartsWith(HunkPrefix, StringComparison.Ordinal))
 			{
@@ -135,24 +173,28 @@
 			index++;
 		}
 
+		// Set after the header lines, so that a mode line in a combined header cannot report an
+		// unmerged path as Added or Deleted.
+		if (isConflicted)
+		{
+			kind = GitChangeKind.Unmerged;
+		}
+
 		string header = output[fileStart..RegionEnd(output, lines, index)];
 		List<GitHunk> hunks = [];
 
 		if (index < lines.Count)
 		{
 			string boundary = Line(output, lines, index);
 
-			if (boundary.StartsWith(ConflictHunkPrefix, StringComparison.Ordinal))
+			if (isConflicted)
 			{
 				// Combined format from an unmerged path is not a patch git apply accepts, so its
 				// body is skipped rather than misread as ordinary hunks. Kind follows the same
 				// enum member GitDiffParser reports for the path, so a caller switching on
 				// GitChangeKind gets one answer from both verbs.
-				isConflicted = true;
-				kind = GitChangeKind.Unmerged;
-				index++;
-
-				while (index < lines.Count && !IsFileStart(Line(output, lines, index)))
+				while (index < lines.Count && !IsFileStart(Line(output, lines, index)) &&
+					!Line(output, lines, index).StartsWith(UnmergedPathPrefix, StringComparison.Ordinal))
 				{
 					index++;
 				}
@@ -202,6 +244,11 @@
 			return UnquotePath(line[CombinedHeaderPrefix.Length..], line);
 		}
 
+		if (line.StartsWith(LongCombinedHeaderPrefix, StringComparison.Ordinal))
+		{
+			return UnquotePath(line[LongCombinedHeaderPrefix.Length..], line);
+		}
+
 		string remainder = line[GitHeaderPrefix.Length..];
 
 		// A path git had to C-quote puts the whole operand, prefix included, inside the quotes.
@@ -258,7 +305,7 @@
 		{
 			if (text[position] == '\\')
 			{
 				position++;
 			}
 			else if (text[position] == '"')
 			{
@@ -278,7 +325,7 @@
 	/// <param name="line">The header line, for the error message.</param>
 	/// <returns>The path as it is named on disk.</returns>
 	/// <exception cref="GitParseException">The quoting is malformed.</exception>
 	internal static string UnquotePath(string value, string line)
 	{
 		if (!value.StartsWith('"'))
 		{
@@ -374,7 +421,7 @@
 		}
 	}
 
 	private static GitHunk ParseHunk(string output, List<(int Start, int End)> lines, ref int index)
 	{
 		int hunkStart = lines[index].Start;
 		(int oldStart, int oldCount, int newStart, int newCount, string heading) =