Skip to content
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

faint color attribute "leaks" to the whole command line #1888

Closed
LuanVSO opened this issue Oct 17, 2020 · 16 comments · Fixed by #2925
Closed

faint color attribute "leaks" to the whole command line #1888

LuanVSO opened this issue Oct 17, 2020 · 16 comments · Fixed by #2925
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Resolution-Fixed

Comments

@LuanVSO
Copy link

LuanVSO commented Oct 17, 2020

Environment

PS version: 7.0.3
PSReadline version: 2.1.0-rc1
os: 10.0.19041.561 (WinBuild.160101.0800)
PS file version: 7.0.3.0
HostName: ConsoleHost (Windows Terminal)
BufferWidth: 110
BufferHeight: 30

Exception report

Steps to reproduce

run

Set-PSReadLineOption -Colors  @{Command = "`e[2;93m"}

and then type a command line with some parameters

Expected behavior

only the command gets the faint color

Actual behavior

the faint color attribute is never reset which results in the hole command line being faint

@ghost ghost added the Needs-Triage 🔍 It's a new issue that core contributor team needs to triage. label Oct 17, 2020
@eabase
Copy link

eabase commented Oct 17, 2020

@daxian-dbw
Copy link
Member

@LuanVSO Thanks for reporting the issue. Font effects are not reset properly for most of the token colors, such as the CommentColor.

@daxian-dbw daxian-dbw added Issue-Bug It either shouldn't be doing this or needs an investigation. and removed Needs-Triage 🔍 It's a new issue that core contributor team needs to triage. labels Oct 30, 2020
@eabase
Copy link

eabase commented Oct 30, 2020

maybe related to #1110

@DEberhardt
Copy link

DEberhardt commented Nov 11, 2020

Have found a similar bleed-through effect by running: Get-PSReadLineOption | FL *
image

This can be "repaired" by running Get-PSReadLineOption
image

PowerShell v7.1.0 (release version), PSReadline v2.1.0-rc1 (that came with it) - had some issues installing just now, so couldn't validate whether this also happens on the latested prerelease.

The interesting thing is where this occurs (maybe that gives us a clue on how to fix?)
DefaultTokenColoris the first one, though it is not visible when running the command as-is. After the CommentColor, instead of displaying the colour as a value to the parmeter, it colours the parameterName. Which is very neat (but I guess not intended^^).

When re-running the command in its native form, this rectifies itself after CommandColor...

Don't know if this should be tracked as a separate issue or be handled here.

@DEberhardt
Copy link

Update:
Also happens on PowerShell v5.1 with v2.2.0-beta2

interesting nugget I just learnt: Without the Module loaded, PowerShell knows no history! 😱 It is like it not only goes colour-blind but also has amnesia 😄

@daxian-dbw
Copy link
Member

@DEberhardt Thanks for reporting the issue. I took a look and it turns out the bleed-through problem for the repro Get-PSReadLineOption | FL * is an issue in the PowerShell formatting system. The PowerShell formatting system obviously doesn't expect the value of a property contains VT sequences. Maybe it's by-design, because that's rare and Get-PSReadLineOption already has a custom formatting to handle it. Please open an issue in the PowerShell repo if you want to.

@eabase
Copy link

eabase commented Nov 18, 2020

... is an issue in the PowerShell formatting system. The PowerShell formatting system obviously doesn't expect the value of a property contains VT sequences. Maybe it's by-design, because that's rare and Get-PSReadLineOption already has a custom formatting to handle it. Please open an issue in the PowerShell repo if you want to.

@daxian-dbw
There is a problem with that, because those guys are repeatedly trying to send everyone to this repo, whenever there is a color problem. I've filed numerous issues there in the past, and usually end up here. Perhaps you would have more leverage if you could file this issue with them.

@daxian-dbw
Copy link
Member

@eabase Got it, and sorry for the not-so-good experience. I will file an issue in the PS repo about this, but I guess this particular issue will be considered "won't fix" given what I explained above.

@eabase
Copy link

eabase commented Nov 19, 2020

@daxian-dbw

I guess this particular issue will be considered "won't fix"

Hmm, too bad. Because a few months ago I was checking the PS code for another color problem, and I discovered they are using RegEx's to find and filter ANSI codes... (Sorry, i don't recall exactly where i found that.) But I do know it's pretty bad practice, since its extremely easy to make mistakes with overly eager/hungry regex's, which almost always break when people start using different character sets and terminal coding, such s UTF-8/16/32 etc.

@daxian-dbw
Copy link
Member

I discovered they are using RegEx's to find and filter ANSI codes...

@eabase It would be great if you can find where that is and why the code is doing so. PowerShell formatting code shouldn't care about ANSI sequences as I understand, but I may be wrong.

@SeeminglyScience
Copy link
Contributor

SeeminglyScience commented Nov 20, 2020

@eabase It would be great if you can find where that is and why the code is doing so. PowerShell formatting code shouldn't care about ANSI sequences as I understand, but I may be wrong.

ConsoleControl.LengthInBufferCells checks for escape sequences to determine length on screen. ConsoleHostRawUserInterface.LengthInBufferCells calls this method, which is called by host based line-output renderers (e.g. the default renderer and Out-Host, but not Out-String or similar).

The implementation doesn't use regex though.

@eabase
Copy link

eabase commented Nov 20, 2020

@daxian-dbw

It would be great if you can find where that is and why the code is doing so

Sorry, I don't recall where I found it. Maybe my memory fail me, because I found my own comment here:

If you look at here at the BackgroundColorMap , there is definitely something fishy with the Blue and perhaps the regex handling of AnsiReset = "\x1b[0m"; which can be shorter (as \x1b[m) than the minimum 4 as mentioned in code comments.

In addition, please check this assumption:

  • // For texts with VT sequences, reset all attributes if not already.
    // We only handle the first 2 elements because that's all will potentially be used.
    int minLength = _promptText.Length == 1 ? 1 : 2;
    for (int i = 0; i < minLength; i ++)
    {
    var text = _promptText[i];
    if (text.Contains('\x1b') && !text.EndsWith(VTColorUtils.AnsiReset, StringComparison.Ordinal))
    {
    _promptText[i] = string.Concat(text, VTColorUtils.AnsiReset);
    }

@daxian-dbw
Copy link
Member

That assumption is correct. The first 2 elements referes to the first 2 elements in the _promptText array.

@eabase
Copy link

eabase commented May 2, 2021

That assumption is correct. The first 2 elements referes to the first 2 elements in the _promptText array.

Are you sure? What happens if the _promptText array contain UTF-16 chars? (Could that even happen?)

@daxian-dbw
Copy link
Member

@eabase Sorry for the loooong silence here 😓 The _promptText is a string array, and .NET string is using UTF-16 character encoding.

@ghost ghost added the In-PR A PR is opened targeting the issue label Oct 21, 2021
@ghost ghost added Resolution-Fixed and removed In-PR A PR is opened targeting the issue labels Oct 21, 2021
@ghost
Copy link

ghost commented Oct 28, 2021

🎉 This issue was addressed in 2925, which has now been successfully released in v2.2.0-beta4. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Resolution-Fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants