diff --git a/RunCommand.Test/LineOutputHandlerTests.cs b/RunCommand.Test/LineOutputHandlerTests.cs index 3b84635..737a701 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 298f69d..01b7f7c 100644 --- a/RunCommand/LineOutputHandler.cs +++ b/RunCommand/LineOutputHandler.cs @@ -21,13 +21,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. @@ -72,8 +72,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() { @@ -95,24 +95,17 @@ internal override OutputHandler CreateRunHandler() => /// /// 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); } @@ -124,36 +117,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; } @@ -163,27 +170,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); } /// @@ -220,4 +207,24 @@ private static void EmitLine(StringBuilder buffer, string data, int start, int e /// The character to test. /// if is LF; otherwise . private static bool IsLineBreak(char c) => c == '\n'; + + /// + /// 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(); + } }