8000 Minimize writing ANSI escape sequences in PSReadline by lzybkr · Pull Request #4110 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@lzybkr
8000 Copy link
Contributor
@lzybkr lzybkr commented Jun 26, 2017

Instead of changing Console.*groundColor on every character, only set
the color when the color changes - this avoids writing out ANSI escape
sequences after every character.

Fixes #4075

Instead of changing Console.*groundColor on every character, only set
the color when the color changes - this avoids writing out ANSI escape
sequences after every character.

Fixes PowerShell#4075
Console.BackgroundColor = buffer[i].BackgroundColor;
var nextForegroundColor = buffer[i].ForegroundColor;
var nextBackgroundColor = buffer[i].BackgroundColor;
if (lastForegroundColor != nextForegroundColor)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nitpick, but it seems that if (nextForegroundColor != lastForegroundColor) is slightly more correct improving readability

8000

{
Console.ForegroundColor = lastForegroundColor = nextForegroundColor;
}
if (lastBackgroundColor != nextBackgroundColor)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above


Console.BackgroundColor = backgroundColor;
Console.ForegroundColor = foregroundColor;
if (backgroundColor != lastBackgroundColor)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty much everywhere else you have foreground first, then background. Another nitpick, but I would reorder these two if statements

@lzybkr lzybkr merged commit ff56097 into PowerShell:master Jun 28, 2017
@lzybkr lzybkr deleted the psreadline_less_ansi branch June 28, 2017 06:22
@mirichmo mirichmo added this to the 6.0.0-beta.4 milestone Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

0