From 0afbe782ed7622e9575c89c97515ce33d19980fc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 03:33:26 +0000 Subject: [PATCH 1/2] Write a partial rename patch as a plain change to the new path GitFilePatch.PatchFor always wrote the file's full header, so for a renamed file it carried the rename from / rename to lines under whatever subset of hunks the caller picked. Reverse-applying one hunk of a staged rename to the index then undid the rename as well: b left the index and the other hunk moved onto a. A subset of a renamed or copied file's hunks is now written under a plain modification header on the new path, and the full header is kept only when every hunk is selected. Fixes ktsu-dev/GitIntegration#126 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01JgoVAth3nGZUgbKrSgVCYX --- .../Integration/GitPatchRoundTripTests.cs | 46 +++++++++++ GitIntegration.Test/Models/GitPatchTests.cs | 43 +++++++++++ GitIntegration/Models/GitPatch.cs | 76 ++++++++++++++++++- 3 files changed, 164 insertions(+), 1 deletion(-) diff --git a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs index 466fa71..81017cb 100644 --- a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs @@ -377,6 +377,52 @@ await IntegrationGitFixture.ConfigureIdentityAsync( return init.Repository; } + [TestMethod] + public async Task UnstagingOneHunkOfAStagedRenameKeepsTheRenameAsync() + { + await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + using TemporaryRepository repository = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + GitRepository seeded = await SeedAsync(client, repository, []).ConfigureAwait(false); + + string[] lines = [.. Enumerable.Range(1, 30).Select(number => $"{number}")]; + repository.WriteFile("a", string.Join('\n', lines) + "\n"); + await CommitAllAsync(seeded).ConfigureAwait(false); + + // Rename a to b, change lines 2 and 28 of b, and stage all of it. + repository.DeleteFile("a"); + lines[1] = "2 changed"; + lines[27] = "28 changed"; + repository.WriteFile("b", string.Join('\n', lines) + "\n"); + _ = await seeded.Add().All() + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); + + GitFilePatch file = (await opened.Patch().Staged().DetectRenames().ExecuteAsync().ConfigureAwait(false)).Files.Single(); + + Assert.AreEqual("a", file.OriginalPath?.WeakString, "The fixture must stage a rename, or this proves nothing."); + Assert.AreEqual(2, file.Hunks.Count, "The fixture must produce two hunks, or this proves nothing."); + + _ = await opened.Apply(file.PatchFor([file.Hunks[0]])).ToIndex().Reversed().ExecuteAsync().ConfigureAwait(false); + + IReadOnlyList staged = await opened.Diff().Staged().DetectRenames().WithLineCounts() + .ExecuteAsync().ConfigureAwait(false); + IReadOnlyList unstaged = await opened.Diff().WithLineCounts() + .ExecuteAsync().ConfigureAwait(false); + + GitDiffEntry rename = staged.Single(); + Assert.AreEqual(GitChangeKind.Renamed, rename.Kind); + Assert.AreEqual("b", rename.Path.WeakString); + Assert.AreEqual("a", rename.OriginalPath?.WeakString); + Assert.AreEqual(1, rename.Insertions, "Only the line-28 change stays staged."); + + GitDiffEntry pending = unstaged.Single(); + Assert.AreEqual("b", pending.Path.WeakString); + Assert.AreEqual(1, pending.Insertions, "Only the line-2 change was unstaged."); + } + /// Stages everything in the working tree and commits it. /// The repository to commit in. private async Task CommitAllAsync(GitRepository repository) diff --git a/GitIntegration.Test/Models/GitPatchTests.cs b/GitIntegration.Test/Models/GitPatchTests.cs index 8b11c81..a768e92 100644 --- a/GitIntegration.Test/Models/GitPatchTests.cs +++ b/GitIntegration.Test/Models/GitPatchTests.cs @@ -46,6 +46,49 @@ public class GitPatchTests Hunks = [HunkOne, HunkTwo], }; + private const string RenameHeader = + "diff --git a/a.txt b/b.txt\nsimilarity index 93%\nrename from a.txt\nrename to b.txt\n" + + "index 92dfa21..81f8caa 100644\n--- a/a.txt\n+++ b/b.txt\n"; + + private static GitFilePatch RenamedFileWithTwoHunks => FileWithTwoHunks with + { + Path = "b.txt".As(), + OriginalPath = "a.txt".As(), + Header = RenameHeader, + }; + + [TestMethod] + public void PatchForSomeHunksOfARenameWritesAPlainChangeToTheNewPath() + { + string patch = RenamedFileWithTwoHunks.PatchFor([HunkOne]); + + Assert.AreEqual( + "diff --git a/b.txt b/b.txt\nindex 92dfa21..81f8caa 100644\n--- a/b.txt\n+++ b/b.txt\n" + HunkOne.Text, + patch); + } + + [TestMethod] + public void PatchForEveryHunkOfARenameKeepsTheRenameHeader() => + Assert.AreEqual( + RenameHeader + HunkOne.Text + HunkTwo.Text, + RenamedFileWithTwoHunks.PatchFor([HunkOne, HunkTwo])); + + [TestMethod] + public void PatchForSomeHunksOfAQuotedRenameKeepsTheQuoting() + { + GitFilePatch file = RenamedFileWithTwoHunks with + { + Path = "b \"q\".txt".As(), + Header = "diff --git a/a.txt \"b/b \\\"q\\\".txt\"\nsimilarity index 93%\nrename from a.txt\n" + + "rename to \"b \\\"q\\\".txt\"\nindex 92dfa21..81f8caa 100644\n--- a/a.txt\n+++ \"b/b \\\"q\\\".txt\"\n", + }; + + Assert.AreEqual( + "diff --git \"a/b \\\"q\\\".txt\" \"b/b \\\"q\\\".txt\"\nindex 92dfa21..81f8caa 100644\n" + + "--- \"a/b \\\"q\\\".txt\"\n+++ \"b/b \\\"q\\\".txt\"\n" + HunkTwo.Text, + file.PatchFor([HunkTwo])); + } + [TestMethod] public void PatchForOneHunkCarriesTheHeaderAndThatHunkOnly() { diff --git a/GitIntegration/Models/GitPatch.cs b/GitIntegration/Models/GitPatch.cs index 8e45513..ab0a6d2 100644 --- a/GitIntegration/Models/GitPatch.cs +++ b/GitIntegration/Models/GitPatch.cs @@ -138,6 +138,11 @@ public sealed record GitFilePatch /// another file would otherwise get a shorter patch than it asked for, or a header with no body /// at all, and neither outcome says which hunk went missing. /// + /// + /// For a renamed or copied file, the full header, rename lines included, is kept only when + /// every hunk is given. A subset is written as a plain modification of , so + /// that reverse-applying it to the index unstages only those hunks and leaves the rename staged. + /// /// /// The hunks to include. /// The patch text. @@ -159,7 +164,12 @@ public string PatchFor(IEnumerable hunks) nameof(hunks)); } - StringBuilder builder = new(Header); + // A renamed or copied file's header moves the path as well as carrying the hunks. Writing + // it under only some of the hunks would apply, or with --reverse undo, the whole rename + // alongside the chosen lines, so a subset goes out as a plain change to the new path. + bool partialRename = OriginalPath is not null && Hunks.Any(hunk => !wanted.Contains(hunk)); + + StringBuilder builder = new(partialRename ? ModificationHeaderForNewPath() : Header); int emitted = 0; foreach (GitHunk hunk in Hunks.Where(wanted.Contains)) @@ -176,6 +186,70 @@ public string PatchFor(IEnumerable hunks) nameof(hunks)) : builder.ToString(); } + + /// + /// Rewrites as a plain modification of the new path: both sides of the + /// diff --git, --- and +++ lines name it, and the similarity, rename and + /// copy lines are dropped. Every other line, the index and mode lines included, is kept. + /// + /// The rewritten header. + private string ModificationHeaderForNewPath() + { + const string NewSidePrefix = "+++ "; + const string OldSidePrefix = "--- "; + + string[] lines = Header.Split('\n'); + string newSide = lines.FirstOrDefault(line => line.StartsWith(NewSidePrefix, StringComparison.Ordinal))?[NewSidePrefix.Length..] + ?? throw new InvalidOperationException( + $"The header for '{Path.WeakString}' has no '+++' line, so its hunks cannot be written under the new path alone."); + + // git C-quotes the whole operand, prefix included, so the prefix sits after the quote. + string oldSide = newSide.StartsWith("\"b/", StringComparison.Ordinal) + ? "\"a/" + newSide[3..] + : newSide.StartsWith("b/", StringComparison.Ordinal) + ? "a/" + newSide[2..] + : throw new InvalidOperationException( + $"The header for '{Path.WeakString}' has a '+++' line with no b/ prefix: '{newSide}'."); + + StringBuilder builder = new(); + + for (int index = 0; index < lines.Length; index++) + { + string line = lines[index]; + + if (index == lines.Length - 1) + { + // The text after the header's final newline, which is empty for a well-formed header. + _ = builder.Append(line); + continue; + } + + if (index == 0) + { + line = $"diff --git {oldSide} {newSide}"; + } + else if (line.StartsWith(OldSidePrefix, StringComparison.Ordinal)) + { + line = OldSidePrefix + oldSide; + } + else if (IsRenameOrCopyLine(line)) + { + continue; + } + + _ = builder.Append(line).Append('\n'); + } + + return builder.ToString(); + } + + private static bool IsRenameOrCopyLine(string line) => + line.StartsWith("similarity index ", StringComparison.Ordinal) || + line.StartsWith("dissimilarity index ", StringComparison.Ordinal) || + line.StartsWith("rename from ", StringComparison.Ordinal) || + line.StartsWith("rename to ", StringComparison.Ordinal) || + line.StartsWith("copy from ", StringComparison.Ordinal) || + line.StartsWith("copy to ", StringComparison.Ordinal); } /// From 5fd2610dea188fe21e654f9d688bc5d19ebee240 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 03:34:20 +0000 Subject: [PATCH 2/2] Move the partial-rename round-trip test away from the end of the class Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01JgoVAth3nGZUgbKrSgVCYX --- .../Integration/GitPatchRoundTripTests.cs | 92 +++++++++---------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs index 81017cb..0aa937a 100644 --- a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs @@ -66,6 +66,52 @@ await IntegrationGitFixture.ConfigureIdentityAsync( Assert.AreEqual(1, unstaged.Single().Insertions); } + [TestMethod] + public async Task UnstagingOneHunkOfAStagedRenameKeepsTheRenameAsync() + { + await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + using TemporaryRepository repository = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + GitRepository seeded = await SeedAsync(client, repository, []).ConfigureAwait(false); + + string[] lines = [.. Enumerable.Range(1, 30).Select(number => $"{number}")]; + repository.WriteFile("a", string.Join('\n', lines) + "\n"); + await CommitAllAsync(seeded).ConfigureAwait(false); + + // Rename a to b, change lines 2 and 28 of b, and stage all of it. + repository.DeleteFile("a"); + lines[1] = "2 changed"; + lines[27] = "28 changed"; + repository.WriteFile("b", string.Join('\n', lines) + "\n"); + _ = await seeded.Add().All() + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); + + GitFilePatch file = (await opened.Patch().Staged().DetectRenames().ExecuteAsync().ConfigureAwait(false)).Files.Single(); + + Assert.AreEqual("a", file.OriginalPath?.WeakString, "The fixture must stage a rename, or this proves nothing."); + Assert.AreEqual(2, file.Hunks.Count, "The fixture must produce two hunks, or this proves nothing."); + + _ = await opened.Apply(file.PatchFor([file.Hunks[0]])).ToIndex().Reversed().ExecuteAsync().ConfigureAwait(false); + + IReadOnlyList staged = await opened.Diff().Staged().DetectRenames().WithLineCounts() + .ExecuteAsync().ConfigureAwait(false); + IReadOnlyList unstaged = await opened.Diff().WithLineCounts() + .ExecuteAsync().ConfigureAwait(false); + + GitDiffEntry rename = staged.Single(); + Assert.AreEqual(GitChangeKind.Renamed, rename.Kind); + Assert.AreEqual("b", rename.Path.WeakString); + Assert.AreEqual("a", rename.OriginalPath?.WeakString); + Assert.AreEqual(1, rename.Insertions, "Only the line-28 change stays staged."); + + GitDiffEntry pending = unstaged.Single(); + Assert.AreEqual("b", pending.Path.WeakString); + Assert.AreEqual(1, pending.Insertions, "Only the line-2 change was unstaged."); + } + [TestMethod] public async Task CheckedReportsFailureWhenTheWorkingTreeMovedAsync() { @@ -377,52 +423,6 @@ await IntegrationGitFixture.ConfigureIdentityAsync( return init.Repository; } - [TestMethod] - public async Task UnstagingOneHunkOfAStagedRenameKeepsTheRenameAsync() - { - await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); - - using TemporaryRepository repository = new(); - GitClient client = IntegrationGitFixture.CreateClient(); - GitRepository seeded = await SeedAsync(client, repository, []).ConfigureAwait(false); - - string[] lines = [.. Enumerable.Range(1, 30).Select(number => $"{number}")]; - repository.WriteFile("a", string.Join('\n', lines) + "\n"); - await CommitAllAsync(seeded).ConfigureAwait(false); - - // Rename a to b, change lines 2 and 28 of b, and stage all of it. - repository.DeleteFile("a"); - lines[1] = "2 changed"; - lines[27] = "28 changed"; - repository.WriteFile("b", string.Join('\n', lines) + "\n"); - _ = await seeded.Add().All() - .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); - - GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); - - GitFilePatch file = (await opened.Patch().Staged().DetectRenames().ExecuteAsync().ConfigureAwait(false)).Files.Single(); - - Assert.AreEqual("a", file.OriginalPath?.WeakString, "The fixture must stage a rename, or this proves nothing."); - Assert.AreEqual(2, file.Hunks.Count, "The fixture must produce two hunks, or this proves nothing."); - - _ = await opened.Apply(file.PatchFor([file.Hunks[0]])).ToIndex().Reversed().ExecuteAsync().ConfigureAwait(false); - - IReadOnlyList staged = await opened.Diff().Staged().DetectRenames().WithLineCounts() - .ExecuteAsync().ConfigureAwait(false); - IReadOnlyList unstaged = await opened.Diff().WithLineCounts() - .ExecuteAsync().ConfigureAwait(false); - - GitDiffEntry rename = staged.Single(); - Assert.AreEqual(GitChangeKind.Renamed, rename.Kind); - Assert.AreEqual("b", rename.Path.WeakString); - Assert.AreEqual("a", rename.OriginalPath?.WeakString); - Assert.AreEqual(1, rename.Insertions, "Only the line-28 change stays staged."); - - GitDiffEntry pending = unstaged.Single(); - Assert.AreEqual("b", pending.Path.WeakString); - Assert.AreEqual(1, pending.Insertions, "Only the line-2 change was unstaged."); - } - /// Stages everything in the working tree and commits it. /// The repository to commit in. private async Task CommitAllAsync(GitRepository repository)