-
Notifications
You must be signed in to change notification settings - Fork 702
Handle console input/output redirection in CliHostEnvironment #12242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: davidfowl <[email protected]>
@copilot We also don't want ansi colors when the output is redirected. |
Co-authored-by: davidfowl <[email protected]>
Done. ANSI colors are now disabled when console output is redirected. Updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds detection for console stream redirection to the CliHostEnvironment
class, ensuring the CLI behaves appropriately when input/output is piped or redirected. The changes prevent interactive features (prompts, spinners, ANSI colors) from being used inappropriately in non-interactive contexts like automation scripts or log files.
Key Changes:
- Enhanced
DetectInteractiveInput()
to checkConsole.IsInputRedirected
- Enhanced
DetectInteractiveOutput()
to checkConsole.IsOutputRedirected
- Enhanced
DetectAnsiSupport()
to checkConsole.IsOutputRedirected
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/Aspire.Cli/Utils/CliHostEnvironment.cs | Added console redirection detection for input, output, and ANSI support |
tests/Aspire.Cli.Tests/Utils/CliHostEnvironmentTests.cs | Updated test assertions to account for console redirection during test execution |
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12242 Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12242" |
@copilot We need to make this testable so these values need to be passed in as input to the constructor |
Co-authored-by: davidfowl <[email protected]>
Made testable by adding a constructor overload that accepts |
@copilot Any test, testing prompting needs to set isInputRedirected to true. 🔴 Aspire.Cli.Tests.Interaction.ConsoleInteractionServiceTests.PromptForSelectionsAsync_EmptyChoices_ThrowsEmptyChoicesException |
Co-authored-by: davidfowl <[email protected]>
Fixed by updating TestHelpers to explicitly set |
@mitchdenny are you sure you used this PR? |
Summary
Successfully refactored CliHostEnvironment to make console redirection state testable and fixed test failures:
Changes Made:
isInputRedirected
andisOutputRedirected
parametersConsole.IsInputRedirected
andConsole.IsOutputRedirected
valuesDetectInteractiveInput()
,DetectInteractiveOutput()
, andDetectAnsiSupport()
to accept parameters instead of directly accessing ConsoleBenefits:
Testing:
Original prompt
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.