Handle redirected input/output in collect-linux#5745
Handle redirected input/output in collect-linux#5745zachcmadsen wants to merge 2 commits intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates dotnet-trace collect-linux to avoid console APIs that throw when stdin/stdout are redirected, aligning behavior with dotnet-trace collect.
Changes:
- Add internal flags to disable “stop on Enter” when input is redirected.
- Disable status/progress line rewriting when output is redirected.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/Tools/dotnet-trace/CommandLine/Commands/CollectLinuxCommand.cs
Outdated
Show resolved
Hide resolved
| if (printStatusUpdate && ot == OutputType.Progress) | ||
| { |
There was a problem hiding this comment.
When input is redirected, stopOnEnter is false, but the progress output printed in this block still says "Press ...". Please adjust the message (or conditionally print it) so it doesn't instruct the user to press Enter when Enter won't be observed.
There was a problem hiding this comment.
Is IsInputRedirected=true + IsOutputRedirected=false useful? I'm guessing we haven't had users hitting that scenario given CollectCommand.cs doesn't condition its "Press ..." message either.
There was a problem hiding this comment.
@mdh1418 I opted to leave it as is given that collect doesn't do that. Up to you though
src/Tools/dotnet-trace/CommandLine/Commands/CollectLinuxCommand.cs
Outdated
Show resolved
Hide resolved
mdh1418
left a comment
mdh1418
left a comment
There was a problem hiding this comment.
Thanks, we should add some unit tests to CollectLinuxCommandFunctionalTests for these scenarios if we can.
| private long statusUpdateTimestamp; | ||
| private Version minRuntimeSupportingUserEventsIPCCommand = new(10, 0, 0); | ||
| private readonly bool cancelOnEnter = true; | ||
| private readonly bool printStatusOverTime = true; |
There was a problem hiding this comment.
Nit: I don't think there is any reason to init these if the constructor will re-init them.
noahfalk
left a comment
noahfalk
left a comment
There was a problem hiding this comment.
Thanks Zach, this looks nice to me. I'm happy to take this whenever you and Mitchell are satisfied with it.
Console.KeyAvailableandLineRewriter.RewriteConsoleLinethrow exceptions when input and output are redirected.dotnet-trace collectworks with both redirected and has similar code to what's proposed here.