Split each run's output into lines in a handler of its own [patch]

RunCommand.Test/RunCommandTests.cs

@@ -1421,13 +1421,55 @@
 		Assert.AreEqual("hello", string.Join(" | ", lines), "The next run's first line should not carry the cancelled run's leftover text.");
 	}
 
+	[TestMethod]
+	public async Task LineOutputHandlerSharedByOverlappingRunsShouldKeepEachRunsLinesApart()
+	{
+		if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+		{
+			Assert.Inconclusive("Needs sh to print part of a line, pause, then finish it. The per-run splitting this covers is platform independent, so the other legs cover it.");
+		}
+
+		// ktsu-dev/RunCommand#130: the second run's Reset threw away the first run's partial line, and
+		// both runs then appended to the same buffer, so their text came out joined into one line.
+		List<string> lines = [];
+		using SemaphoreSlim firstPartArrived = new(0);
+		LineOutputHandler handler = new(line =>
+		{
+			lock (lines)
+			{
+				lines.Add(line);
+			}
+		});
+
+		Task<int> first = RunCommand.ExecuteAsync(
+			"sh",
+			["-c", "printf AAAA; sleep 1; printf 'AAAA\\n'"],
+			new SignallingLineOutputHandler(handler, firstPartArrived));
+
+		Assert.IsTrue(await firstPartArrived.WaitAsync(TimeSpan.FromSeconds(10)).ConfigureAwait(false), "Expected the first run's partial line to arrive.");
+
+		Task<int> second = RunCommand.ExecuteAsync("sh", ["-c", "printf BBBB; sleep 0.2; printf 'BBBB\\n'"], handler);
+
+		int[] exitCodes = await Task.WhenAll(first, second).ConfigureAwait(false);
+
+		Assert.AreEqual(0, exitCodes[0], "The first run should succeed.");
+		Assert.AreEqual(0, exitCodes[1], "The second run should succeed.");
+		List<string> sorted;
+		lock (lines)
+		{
+			sorted = [.. lines.Order(StringComparer.Ordinal)];
+		}
+
+		Assert.AreEqual("AAAAAAAA | BBBBBBBB", string.Join(" | ", sorted));
+	}
+
 	[TestMethod]
 	public async Task ReusedLineOutputHandlerShouldDropTheLineARunWhoseCallbackThrewLeftUnfinished()
 	{
 		if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
 		{
 			Assert.Inconclusive("Needs sh to print a line without a line break. The reset this covers is in platform independent code, so the other legs cover it.");
 		}
 
 		List<string> lines = [];
 		bool throwOnError = true;
@@ -1445,7 +1487,8 @@
 		await Assert.ThrowsAsync<InvalidOperationException>(
 			() => RunCommand.ExecuteAsync("sh", ["-c", "printf partial; sleep 0.5; echo boom >&2; sleep 30"], handler)).ConfigureAwait(false);
 
-		Assert.AreEqual("partial", handler.outputBuffer.ToString(), "The failed run should have left its partial line buffered.");
+		// The failed run's partial line stays in that run's own handler (#130), so nothing is left on this one.
+		Assert.AreEqual(string.Empty, handler.outputBuffer.ToString(), "A run should split lines in its own handler, not the shared one.");
 
 		throwOnError = false;
 		int exitCode = await RunCommand.ExecuteAsync("sh", ["-c", "echo hello"], handler).ConfigureAwait(false);

RunCommand/LineOutputHandler.cs

@@ -8,6 +8,10 @@ namespace ktsu.RunCommand;
 /// Handles the output from a command process, processing both standard output and standard error streams.
 /// It processes the data line-by-line, calling the specified actions for each line received.
 /// </summary>
+/// <remarks>
+/// One handler can serve several runs at once. Each run splits its own output into lines, so a partial
+/// line from one run is never joined to another run's output.
+/// </remarks>
 public class LineOutputHandler : OutputHandler
 {
 	/// <summary>
@@ -77,6 +81,15 @@ internal override void Reset()
 		errorBuffer = new();
 	}
 
+	/// <summary>
+	/// Returns a new handler with the same delegates and encoding and empty buffers, so each run splits
+	/// its own output into lines. Two runs sharing this handler's buffers lost one run's partial line and
+	/// joined the two runs' text into one line (ktsu-dev/RunCommand#130).
+	/// </summary>
+	/// <returns>A handler for a single run.</returns>
+	internal override OutputHandler CreateRunHandler() =>
+		new LineOutputHandler(OnStandardOutput, OnStandardError, Encoding);
+
 	/// <summary>
 	/// Invokes <paramref name="onLineReceived"/> with the buffered final line, if there is one, and clears the buffer.
 	/// </summary>

RunCommand/OutputHandler.cs

@@ -12,6 +12,9 @@ namespace ktsu.RunCommand;
 /// The two streams are read concurrently, but the actions are never run at the same time: each one
 /// returns before the other is called, so both may share state, such as one list collecting all of a
 /// command's output, without locking. Each stream's chunks arrive in the order they were written.
+/// That guarantee covers one run. A handler passed to several runs at once, for example under
+/// <see cref="Task.WhenAll(Task[])"/>, may have its actions called by those runs at the same time,
+/// so state shared between them needs its own lock.
 /// </remarks>
 public class OutputHandler
 {
@@ -78,4 +81,12 @@ internal virtual void Complete() { }
 	/// and its leftover text is not a real line, so it is dropped rather than delivered.
 	/// </summary>
 	internal virtual void Reset() { }
+
+	/// <summary>
+	/// Returns the handler that one run feeds its output to. A handler that keeps per-run state, such as a
+	/// partial line, returns a fresh handler sharing its delegates and encoding, so runs that overlap on
+	/// this handler each keep their own state. A stateless handler returns itself.
+	/// </summary>
+	/// <returns>The handler for a single run.</returns>
+	internal virtual OutputHandler CreateRunHandler() => this;
 }

RunCommand/RunCommand.cs

@@ -23,7 +23,7 @@
 	/// </summary>
 	/// <param name="command">The command to execute.</param>
 	/// <returns>The exit code of the executed process.</returns>
 	[Obsolete("A command string is split on its first space, which cannot handle an executable path "
 		+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
 	public static int Execute(string command) =>
 		ExecuteAsync(command).Result;
@@ -34,7 +34,7 @@
 	/// <param name="command">The command to execute.</param>
 	/// <param name="outputHandler">The handler for processing command output.</param>
 	/// <returns>The exit code of the executed process.</returns>
 	[Obsolete("A command string is split on its first space, which cannot handle an executable path "
 		+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
 	public static int Execute(string command, OutputHandler outputHandler) =>
 		ExecuteAsync(command, outputHandler).Result;
@@ -45,7 +45,7 @@
 	/// <param name="command">The command to execute.</param>
 	/// <param name="elevation">The privilege level under which to run the command.</param>
 	/// <returns>The exit code of the executed process.</returns>
 	[Obsolete("A command string is split on its first space, which cannot handle an executable path "
 		+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
 	public static int Execute(string command, Elevation elevation) =>
 		ExecuteAsync(command, elevation).Result;
@@ -61,7 +61,7 @@
 	/// </param>
 	/// <param name="elevation">The privilege level under which to run the command.</param>
 	/// <returns>The exit code of the executed process.</returns>
 	[Obsolete("A command string is split on its first space, which cannot handle an executable path "
 		+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
 	public static int Execute(string command, OutputHandler outputHandler, Elevation elevation) =>
 		ExecuteAsync(command, outputHandler, elevation).Result;
@@ -108,7 +108,7 @@
 	/// </summary>
 	/// <param name="command">The command to execute.</param>
 	/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
 	[Obsolete("A command string is split on its first space, which cannot handle an executable path "
 		+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
 	public static async Task<int> ExecuteAsync(string command)
 		=> await ExecuteAsync(command, new OutputHandler()).ConfigureAwait(false);
@@ -119,7 +119,7 @@
 	/// <param name="command">The command to execute.</param>
 	/// <param name="outputHandler">The handler for processing command output.</param>
 	/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
 	[Obsolete("A command string is split on its first space, which cannot handle an executable path "
 		+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
 	public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler)
 		=> await ExecuteAsync(command, outputHandler, Elevation.Default).ConfigureAwait(false);
@@ -130,7 +130,7 @@
 	/// <param name="command">The command to execute.</param>
 	/// <param name="elevation">The privilege level under which to run the command.</param>
 	/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
 	[Obsolete("A command string is split on its first space, which cannot handle an executable path "
 		+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
 	public static async Task<int> ExecuteAsync(string command, Elevation elevation)
 		=> await ExecuteAsync(command, new(), elevation).ConfigureAwait(false);
@@ -146,7 +146,7 @@
 	/// </param>
 	/// <param name="elevation">The privilege level under which to run the command.</param>
 	/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
 	[Obsolete("A command string is split on its first space, which cannot handle an executable path "
 		+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
 	public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler, Elevation elevation)
 		=> await ExecuteAsync(command, outputHandler, elevation, CancellationToken.None).ConfigureAwait(false);
@@ -159,7 +159,7 @@
 	/// A token that, when cancelled, terminates the running process and its children.
 	/// </param>
 	/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
 	[Obsolete("A command string is split on its first space, which cannot handle an executable path "
 		+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
 	public static async Task<int> ExecuteAsync(string command, CancellationToken cancellationToken)
 		=> await ExecuteAsync(command, new OutputHandler(), cancellationToken).ConfigureAwait(false);
@@ -174,7 +174,7 @@
 	/// A token that, when cancelled, terminates the running process and its children.
 	/// </param>
 	/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
 	[Obsolete("A command string is split on its first space, which cannot handle an executable path "
 		+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
 	public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler, CancellationToken cancellationToken)
 		=> await ExecuteAsync(command, outputHandler, Elevation.Default, cancellationToken).ConfigureAwait(false);
@@ -460,8 +460,10 @@
 	{
 		cancellationToken.ThrowIfCancellationRequested();
 
-		// A handler can be reused, and a run that was cancelled or failed never reached Complete, so
-		// whatever partial line it left behind would otherwise be glued onto this run's first line.
+		// A handler can be reused, including by runs that overlap, so each run feeds a handler of its
+		// own: a partial line one run leaves behind, whether it was cancelled, failed or is still going,
+		// is never glued onto another run's output. Reset covers a stateful handler that returns itself.
+		outputHandler = outputHandler.CreateRunHandler();
 		outputHandler.Reset();
 
 		using Process process = new() { StartInfo = startInfo };