FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Update the map between console color to VT sequences by daxian-dbw · Pull Request #11891 · PowerShell/PowerShell · GitHub

Update the map between console color to VT sequences - #11891

Merged
Aditya Patwardhan (adityapatwardhan) merged 1 commit into
PowerShell:masterfrom
daxian-dbw:color
Feb 20, 2020
Merged

Update the map between console color to VT sequences#11891
Aditya Patwardhan (adityapatwardhan) merged 1 commit into
PowerShell:masterfrom
daxian-dbw:color

Conversation

Dongbo Wang (daxian-dbw) commented Feb 19, 2020
edited
Loading

Copy link
Copy Markdown
Member

PR Summary

Update the map between console color to VT sequences.
The ConsoleColor to VT Escape Sequences mapping in VTUtils.cs is not accurate, and thus is updated according to https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences#text-formatting

private static readonly Dictionary<ConsoleColor, string> ConsoleColors = new Dictionary<ConsoleColor, string>
{
{ ConsoleColor.Black, "\x1b[2;30m" },
{ ConsoleColor.Gray, "\x1b[2;37m" },
{ ConsoleColor.Red, "\x1b[1;31m" },
{ ConsoleColor.Green, "\x1b[1;32m" },
{ ConsoleColor.Yellow, "\x1b[1;33m" },
{ ConsoleColor.Blue, "\x1b[1;34m" },
{ ConsoleColor.Magenta, "\x1b[1;35m" },
{ ConsoleColor.Cyan, "\x1b[1;36m" },
{ ConsoleColor.White, "\x1b[1;37m" },
{ ConsoleColor.DarkRed, "\x1b[2;31m" },
{ ConsoleColor.DarkGreen, "\x1b[2;32m" },
{ ConsoleColor.DarkYellow, "\x1b[2;33m" },
{ ConsoleColor.DarkBlue, "\x1b[2;34m" },
{ ConsoleColor.DarkMagenta, "\x1b[2;35m" },
{ ConsoleColor.DarkCyan, "\x1b[2;36m" },
{ ConsoleColor.DarkGray, "\x1b[1;30m" },
};

The update is needed as "Bright foreground color" (e.g. bright red \x1b[91m) is rendered differently from bold color (e.g. \x1b[1;31m) in some front-end client, such as the Jupyter Notebook client.
See the 3rd and 4th output below as an example:

PR Checklist

Dongbo Wang (daxian-dbw) added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Feb 19, 2020
Dongbo Wang (daxian-dbw) added this to the GA-consider milestone Feb 19, 2020

Steve Lee (SteveL-MSFT) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Did you check the color rendering on Linux and/or macOS and Windows Terminal?

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Feb 20, 2020

Copy link
Copy Markdown
Member Author

I did check on console host and win terminal, but not Linux and macOS yet. Will do that and update here.

ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Feb 20, 2020

Dongbo Wang (daxian-dbw) commented Feb 20, 2020
edited
Loading

Copy link
Copy Markdown
Member Author

I tested the changes on console host, windows terminal, Linux terminal and macOS terminal, and all look good:

Windows console host

Windows terminal

Linux

macOS

Copy link
Copy Markdown
Member

Steve Lee (@SteveL-MSFT) Please update your review

Copy link
Copy Markdown
Member

LGTM. Let's merge so we can start the build

Copy link
Copy Markdown
Member Author

Since I have tested it on all platforms, I'm OK to merge this PR.

Copy link
Copy Markdown
Member

I am OK to merge this PR.

Copy link
Copy Markdown

🎉v7.0.0-rc.3 has been released which incorporates this pull request.:tada:

Handy links:

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

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL