WIP: Fix decoration leaking into newline at end of screen buffer

[SteveL-MSFT]

PR Summary

When writing content and the screen buffer scrolls, the decorations also get written to the next line. This is most evident when using a background color and the entire newline contains the background color. Fix is to insert a VT Reset sequence if the terminal supports VT.

This was validated on macOS and Windows. This shows the before and after:

write-color-fix.mp4

PR Context

This fixes part of the issue reported below, but PSCal needs to account for when the screen buffer scrolls and the cursor position has changed. Ideally, it should not be using cursor position to render, but instead pre-calculate all the rows.

Fix #18984

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
• Breaking changes
  • None
  • OR
  • Experimental feature(s) needed
    • Experimental feature name(s):
• User-facing changes
  • Not Applicable
  • OR
  • Documentation needed
    • Issue filed:
• Testing - New and feature
  • N/A or can only be tested interactively
  • OR
  • Make sure you've added a new test if existing tests do not effectively test the code changed
• Tooling
  • I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
  • OR
  • I have considered the user experience from a tooling perspective and opened an issue in the relevant tool repository. This may include:
    • Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode
      (which runs in a different PS Host).
      • Issue filed:
    • Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
      • Issue filed:
    • Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
      • Issue filed:
    • Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).
      • Issue filed:

[daxian-dbw]

This seems to be a by-design behavior in the .NET and terminal perspective. The problem is that we are writing out the newline along with the text, so the restoring of the original background color doesn't happen until after the newline gets written out. When it's at the last line in terminal, that newline triggers a buffer scroll when the background color is still Blue, and that's why the newly added line also has the Blue background color, hence the leak.

It works fine in Windows PowerShell because it writes out newline in a different Write call, where foreground and background are not changed.

So, I think maybe the right fix is to check if we are at the end of terminal before writing in Host, and if so, write out the newline in a separate call without changing the foreground and background.

[SteveL-MSFT]

@daxian-dbw Windows PowerShell doesn't use VT, so the behavior is different for the Console API. If the VT background sequence exists and you write a newline by itself, the entire line after the newline will have the background color. You can see this in zsh/bash:

echo -e "\e[44m1\n2"

So writing a newline in a separate call w/o reseting will result in a whole line with that decoration.

[daxian-dbw]

This behavior is not related to VT decorations, and I can reproduce it with the following C# code as a console application:

// See https://aka.ms/new-console-template for more information

ConsoleColor bg = Console.BackgroundColor;
Console.BackgroundColor = ConsoleColor.Blue;

try {
    Console.WriteLine("Hello, World!");
}
finally {
    Console.BackgroundColor = bg;
}

If I choose to write the newline in a separate call, after restoring the background color, then it will work as expected.
This is what Windows PowerShell does.

// See https://aka.ms/new-console-template for more information

ConsoleColor bg = Console.BackgroundColor;
Console.BackgroundColor = ConsoleColor.Blue;

try {
    Console.Write("Hello, World!");   // <= write out the text, no newline
}
finally {
    Console.BackgroundColor = bg;
}

Console.WriteLine();   // write out the newline after the restoring of background/foreground color

But this doesn't mean this problem doesn't exist in Windows PowerShell, you can reproduce it in Windows PowerShell by running write-Host "hello`n" -BackgroundColor Red at the last line of the terminal window:

[SteveL-MSFT]

I see. I misunderstood what you meant previously. I'll make the change. Note that if you use Write-Output with content including VT, I need the other change to reset anyways.

[daxian-dbw]

@SteveL-MSFT, I'm just thinking aloud here, still not sure what is the right fix yet.
If we care about non-VT terminals, then we should consider do what Windows PowerShell does -- use a separate write for the newline, without changing foreground/background when writing.

As for VT decoration, when writing out "`e[44m1`n2" in PowerShell, it leaks background color even if it's not at the last line of the terminal:

So, the question is -- is that the expected behavior? The user may intentionally omit the Reset sequence, but should be rare.
If that is not the desired behavior, then we should prepend the Reset sequence to the prompt we write out, to avoid the leak (not super sure if that would be the right fix though, let me know what you think).

[SteveL-MSFT]

@daxian-dbw the current fix has the same behavior as in zsh. The first here is the fix, the second zsh, the last current pwsh.

[iSazonov]

I wonder why we reset only for newline.
I believe we should think about every output as atomic operation - set all attributes at start and reset all attributes at end because in the general case we do not know who and what will be output to the console in the next moment.

[SteveL-MSFT]

Even if you Write-Host -nonewline, the formatandoutput will eventually emit a newline before the prompt.

[iSazonov]

I mean some sequential write operations in script (before prompt) .

[pull-request-quantifier-deprecated]

This PR has 18 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

---

Quantification details

Label      : Extra Small
Size       : +12 -6
Percentile : 7.2%

Total files changed: 2

Change summary by file extension:
.cs : +8 -2
.ps1 : +4 -4

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

• Fast and predictable releases to production:
  • Optimal size changes are more likely to be reviewed faster with fewer
    iterations.
  • Similarity in low PR complexity drives similar review times.
• Review quality is likely higher as complexity is lower:
  • Bugs are more likely to be detected.
  • Code inconsistencies are more likely to be detected.
• Knowledge sharing is improved within the participants:
  • Small portions can be assimilated better.
• Better engineering practices are exercised:
  • Solving big problems by dividing them in well contained, smaller problems.
  • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

• Use the PullRequestQuantifier to quantify your PR accurately
  • Create a context profile for your repo using the context generator
  • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
  • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
  • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
• Change your engineering behaviors
  • For PRs that fall outside of the desired spectrum, review the details and check if:
    • Your PR could be split in smaller, self-contained PRs instead
    • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

• One line was added: +1 -0
• One line was deleted: +0 -1
• One line was modified: +1 -1 (git diff doesn't know about modified, it will
  interpret that line like one addition plus one deletion)
• Change percentiles: Change characteristics (addition, deletion, modification)
  of this PR in relation to all other PRs within the repository.

---

Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

[SteveL-MSFT]

I'll hold off on fixing the test issues until we agree on how this should be fixed

[ghost]

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

[tillig]

Has there been any continued discussion on how this should be fixed? This is painful for enhanced prompt tools like oh-my-posh, too, when working in Powershell.

[dkaszews]

I really dislike manipulating ConsoleColor directly, as it has given me trouble when trying to fix #18003 . Can we not switch to OSC sequences from $PSStyle instead?

[SteveL-MSFT]

This is waiting on WG-Interactive to discuss and make a decision

[microsoft-github-policy-service]

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author