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

Colorize vswhere output #244

Merged
merged 6 commits into from
Sep 15, 2021
Merged

Colorize vswhere output #244

merged 6 commits into from
Sep 15, 2021

Conversation

heaths
Copy link
Member

@heaths heaths commented Sep 13, 2021

Print instances to the terminal using colors from Visual Studio Code's Dark+ theme suitable for dark-themed terminals (common). A -nocolor switch is added in case it's necessary, but piping stdout to other programs or even using PowerShell accelerators like [xml] should work without -nocolor since vswhere is not printing directly to the terminal.

heaths and others added 3 commits September 12, 2021 22:13
Fails in PowerShell if you do something like `[xml](vswhere -format xml)` if color is enabled, so had to add -color switch. Not ideal since it otherwise disables color when piped, but still easier to read if colorized explicitly.
@heaths
Copy link
Member Author

heaths commented Sep 13, 2021

Colorizing the output is something I've been wanting to do for a while, and since I did it with the Go binding's test app I decided to take a crack at it now.

image

image

image

Initially color was enabled by default if stdout was to a terminal and the terminal supported VT100 sequences (which the conhost does in Windows 7 and newer). This works even when output is piped since stdout is no longer to a terminal, and is common in colorized console apps.

Unfortunately, this fails in PowerShell like so:

$instances = [xml](vswhere -format xml -utf8)

The output is not piped, so the escape sequences are present in the output, which are not valid XML. As such, I added the -color switch. It's not idea, but as you can see above makes it much easier to read output, especially in the default formatter.

Maybe someday we bump the major version to 3 and enable colors by default as well as UTF8 output, since that would've been a breaking change as well but probably something we should've done originally when JSON was added.

The colors come from Visual Studio Code's Dark+ theme and should be suitable for the vast majority of terminals, which are generally dark.

@heaths heaths changed the title colorize Colorize vswhere output with new -color switch Sep 13, 2021
@heaths
Copy link
Member Author

heaths commented Sep 13, 2021

Windows PowerShell i.e. powershell.exe defaults to a blue background, but these colors are still quite legible:

image

@heaths
Copy link
Member Author

heaths commented Sep 13, 2021

Per offline discussion, going back to color by default but with a -nocolor escape hatch. I'm not sure what happened such that the [xml] accelerator failed when it ran into the first \033 aka \1xb character, but I can't repro it now after doing a completely rebuild of both Debug and Release configurations. I had been running into sporadic incremental build failures with Debug that have always been an issue with this project's .vcxproj references, so that might've been a factor.

The integration tests that run in a Docker container make heavy use of this accelerator so we'll have a better sense for whether it's an issue in an isolated environment.

@heaths heaths changed the title Colorize vswhere output with new -color switch Colorize vswhere output Sep 13, 2021
Copy link
Member

@tydunkel tydunkel left a comment

Choose a reason for hiding this comment

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

LGTM

src/vswhere.lib/CommandArgs.h Outdated Show resolved Hide resolved
src/vswhere.lib/JsonFormatter.cpp Show resolved Hide resolved
@heaths
Copy link
Member Author

heaths commented Sep 13, 2021

There's one "problem" I want to try to sort out first. Doesn't hurt anything, but not fantastic. Seems I need to write and flush to stdout at least once before color attributes take effect. You don't notice it with JSON because of the [ array start not being colored (like the dark+ theme I mimicked) nor the default text formatter because of the logo (JSON, XML, and value formatters don't use the logo, though value doesn't use color anyway because it's pointless). XML and text with -nologo are where you see it.

Perhaps @DHowett has some idea how to resolve the issue. I'm anxious about writing and clearing a single block character because of how that how that might affect output but, then again, that's only when stdout isn't redirected and I'm writing directly to the terminal buffer.

Could simply call Initialize wherever it needs to be called, but would rather be explicit.
@heaths heaths merged commit 4df5b7a into microsoft:develop Sep 15, 2021
@heaths heaths deleted the colorize branch September 15, 2021 02:37
heaths added a commit that referenced this pull request Feb 25, 2022
* Update docker image (#240)

* Update microbuild pool to VS2019

* Use correct pool name

* Update docker images

* Colorize vswhere output (#244)

* Refactor formatters to consolidate arguments

* Colorize output by default unless piped

* Opt into color and add tests

Fails in PowerShell if you do something like `[xml](vswhere -format xml)` if color is enabled, so had to add -color switch. Not ideal since it otherwise disables color when piped, but still easier to read if colorized explicitly.

* Enable colors by default, add -nocolor switch

* Resolve PR feedback

* Ensure console is initialized

Could simply call Initialize wherever it needs to be called, but would rather be explicit.

* Update AzDO build to use new 1ES pool

* Add compliance build

* Update targeting to Windows 7

* Enable ControlFlowGuard

* Fix Prefast errors in source

* Fix test prefast issues

* Update to latest cached docker image

* Suppress individual prefast warnings for fallthrough cases

* FIxup comments for variants

* Upgrade agent to Server 2019 with VS2019 (#260)

Co-authored-by: Laurel Williams <20650063+lwillia@users.noreply.github.com>
Co-authored-by: Davi Paulino <dalimapa@microsoft.com>
Co-authored-by: Davi Paulino <davilimap@gmail.com>
Co-authored-by: Tyler Dunkel <40210514+tydunkel@users.noreply.github.com>
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.

2 participants