Keep line endings and BOM when taking a diff block [patch]

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

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;
+
+/// <summary>
+/// Tests that taking one block in the file-diff view changes only that block's bytes.
+/// </summary>
+/// <remarks>
+/// The take arrows used to rebuild the destination with <see cref="Environment.NewLine"/> 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 <see cref="DiffTake"/> 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.
+/// </remarks>
+[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));
+}

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;
+
+/// <summary>
+/// Builds and writes the file that taking one diff block produces.
+/// </summary>
+/// <remarks>
+/// 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
+/// <see cref="Environment.NewLine"/> and a plain <see cref="File.WriteAllText(string, string?)"/>
+/// 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.
+/// </remarks>
+internal static class DiffTake
+{
+	/// <summary>
+	/// Builds the lines of the right-hand file after the left-hand side of <paramref name="block"/> is taken into it.
+	/// </summary>
+	/// <param name="diff">The diff of the left-hand file (old) against the right-hand file (new).</param>
+	/// <param name="block">The block to take.</param>
+	/// <returns>The right-hand file's lines with the block replaced by the left-hand lines.</returns>
+	internal static Collection<string> 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),
+		];
+	}
+
+	/// <summary>
+	/// Builds the lines of the left-hand file after the right-hand side of <paramref name="block"/> is taken into it.
+	/// </summary>
+	/// <param name="diff">The diff of the left-hand file (old) against the right-hand file (new).</param>
+	/// <param name="block">The block to take.</param>
+	/// <returns>The left-hand file's lines with the block replaced by the right-hand lines.</returns>
+	internal static Collection<string> 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),
+		];
+	}
+
+	/// <summary>
+	/// Writes <paramref name="lines"/> over <paramref name="path"/>, keeping the line ending and
+	/// encoding (including any byte order mark) the file already has.
+	/// </summary>
+	/// <param name="path">The file to overwrite.</param>
+	/// <param name="lines">The file's new lines, without terminators.</param>
+	/// <remarks>
+	/// A file that does not exist yet, or has no line break to copy, gets
+	/// <see cref="Environment.NewLine"/> and UTF-8 without a BOM, which is what was written before.
+	/// </remarks>
+	internal static void WriteLinesPreservingFormat(string path, IEnumerable<string> 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);
+	}
+
+	/// <summary>
+	/// Finds the line ending a text uses, judged by its first line break.
+	/// </summary>
+	/// <param name="text">The text to inspect.</param>
+	/// <returns>The first line break's characters, or <see langword="null"/> when there is none.</returns>
+	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";
+	}
+}

ProjectDirector/ProjectDirector.cs

@@ -18,7 +18,7 @@
 using ktsu.ImGui.Widgets;
 using ktsu.ImGui.Styler;
 using Octokit;
 // using OpenAI.Chat;
 using Semantics.Paths;
 
 #pragma warning disable CA1506
@@ -61,7 +61,7 @@
 	/// </summary>
 	private CloneTracker Clones { get; } = new();
 
 	// private ChatClient ChatClient { get; init; }
 
 	private static void Main(string[] _)
 	{
@@ -84,7 +84,7 @@
 		_ = MakeLoadedOptionsSafe(Options, QueueLog);
 
 		Options.Save();
 		// ChatClient = new(model: "gpt-4o", new ApiKeyCredential(Options.OpenAIToken));
 		DividerDiff = new("DiffDivider", DividerResized, ImGuiWidgets.DividerLayout.Columns);
 		DividerContainerCols = new("VerticalDivider", DividerResized, ImGuiWidgets.DividerLayout.Columns);
 		DividerContainerRows = new("HorizontalDivider", DividerResized, ImGuiWidgets.DividerLayout.Rows);
@@ -743,7 +743,7 @@
 						});
 					}
 
 					//int fetchInterval = repo.MinFetchIntervalSeconds;
 					//if (ImGuiWidgets.Knob("Min Fetch Interval", ref fetchInterval, 0, 300, 150))
 					//{
 					//	repo.MinFetchIntervalSeconds = fetchInterval;
@@ -1782,12 +1782,7 @@
 		{
 			if (ImGui.ArrowButton($"DiffTakeLeft{i}", ImGuiDir.Right))
 			{
-				List<string> 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.Join(repoB.LocalPath, Options.CompareFile), DiffTake.TakeOldIntoNew(diff, block));
 				RefreshFileDiff(repoA, repoB, Options.CompareFile);
 			}
 
@@ -1818,25 +1813,6 @@
 
 		ScrollLeft = new(ImGui.GetScrollX(), ImGui.GetScrollY());
 
-		void AddPrologueForDeletedLines(DiffResult diff, DiffBlock block, List<string> newLines)
-		{
-			int endIndex = block.InsertStartB;
-			newLines.AddRange(diff.PiecesNew.Take(endIndex));
-		}
-
-		void AddEpilogueForDeletedLines(DiffResult diff, DiffBlock block, List<string> newLines)
-		{
-			int startIndex = block.InsertStartB + block.InsertCountB;
-			newLines.AddRange(diff.PiecesNew.Skip(startIndex));
-		}
-
-		void AddDeletedLines(DiffResult diff, DiffBlock block, List<string> 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 @@
 		{
 			if (ImGui.ArrowButton($"DiffTakeRight{i}", ImGuiDir.Left))
 			{
-				List<string> 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.Join(repoA.LocalPath, Options.CompareFile), DiffTake.TakeNewIntoOld(diff, block));
 				RefreshFileDiff(repoA, repoB, Options.CompareFile);
 			}
 
@@ -1913,25 +1884,6 @@
 
 		ScrollRight = new(ImGui.GetScrollX(), ImGui.GetScrollY());
 
-		void AddPrologueForNewLines(DiffResult diff, DiffBlock block, List<string> newLines)
-		{
-			int endIndex = block.DeleteStartA;
-			newLines.AddRange(diff.PiecesOld.Take(endIndex));
-		}
-
-		void AddNewLines(DiffResult diff, DiffBlock block, List<string> 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<string> 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);
@@ -2250,7 +2202,7 @@
 
 				if (ImGui.TableNextColumn())
 				{
 					//if (ImGui.Button($"Propagate Directory###Propagate{path.Replace(Path.DirectorySeparatorChar, '.').Replace(Path.AltDirectorySeparatorChar, '.')}"))
 					//{
 					//	shouldOpenPopup |= true;
 					//	Options.PropagatePath = path;