Repository navigation
Stop writing DECCKM sequences when reading commands from stdin - #28185
Open
candygoogy (candygoogy) wants to merge 2 commits into
Open
candygoogy (candygoogy) wants to merge 2 commits into
candygoogy (candygoogy) wants to merge 2 commits into
Conversation
On Unix, the prompt loop turns DECCKM on before reading a line and off before running it, so that .NET gets VT input for the cursor keys. When the commands come from stdin (-Command -, -File -, or redirected input), the lines are read from stdin and the cursor keys are never used, so the sequences only ended up in the output, for example in a redirected file. Skip both writes when reading from stdin, as the exit path already does.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new test parameter violates the repository’s PowerShell parameter-naming convention.
1 open finding
What changed in this PR
Prevents Unix console hosts from emitting DECCKM sequences when commands are read from redirected stdin.
Changes:
- Gates DECCKM toggling on
ReadFromStdin. - Adds Pester coverage for four stdin invocation forms.
| File | Description |
|---|---|
ConsoleHost.cs |
Suppresses DECCKM for redirected stdin. |
ConsoleHost.Tests.ps1 |
Tests redirected-stdin output for escape characters. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+664
to
+671
| It "Does not write VT sequences to the output: <Arguments>" -TestCases @( | ||
| @{Arguments = "-noprofile -c -"} | ||
| @{Arguments = "-noprofile -Command -"} | ||
| @{Arguments = "-noprofile -File -"} | ||
| @{Arguments = "-noprofile"} | ||
| ) { | ||
| param($Arguments) | ||
| $si = NewProcessStartInfo $Arguments -RedirectStdIn |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
When pwsh reads its commands from stdin (
pwsh -c -,pwsh -Command -,pwsh -File -, or plainpwshwith redirected input) on Linux or macOS, it no longer writes theESC[?1h/ESC[?1l(DECCKM) sequences into the output.
The console host turns DECCKM on before reading each line and off before running it, so .NET gets
VT input for the cursor keys (added in #14943). That only matters when a person is typing at a
terminal. When commands come from stdin, the input is read as plain lines and the cursor keys are
never used, so these writes ended up in the output, for example in a file when stdout is redirected.
The change adds
!ui.ReadFromStdinto the two checks in the prompt loop inConsoleHost.cs. Theexit path already skips DECCKM in this case, because it checks
IsInteractive, which is falsewhen reading from stdin. The interactive behavior from #14943 doesn't change. The code is inside
#if UNIX, so Windows was never affected.PR Context
Fix #26631
The Engine WG agreed that the host should not write these VT sequences when it processes stdin as
commands.
Before (Linux,
printf 'echo hi\necho there\n' | pwsh -NoProfile -c - > out.txt):After:
Testing:
test/powershell/Host/ConsoleHost.Tests.ps1: it runs pwsh with commands on stdin andredirected output (
-c -,-Command -,-File -and no arguments) and checks that the outputhas no ESC character. The test removes
NO_COLORand setsTERM, becauseNO_COLORmakes thehost strip these sequences. This is why the existing redirected-stdin tests (which set
NO_COLOR)didn't catch the bug.
The existing "Redirected standard input for 'interactive' use" tests pass with and without the fix.
I didn't run the rest of
ConsoleHost.Tests.ps1on Linux.ConsoleHost.Tests.ps1gives the same results with and without the fix.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header