From 7dbf57fde4582d84653dda969a67b6303b8d7d73 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 00:25:44 +0000 Subject: [PATCH 1/2] Keep line endings and BOM when taking a diff block [patch] The take arrows in the file-diff view rebuilt the destination with Environment.NewLine and wrote it as UTF-8 without a BOM, so taking one hunk rewrote every line ending and dropped the byte order mark. The block-building and the write now live in DiffTake, which reads the destination's encoding and first line break back from disk and writes with them. Fixes ktsu-dev/ProjectDirector#445 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FRrKUmpjRrYrUchAi6wGqR --- CLAUDE.md | 2 +- ProjectDirector.Test/DiffTakeTests.cs | 131 ++++++++++++++++++++++++++ ProjectDirector/DiffTake.cs | 111 ++++++++++++++++++++++ ProjectDirector/ProjectDirector.cs | 52 +--------- 4 files changed, 245 insertions(+), 51 deletions(-) create mode 100644 ProjectDirector.Test/DiffTakeTests.cs create mode 100644 ProjectDirector/DiffTake.cs diff --git a/CLAUDE.md b/CLAUDE.md index a4a95a7..c9bad2c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,7 +24,7 @@ dotnet publish --configuration Release --output ./staging Tests live in `ProjectDirector.Test` (MSTest, via `MSTest.Sdk` + `ktsu.Sdk`). The app exposes its internals to the test project through `InternalsVisibleTo` in `ProjectDirector/AssemblyInfo.cs`. `GitCliTests` drives `GitCli` against throwaway repositories under the temp directory; the ImGui layer is not unit-tested. -That last point is why the repository actions are shaped the way they are: the part of each with a rule in it is pulled out into a plain method so it can be driven without a live ImGui context or a display. `ProjectDirector.DecidePull` decides whether pulling interrupts the user first (`PullDecisionTests`), and `GitCli.ListPendingChanges` plus `ProjectDirector.DescribePendingChanges` decide what a commit will sweep up and how that is shown (`CommitTests`). Anything genuinely worth testing that is still tangled up with drawing is usually worth extracting the same way. +That last point is why the repository actions are shaped the way they are: the part of each with a rule in it is pulled out into a plain method so it can be driven without a live ImGui context or a display. `ProjectDirector.DecidePull` decides whether pulling interrupts the user first (`PullDecisionTests`), and `GitCli.ListPendingChanges` plus `ProjectDirector.DescribePendingChanges` decide what a commit will sweep up and how that is shown (`CommitTests`). `DiffTake` builds the file a take arrow in the diff view produces and writes it back with the destination's own line ending and BOM (`DiffTakeTests`). Anything genuinely worth testing that is still tangled up with drawing is usually worth extracting the same way. ```powershell dotnet test --configuration Release diff --git a/ProjectDirector.Test/DiffTakeTests.cs b/ProjectDirector.Test/DiffTakeTests.cs new file mode 100644 index 0000000..8084b7a --- /dev/null +++ b/ProjectDirector.Test/DiffTakeTests.cs @@ -0,0 +1,131 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector.Test; + +using System; +using System.IO; +using System.Linq; +using System.Text; + +using DiffPlex; +using DiffPlex.Model; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests that taking one block in the file-diff view changes only that block's bytes. +/// +/// +/// The take arrows used to rebuild the destination with and write it +/// as UTF-8 without a BOM, so taking one line rewrote every line ending in the file and dropped its +/// byte order mark. These drive the way the view does: read both files as text, +/// diff them with DiffPlex, take a block, and write the result back over real files on disk. +/// +[TestClass] +public sealed class DiffTakeTests +{ + private static readonly byte[] Utf8Bom = [0xEF, 0xBB, 0xBF]; + + private static string CreateWorkspace() + { + string root = Path.Join(Path.GetTempPath(), $"ktsu_pd_take_{Guid.NewGuid():N}"); + _ = Directory.CreateDirectory(root); + return root; + } + + private static void Cleanup(string root) + { + try + { + Directory.Delete(root, recursive: true); + } + catch (IOException) + { + // A leaked temp directory is not worth failing an otherwise passing test over. + } + catch (UnauthorizedAccessException) + { + // Same. + } + } + + private static byte[] Bytes(string text, bool bom = false) => + [.. bom ? Utf8Bom : [], .. Encoding.UTF8.GetBytes(text)]; + + private static DiffResult Diff(string oldPath, string newPath) => + Differ.Instance.CreateLineDiffs(File.ReadAllText(oldPath), File.ReadAllText(newPath), ignoreWhitespace: false, ignoreCase: false); + + [TestMethod] + public void TakingARightBlockKeepsTheLeftFilesCrLfAndBom() + { + string root = CreateWorkspace(); + try + { + string left = Path.Join(root, "left.txt"); + string right = Path.Join(root, "right.txt"); + File.WriteAllBytes(left, Bytes("one\r\ntwo\r\nthree\r\n", bom: true)); + File.WriteAllBytes(right, Bytes("one\r\nTWO\r\nthree\r\n")); + + DiffResult diff = Diff(left, right); + DiffTake.WriteLinesPreservingFormat(left, DiffTake.TakeNewIntoOld(diff, diff.DiffBlocks.Single())); + + CollectionAssert.AreEqual(Bytes("one\r\nTWO\r\nthree\r\n", bom: true), File.ReadAllBytes(left)); + } + finally + { + Cleanup(root); + } + } + + [TestMethod] + public void TakingALeftBlockKeepsTheRightFilesCrLfWithoutAddingABom() + { + string root = CreateWorkspace(); + try + { + string left = Path.Join(root, "left.txt"); + string right = Path.Join(root, "right.txt"); + File.WriteAllBytes(left, Bytes("one\ntwo\nthree\n", bom: true)); + File.WriteAllBytes(right, Bytes("one\r\nTWO\r\nthree\r\n")); + + DiffResult diff = Diff(left, right); + DiffTake.WriteLinesPreservingFormat(right, DiffTake.TakeOldIntoNew(diff, diff.DiffBlocks.Single())); + + CollectionAssert.AreEqual(Bytes("one\r\ntwo\r\nthree\r\n"), File.ReadAllBytes(right)); + } + finally + { + Cleanup(root); + } + } + + [TestMethod] + public void TakingABlockKeepsAnLfFileLf() + { + string root = CreateWorkspace(); + try + { + string left = Path.Join(root, "left.txt"); + string right = Path.Join(root, "right.txt"); + File.WriteAllBytes(left, Bytes("one\ntwo\nthree\n")); + File.WriteAllBytes(right, Bytes("one\r\nTWO\r\nthree\r\n")); + + DiffResult diff = Diff(left, right); + DiffTake.WriteLinesPreservingFormat(left, DiffTake.TakeNewIntoOld(diff, diff.DiffBlocks.Single())); + + CollectionAssert.AreEqual(Bytes("one\nTWO\nthree\n"), File.ReadAllBytes(left)); + } + finally + { + Cleanup(root); + } + } + + [TestMethod] + [DataRow("one\r\ntwo\n", "\r\n")] + [DataRow("one\ntwo\r\n", "\n")] + [DataRow("one\rtwo", "\r")] + [DataRow("one", null)] + [DataRow("", null)] + public void DetectNewLineReportsTheFirstLineBreak(string text, string? expected) => + Assert.AreEqual(expected, DiffTake.DetectNewLine(text)); +} diff --git a/ProjectDirector/DiffTake.cs b/ProjectDirector/DiffTake.cs new file mode 100644 index 0000000..278af8e --- /dev/null +++ b/ProjectDirector/DiffTake.cs @@ -0,0 +1,111 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector; + +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.IO; +using System.Text; + +using DiffPlex.Model; + +/// +/// Builds and writes the file that taking one diff block produces. +/// +/// +/// DiffPlex hands back lines with their terminators stripped, and the file was read as text, which +/// drops any byte order mark. Rebuilding the file from those pieces with +/// and a plain +/// therefore rewrote every line ending and lost the BOM, turning a one-line take into a whole-file +/// change. The destination's own line ending and encoding are read back from disk here instead, so +/// only the taken lines differ. +/// +internal static class DiffTake +{ + /// + /// Builds the lines of the right-hand file after the left-hand side of is taken into it. + /// + /// The diff of the left-hand file (old) against the right-hand file (new). + /// The block to take. + /// The right-hand file's lines with the block replaced by the left-hand lines. + internal static Collection TakeOldIntoNew(DiffResult diff, DiffBlock block) + { + Ensure.NotNull(diff); + Ensure.NotNull(block); + + return + [ + .. diff.PiecesNew.Take(block.InsertStartB), + .. diff.PiecesOld.Skip(block.DeleteStartA).Take(block.DeleteCountA), + .. diff.PiecesNew.Skip(block.InsertStartB + block.InsertCountB), + ]; + } + + /// + /// Builds the lines of the left-hand file after the right-hand side of is taken into it. + /// + /// The diff of the left-hand file (old) against the right-hand file (new). + /// The block to take. + /// The left-hand file's lines with the block replaced by the right-hand lines. + internal static Collection TakeNewIntoOld(DiffResult diff, DiffBlock block) + { + Ensure.NotNull(diff); + Ensure.NotNull(block); + + return + [ + .. diff.PiecesOld.Take(block.DeleteStartA), + .. diff.PiecesNew.Skip(block.InsertStartB).Take(block.InsertCountB), + .. diff.PiecesOld.Skip(block.DeleteStartA + block.DeleteCountA), + ]; + } + + /// + /// Writes over , keeping the line ending and + /// encoding (including any byte order mark) the file already has. + /// + /// The file to overwrite. + /// The file's new lines, without terminators. + /// + /// A file that does not exist yet, or has no line break to copy, gets + /// and UTF-8 without a BOM, which is what was written before. + /// + internal static void WriteLinesPreservingFormat(string path, IEnumerable lines) + { + Ensure.NotNull(path); + Ensure.NotNull(lines); + + Encoding encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false); + string newLine = Environment.NewLine; + + if (File.Exists(path)) + { + using StreamReader reader = new(path, encoding, detectEncodingFromByteOrderMarks: true); + string existing = reader.ReadToEnd(); + encoding = reader.CurrentEncoding; + newLine = DetectNewLine(existing) ?? newLine; + } + + File.WriteAllText(path, string.Join(newLine, lines), encoding); + } + + /// + /// Finds the line ending a text uses, judged by its first line break. + /// + /// The text to inspect. + /// The first line break's characters, or when there is none. + internal static string? DetectNewLine(string text) + { + Ensure.NotNull(text); + + int index = text.IndexOfAny(['\r', '\n']); + if (index < 0) + { + return null; + } + + return text[index] == '\r' + ? index + 1 < text.Length && text[index + 1] == '\n' ? "\r\n" : "\r" + : "\n"; + } +} diff --git a/ProjectDirector/ProjectDirector.cs b/ProjectDirector/ProjectDirector.cs index 8bdac12..08b0390 100644 --- a/ProjectDirector/ProjectDirector.cs +++ b/ProjectDirector/ProjectDirector.cs @@ -1782,12 +1782,7 @@ private void ShowDiffLeft(GitRepository repoA, GitRepository repoB, DiffResult? { if (ImGui.ArrowButton($"DiffTakeLeft{i}", ImGuiDir.Right)) { - List newLines = []; - AddPrologueForDeletedLines(diff, block, newLines); - AddDeletedLines(diff, block, newLines); - AddEpilogueForDeletedLines(diff, block, newLines); - string newText = string.Join(Environment.NewLine, newLines); - File.WriteAllText(Path.Combine(repoB.LocalPath, Options.CompareFile), newText); + DiffTake.WriteLinesPreservingFormat(Path.Combine(repoB.LocalPath, Options.CompareFile), DiffTake.TakeOldIntoNew(diff, block)); RefreshFileDiff(repoA, repoB, Options.CompareFile); } @@ -1818,25 +1813,6 @@ private void ShowDiffLeft(GitRepository repoA, GitRepository repoB, DiffResult? ScrollLeft = new(ImGui.GetScrollX(), ImGui.GetScrollY()); - void AddPrologueForDeletedLines(DiffResult diff, DiffBlock block, List newLines) - { - int endIndex = block.InsertStartB; - newLines.AddRange(diff.PiecesNew.Take(endIndex)); - } - - void AddEpilogueForDeletedLines(DiffResult diff, DiffBlock block, List newLines) - { - int startIndex = block.InsertStartB + block.InsertCountB; - newLines.AddRange(diff.PiecesNew.Skip(startIndex)); - } - - void AddDeletedLines(DiffResult diff, DiffBlock block, List newLines) - { - int startIndex = block.DeleteStartA; - int endIndex = startIndex + block.DeleteCountA; - newLines.AddRange(diff.PiecesOld.Skip(startIndex).Take(endIndex - startIndex)); - } - void ShowPrologueForDeletedLines(DiffResult diff, DiffBlock block) { int startIndex = Math.Max(block.DeleteStartA - 3, 0); @@ -1877,12 +1853,7 @@ private void ShowDiffRight(GitRepository repoA, GitRepository repoB, DiffResult? { if (ImGui.ArrowButton($"DiffTakeRight{i}", ImGuiDir.Left)) { - List newLines = []; - AddPrologueForNewLines(diff, block, newLines); - AddNewLines(diff, block, newLines); - AddEpilogueForNewLines(diff, block, newLines); - string newText = string.Join(Environment.NewLine, newLines); - File.WriteAllText(Path.Combine(repoA.LocalPath, Options.CompareFile), newText); + DiffTake.WriteLinesPreservingFormat(Path.Combine(repoA.LocalPath, Options.CompareFile), DiffTake.TakeNewIntoOld(diff, block)); RefreshFileDiff(repoA, repoB, Options.CompareFile); } @@ -1913,25 +1884,6 @@ private void ShowDiffRight(GitRepository repoA, GitRepository repoB, DiffResult? ScrollRight = new(ImGui.GetScrollX(), ImGui.GetScrollY()); - void AddPrologueForNewLines(DiffResult diff, DiffBlock block, List newLines) - { - int endIndex = block.DeleteStartA; - newLines.AddRange(diff.PiecesOld.Take(endIndex)); - } - - void AddNewLines(DiffResult diff, DiffBlock block, List newLines) - { - int startIndex = block.InsertStartB; - int endIndex = startIndex + block.InsertCountB; - newLines.AddRange(diff.PiecesNew.Skip(startIndex).Take(endIndex - startIndex)); - } - - void AddEpilogueForNewLines(DiffResult diff, DiffBlock block, List newLines) - { - int startIndex = block.DeleteStartA + block.DeleteCountA; - newLines.AddRange(diff.PiecesOld.Skip(startIndex)); - } - void ShowPrologueForNewLines(DiffResult diff, DiffBlock block) { int startIndex = Math.Max(block.InsertStartB - 3, 0); From 54c125912e960284c6244c304dd01c045ab5df9f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 00:32:29 +0000 Subject: [PATCH 2/2] Join the take destination with Path.Join so the repository root cannot be dropped Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FRrKUmpjRrYrUchAi6wGqR --- ProjectDirector/ProjectDirector.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ProjectDirector/ProjectDirector.cs b/ProjectDirector/ProjectDirector.cs index 08b0390..1e4ab07 100644 --- a/ProjectDirector/ProjectDirector.cs +++ b/ProjectDirector/ProjectDirector.cs @@ -1782,7 +1782,7 @@ private void ShowDiffLeft(GitRepository repoA, GitRepository repoB, DiffResult? { if (ImGui.ArrowButton($"DiffTakeLeft{i}", ImGuiDir.Right)) { - DiffTake.WriteLinesPreservingFormat(Path.Combine(repoB.LocalPath, Options.CompareFile), DiffTake.TakeOldIntoNew(diff, block)); + DiffTake.WriteLinesPreservingFormat(Path.Join(repoB.LocalPath, Options.CompareFile), DiffTake.TakeOldIntoNew(diff, block)); RefreshFileDiff(repoA, repoB, Options.CompareFile); } @@ -1853,7 +1853,7 @@ private void ShowDiffRight(GitRepository repoA, GitRepository repoB, DiffResult? { if (ImGui.ArrowButton($"DiffTakeRight{i}", ImGuiDir.Left)) { - DiffTake.WriteLinesPreservingFormat(Path.Combine(repoA.LocalPath, Options.CompareFile), DiffTake.TakeNewIntoOld(diff, block)); + DiffTake.WriteLinesPreservingFormat(Path.Join(repoA.LocalPath, Options.CompareFile), DiffTake.TakeNewIntoOld(diff, block)); RefreshFileDiff(repoA, repoB, Options.CompareFile); }