From bd76202df39342e9f3d1734e3fd36c5aafd350b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 12:31:53 +0000 Subject: [PATCH] Deliver a line ending in a bare CR at once instead of on the next write [patch] Since #85, a read that ended in CR kept its whole line buffered until the next read showed whether an LF followed. A progress or status line ending in a bare CR therefore arrived only when the command next wrote, or at EOF, which could be seconds late. A CR now ends its line immediately, even as the last character of a read. Each stream remembers that its last read ended on a CR, and an LF at the start of the next read completes the CRLF and is skipped instead of ending an empty line, which is what Process.OutputDataReceived does. Reset() and Complete() clear that state, and Complete() no longer strips a trailing CR. The test that pinned the held-back CR is replaced by tests that the line is delivered at once on both streams, that Complete() does not deliver it again, and that Reset() forgets the pending LF skip. The #85 CRLF-split tests are unchanged and still pass. Fixes #128 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW24pNCmCLDhz6z3ZdY --- RunCommand.Test/LineOutputHandlerTests.cs | 54 ++++++++++++- RunCommand/LineOutputHandler.cs | 95 ++++++++++++----------- 2 files changed, 102 insertions(+), 47 deletions(-) diff --git a/RunCommand.Test/LineOutputHandlerTests.cs b/RunCommand.Test/LineOutputHandlerTests.cs index b21b819..6a7ddba 100644 --- a/RunCommand.Test/LineOutputHandlerTests.cs +++ b/RunCommand.Test/LineOutputHandlerTests.cs @@ -6,6 +6,8 @@ namespace ktsu.RunCommand.Test; public class LineOutputHandlerTests { private static readonly string[] ExpectedXY = ["x", "y"]; + private static readonly string[] ExpectedX = ["x"]; + private static readonly string[] ExpectedXEmptyY = ["x", "", "y"]; private static readonly string[] ExpectedEveryLineEndingKind = ["a", "b", "c", "d", ""]; private static readonly string[] ExpectedALast = ["a", "last"]; private static readonly string[] ExpectedALastNext = ["a", "last", "next"]; @@ -135,7 +137,7 @@ public void HandleStandardErrorDataShouldTreatCrlfSplitAcrossChunksAsOneLineBrea } [TestMethod] - public void HandleStandardOutputDataShouldHoldBackTrailingCarriageReturn() + public void HandleStandardOutputDataShouldDeliverALineEndingInATrailingCarriageReturnAtOnce() { // Arrange List lines = []; @@ -144,9 +146,55 @@ public void HandleStandardOutputDataShouldHoldBackTrailingCarriageReturn() // Act handler.HandleStandardOutputData("x\r"); + // Assert: delivered before any further read and before Complete() + Assert.AreSequenceEqual(ExpectedX, lines); + Assert.AreEqual(string.Empty, handler.outputBuffer.ToString()); + } + + [TestMethod] + public void HandleStandardErrorDataShouldDeliverALineEndingInATrailingCarriageReturnAtOnce() + { + // Arrange + List lines = []; + LineOutputHandler handler = new(onStandardError: lines.Add); + + // Act + handler.HandleStandardErrorData("x\r"); + + // Assert + Assert.AreSequenceEqual(ExpectedX, lines); + Assert.AreEqual(string.Empty, handler.errorBuffer.ToString()); + } + + [TestMethod] + public void CompleteShouldNotDeliverALineAlreadyEndedByATrailingCarriageReturn() + { + // Arrange + List lines = []; + LineOutputHandler handler = new(onStandardOutput: lines.Add); + handler.HandleStandardOutputData("x\r"); + + // Act + handler.Complete(); + // Assert - Assert.IsEmpty(lines); - Assert.AreEqual("x\r", handler.outputBuffer.ToString()); + Assert.AreSequenceEqual(ExpectedX, lines); + } + + [TestMethod] + public void ResetShouldForgetAnLfStillDueToBeSkipped() + { + // Arrange + List lines = []; + LineOutputHandler handler = new(onStandardOutput: lines.Add); + handler.HandleStandardOutputData("x\r"); + + // Act + handler.Reset(); + handler.HandleStandardOutputData("\ny\n"); + + // Assert: the next run's leading LF ends an empty line of its own + Assert.AreSequenceEqual(ExpectedXEmptyY, lines); } [TestMethod] diff --git a/RunCommand/LineOutputHandler.cs b/RunCommand/LineOutputHandler.cs index d04223a..9fc77c6 100644 --- a/RunCommand/LineOutputHandler.cs +++ b/RunCommand/LineOutputHandler.cs @@ -17,13 +17,13 @@ public class LineOutputHandler : OutputHandler /// Replaced rather than cleared by , so a delivery still in progress from a /// cancelled run finishes into the old buffer instead of the new run's. /// - internal StringBuilder outputBuffer = new(); + internal LineBuffer outputBuffer = new(); /// /// Buffer to store incomplete lines from standard error. /// /// See . - internal StringBuilder errorBuffer = new(); + internal LineBuffer errorBuffer = new(); /// /// Initializes a new instance of the class. @@ -68,8 +68,8 @@ internal override void Complete() } /// - /// Discards any partial line, including a CR still waiting to see whether an LF follows, that a - /// cancelled or failed run left in the buffers. + /// Discards any partial line, and any LF still due to be skipped after a CR, that a cancelled or + /// failed run left in the buffers. /// internal override void Reset() { @@ -82,24 +82,17 @@ internal override void Reset() /// /// The buffer holding an incomplete line. /// The action to be invoked for the final line. - /// - /// A buffer that ends in a CR is a line whose break had not yet been confirmed as CR or CRLF. - /// At the end of the stream it is a CR on its own, so it ends the line rather than being part of it. - /// - private static void FlushBuffer(StringBuilder buffer, Action? onLineReceived) + private static void FlushBuffer(LineBuffer buffer, Action? onLineReceived) { - if (buffer.Length == 0) - { - return; - } + buffer.SkipLeadingLineFeed = false; - if (buffer[^1] == '\r') + if (buffer.Text.Length == 0) { - buffer.Length--; + return; } - string line = buffer.ToString(); - buffer.Clear(); + string line = buffer.Text.ToString(); + buffer.Text.Clear(); onLineReceived?.Invoke(line); } @@ -111,36 +104,50 @@ private static void FlushBuffer(StringBuilder buffer, Action? onLineRece /// The action to be invoked for each complete line received. /// /// Line endings are recognised across reads, so a CRLF split across two reads is still one line break. - /// A trailing CR stays in the buffer until the next chunk shows whether an LF follows. + /// A CR ends its line at once, even as the last character of a read, so a progress or status line + /// ending in a bare CR is delivered without waiting for the command's next write. If the next read + /// then starts with an LF, that LF completes the CRLF and is skipped rather than ending an empty + /// line. This is what Process.OutputDataReceived does. /// Only the newly arrived is scanned, and the partial line is appended to rather than /// copied, so the cost is linear in the output size however long a line grows. /// - private static void ProcessDataByLine(string data, StringBuilder buffer, Action? onLineReceived) + private static void ProcessDataByLine(string data, LineBuffer buffer, Action? onLineReceived) { if (data.Length == 0) { return; } - int i = CompletePendingCarriageReturn(data, buffer, onLineReceived); + int i = 0; + if (buffer.SkipLeadingLineFeed) + { + buffer.SkipLeadingLineFeed = false; + if (data[0] == '\n') + { + i = 1; + } + } + int lineStart = i; while (i < data.Length) { char c = data[i]; if (c == '\r') { + EmitLine(buffer.Text, data, lineStart, i, onLineReceived); if (i == data.Length - 1) { - break; + // The LF of a CRLF may arrive with the next read + buffer.SkipLeadingLineFeed = true; + return; } - EmitLine(buffer, data, lineStart, i, onLineReceived); i += data[i + 1] == '\n' ? 2 : 1; lineStart = i; } else if (IsLineBreak(c)) { - EmitLine(buffer, data, lineStart, i, onLineReceived); + EmitLine(buffer.Text, data, lineStart, i, onLineReceived); i++; lineStart = i; } @@ -150,27 +157,7 @@ private static void ProcessDataByLine(string data, StringBuilder buffer, Action< } } - buffer.Append(data, lineStart, data.Length - lineStart); - } - - /// - /// Ends the buffered line when the previous read finished on a CR, now that shows whether - /// an LF follows it. - /// - /// The newly arrived data, which must not be empty. - /// The buffer holding an incomplete line. - /// The action to be invoked for the completed line. - /// The index in to resume scanning from: 1 to skip the LF of a CRLF, otherwise 0. - private static int CompletePendingCarriageReturn(string data, StringBuilder buffer, Action? onLineReceived) - { - if (buffer.Length == 0 || buffer[^1] != '\r') - { - return 0; - } - - buffer.Length--; - EmitLine(buffer, data, 0, 0, onLineReceived); - return data[0] == '\n' ? 1 : 0; + buffer.Text.Append(data, lineStart, data.Length - lineStart); } /// @@ -205,4 +192,24 @@ private static void EmitLine(StringBuilder buffer, string data, int start, int e /// The character to test. /// if is LF, NEL, LS, PS or FF; otherwise . private static bool IsLineBreak(char c) => c is '\n' or '\u0085' or '\u2028' or '\u2029' or '\f'; + + /// + /// The partial line one stream has delivered so far, and whether its last read ended on a CR. + /// + internal sealed class LineBuffer + { + /// + /// Gets the start of a line whose line break has not arrived yet. + /// + internal StringBuilder Text { get; } = new(); + + /// + /// Gets or sets a value indicating whether the last read ended on a CR, whose line has already + /// been delivered, so an LF at the start of the next read completes a CRLF and is skipped. + /// + internal bool SkipLeadingLineFeed { get; set; } + + /// + public override string ToString() => Text.ToString(); + } }