Refactor process output handling

src/BuiltInTools/dotnet-watch/Browser/BrowserConnector.cs

@@ -6,6 +6,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Text.RegularExpressions;
 using Microsoft.Build.Graph;
+using Microsoft.DotNet.Watcher.Internal;
 using Microsoft.Extensions.Tools.Internal;
 
 namespace Microsoft.DotNet.Watcher.Tools
@@ -104,7 +105,7 @@ public bool TryGetRefreshServer(ProjectGraphNode projectNode, [NotNullWhen(true)
         /// <summary>
         /// Get process output handler that will be subscribed to the process output event every time the process is launched.
         /// </summary>
-        public DataReceivedEventHandler? GetBrowserLaunchTrigger(ProjectGraphNode projectNode, ProjectOptions projectOptions, BrowserRefreshServer? server, CancellationToken cancellationToken)
+        public Action<OutputLine>? GetBrowserLaunchTrigger(ProjectGraphNode projectNode, ProjectOptions projectOptions, BrowserRefreshServer? server, CancellationToken cancellationToken)
         {
             if (!CanLaunchBrowser(context, projectNode, projectOptions, out var launchProfile))
             {
@@ -120,17 +121,17 @@ public bool TryGetRefreshServer(ProjectGraphNode projectNode, [NotNullWhen(true)
 
             return handler;
 
-            void handler(object sender, DataReceivedEventArgs eventArgs)
+            void handler(OutputLine line)
             {
                 // We've redirected the output, but want to ensure that it continues to appear in the user's console.
-                Console.WriteLine(eventArgs.Data);
+                (line.IsError ? Console.Error : Console.Out).WriteLine(line.Content);
 
                 if (matchFound)
                 {
                     return;
                 }
 
-                var match = s_nowListeningRegex.Match(eventArgs.Data ?? "");
+                var match = s_nowListeningRegex.Match(line.Content);
                 if (!match.Success)
                 {
                     return;

src/BuiltInTools/dotnet-watch/HotReload/ProjectLauncher.cs

@@ -60,19 +60,6 @@ public async Task<RunningProject> LaunchProcessAsync(ProjectOptions projectOptio
                 : [projectOptions.Command, "--no-build", .. projectOptions.CommandArguments]
         };
 
-        // allow tests to watch for application output:
-        if (Reporter.ReportProcessOutput)
-        {
-            var projectPath = projectNode.ProjectInstance.FullPath;
-            processSpec.OnOutput += (sender, args) =>
-            {
-                if (args.Data != null)
-                {
-                    Reporter.ProcessOutput(projectPath, args.Data);
-                }
-            };
-        }
-
         var environmentBuilder = EnvironmentVariablesBuilder.FromCurrentEnvironment();
         var namedPipeName = Guid.NewGuid().ToString();
 
@@ -117,7 +104,7 @@ public async Task<RunningProject> LaunchProcessAsync(ProjectOptions projectOptio
         var browserRefreshServer = await browserConnector.LaunchOrRefreshBrowserAsync(projectNode, processSpec, environmentBuilder, projectOptions, cancellationToken);
         environmentBuilder.ConfigureProcess(processSpec);
 
-        var processReporter = new MessagePrefixingReporter($"[{projectNode.GetDisplayName()}] ", Reporter);
+        var processReporter = new ProjectSpecificReporter(projectNode, Reporter);
 
         return await compilationHandler.TrackRunningProjectAsync(
             projectNode,

src/BuiltInTools/dotnet-watch/Internal/ConsoleReporter.cs

@@ -1,8 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Diagnostics;
-using Microsoft.Build.Tasks;
+using Microsoft.Build.Graph;
+using Microsoft.DotNet.Watcher.Internal;
 
 namespace Microsoft.Extensions.Tools.Internal
 {
@@ -18,10 +18,13 @@ internal sealed class ConsoleReporter(IConsole console, bool verbose, bool quiet
 
         private readonly object _writeLock = new();
 
-        public bool ReportProcessOutput
+        public bool EnableProcessOutputReporting
             => false;
 
-        public void ProcessOutput(string projectPath, string data)
+        public void ReportProcessOutput(OutputLine line)
+            => throw new InvalidOperationException();
+
+        public void ReportProcessOutput(ProjectGraphNode project, OutputLine line)
             => throw new InvalidOperationException();
 
         private void WriteLine(TextWriter writer, string message, ConsoleColor? color, string emoji)

src/BuiltInTools/dotnet-watch/Internal/IReporter.cs

@@ -3,8 +3,10 @@
 
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
+using Microsoft.Build.Graph;
 using Microsoft.Build.Tasks;
 using Microsoft.DotNet.Watcher;
+using Microsoft.DotNet.Watcher.Internal;
 
 namespace Microsoft.Extensions.Tools.Internal
 {
@@ -79,16 +81,18 @@ public bool TryGetMessage(string? prefix, object?[] args, [NotNullWhen(true)] ou
     internal interface IReporter
     {
         void Report(MessageDescriptor descriptor, string prefix, object?[] args);
-        void ProcessOutput(string projectPath, string data);
 
         public bool IsVerbose
             => false;
 
         /// <summary>
-        /// True to call <see cref="ProcessOutput"/> when launched process writes to standard output.
+        /// True to call <see cref="ReportProcessOutput"/> when launched process writes to standard output.
         /// Used for testing.
         /// </summary>
-        bool ReportProcessOutput { get; }
+        bool EnableProcessOutputReporting { get; }
+
+        void ReportProcessOutput(OutputLine line);
+        void ReportProcessOutput(ProjectGraphNode project, OutputLine line);
 
         void Report(MessageDescriptor descriptor, params object?[] args)
             => Report(descriptor, prefix: "", args);

src/BuiltInTools/dotnet-watch/Internal/MsBuildFileSetFactory.cs

@@ -21,60 +21,37 @@ namespace Microsoft.DotNet.Watcher.Tools
     internal class MSBuildFileSetFactory(
         string rootProjectFile,
         string? targetFramework,
-        IReadOnlyList<(string, string)> buildProperties,
+        IReadOnlyList<(string name, string value)> buildProperties,
         EnvironmentOptions environmentOptions,
-        IReporter reporter,
-        OutputSink? outputSink,
-        bool trace)
+        IReporter reporter)
     {
         private const string TargetName = "GenerateWatchList";
         private const string WatchTargetsFileName = "DotNetWatch.targets";
 
-        private readonly OutputSink _outputSink = outputSink ?? new OutputSink();
-        private readonly IReadOnlyList<string> _buildFlags = InitializeArgs(FindTargetsFile(), targetFramework, buildProperties, trace);
-
         public string RootProjectFile => rootProjectFile;
 
         // Virtual for testing.
-
         public virtual async ValueTask<EvaluationResult?> TryCreateAsync(bool? requireProjectGraph, CancellationToken cancellationToken)
         {
             var watchList = Path.GetTempFileName();
             try
             {
                 var projectDir = Path.GetDirectoryName(rootProjectFile);
-
-                var capture = _outputSink.StartCapture();
-                var arguments = new List<string>
-                {
-                    "msbuild",
-                    "/restore",
-                    "/nologo",
-                    "/v:m",
-                    rootProjectFile,
-                    $"/p:_DotNetWatchListFile={watchList}",
-                };
-
-#if !DEBUG
-                if (environmentOptions.TestFlags.HasFlag(TestFlags.RunningAsTest))
-#endif
-                {
-                    arguments.Add("/bl:DotnetWatch.GenerateWatchList.binlog");
-                }
-
-                if (environmentOptions.SuppressHandlingStaticContentFiles)
-                {
-                    arguments.Add("/p:DotNetWatchContentFiles=false");
-                }
-
-                arguments.AddRange(_buildFlags);
+                var arguments = GetMSBuildArguments(watchList);
+                var capturedOutput = new List<OutputLine>();
 
                 var processSpec = new ProcessSpec
                 {
                     Executable = environmentOptions.MuxerPath,
                     WorkingDirectory = projectDir,
                     Arguments = arguments,
-                    OutputCapture = capture
+                    OnOutput = line =>
+                    {
+                        lock (capturedOutput)
+                        {
+                            capturedOutput.Add(line);
+                        }
+                    }
                 };
 
                 reporter.Verbose($"Running MSBuild target '{TargetName}' on '{rootProjectFile}'");
@@ -88,9 +65,17 @@ internal class MSBuildFileSetFactory(
                     reporter.Output($"MSBuild output from target '{TargetName}':");
                     reporter.Output(string.Empty);
 
-                    foreach (var line in capture.Lines)
+                    foreach (var (line, isError) in capturedOutput)
                     {
-                        reporter.Output($"   {line}");
+                        var message = "   " + line;
+                        if (isError)
+                        {
+                            reporter.Error(message);
+                        }
+                        else
+                        {
+                            reporter.Output(message);
+                        }
                     }
 
                     reporter.Output(string.Empty);
@@ -164,33 +149,49 @@ void AddFile(string filePath, string? staticWebAssetPath)
             }
         }
 
-        private static IReadOnlyList<string> InitializeArgs(string watchTargetsFile, string? targetFramework, IReadOnlyList<(string name, string value)> buildProperties, bool trace)
+        private IReadOnlyList<string> GetMSBuildArguments(string watchListFilePath)
         {
-            var args = new List<string>
+            var watchTargetsFile = FindTargetsFile();
+
+            var arguments = new List<string>
             {
+                "msbuild",
+                "/restore",
                 "/nologo",
-                "/v:n",
-                "/t:" + TargetName,
-                "/p:DotNetWatchBuild=true", // extensibility point for users
-                "/p:DesignTimeBuild=true", // don't do expensive things
-                "/p:CustomAfterMicrosoftCommonTargets=" + watchTargetsFile,
-                "/p:CustomAfterMicrosoftCommonCrossTargetingTargets=" + watchTargetsFile,
+                "/v:m",
+                rootProjectFile,
+                "/t:" + TargetName
             };
 
-            if (targetFramework != null)
+#if !DEBUG
+            if (environmentOptions.TestFlags.HasFlag(TestFlags.RunningAsTest))
+#endif
             {
-                args.Add("/p:TargetFramework=" + targetFramework);
+                arguments.Add("/bl:DotnetWatch.GenerateWatchList.binlog");
             }
 
-            args.AddRange(buildProperties.Select(p => $"/p:{p.name}={p.value}"));
+            arguments.AddRange(buildProperties.Select(p => $"/p:{p.name}={p.value}"));
+
+            // Set dotnet-watch reserved properties after the user specified propeties,
+            // so that the former take precedence.
+
+            if (environmentOptions.SuppressHandlingStaticContentFiles)
+            {
+                arguments.Add("/p:DotNetWatchContentFiles=false");
+            }
 
-            if (trace)
+            if (targetFramework != null)
             {
-                // enables capturing markers to know which projects have been visited
-                args.Add("/p:_DotNetWatchTraceOutput=true");
+                arguments.Add("/p:TargetFramework=" + targetFramework);
             }
 
-            return args;
+            arguments.Add("/p:_DotNetWatchListFile=" + watchListFilePath);
+            arguments.Add("/p:DotNetWatchBuild=true"); // extensibility point for users
+            arguments.Add("/p:DesignTimeBuild=true"); // don't do expensive things
+            arguments.Add("/p:CustomAfterMicrosoftCommonTargets=" + watchTargetsFile);
+            arguments.Add("/p:CustomAfterMicrosoftCommonCrossTargetingTargets=" + watchTargetsFile);
+
+            return arguments;
         }
 
         private static string FindTargetsFile()

src/BuiltInTools/dotnet-watch/Internal/NullReporter.cs

@@ -1,6 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Microsoft.Build.Graph;
+using Microsoft.DotNet.Watcher.Internal;
+
 namespace Microsoft.Extensions.Tools.Internal
 {
     /// <summary>
@@ -14,9 +17,15 @@ private NullReporter()
 
         public static IReporter Singleton { get; } = new NullReporter();
 
-        public bool ReportProcessOutput => false;
+        public bool EnableProcessOutputReporting
+            => false;
+
+        public void ReportProcessOutput(OutputLine line)
+            => throw new InvalidOperationException();
+
+        public void ReportProcessOutput(ProjectGraphNode project, OutputLine line)
+            => throw new InvalidOperationException();
 
-        public void ProcessOutput(string projectPath, string data) => throw new InvalidOperationException();
 
         public void Report(MessageDescriptor descriptor, string prefix, object?[] args)
         {

src/BuiltInTools/dotnet-watch/Internal/OutputLine.cs

@@ -0,0 +1,6 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.DotNet.Watcher.Internal;
+
+internal readonly record struct OutputLine(string Content, bool IsError);