Horton

Re: BUG #16108: Colorization to the output of command-line has unproperly behaviors at Windows platform

Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>

View thread View on postgr.es
From: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>
To: Michail Nikolaev <michail.nikolaev@gmail.com>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: "Tang, Haiying" <tanghy.fnst@cn.fujitsu.com>, Thomas Munro <thomas.munro@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-02-19T16:16:32Z
Lists: pgsql-bugs, pgsql-hackers

Attachments

On Tue, Feb 18, 2020 at 11:39 PM Michail Nikolaev <
michail.nikolaev@gmail.com> wrote:

>
> I have checked the patch source code and it seems to be working. But a
> few moments I want to mention:
>

Thanks for looking into this.


> I think it is not good idea to mix the logic of detecting the fact of
> TTY with enabling of the VT100 mode. Yeah, it seems to be correct for
> current case but a little confusing.
> Maybe is it better to detect terminal using *isatty* and later call
> *enable_vt_mode*?
>

Most of what enable_vt_mode() does is actually detecting the terminal, but
I can see why that is confusing without better comments.


> Also, it seems like if GetConsoleMode returns
> ENABLE_VIRTUAL_TERMINAL_PROCESSING flag already set - we could skip
> SetConsoleMode call (not a big deal of course).
>

Agreed.

The patch about making color by default [1] introduces the
function terminal_supports_color(), that I think is relevant for this
issue. Please find attached a new version based on that idea.

Also, adding Peter to weight on this approach.

[1] https://commitfest.postgresql.org/27/2406/

Regards,

Juan José Santamaría Flecha

Commits

  1. Fix command-line colorization on Windows with VT100-compatible environments