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

On Tue, Feb 18, 2020 at 11:39 PM Michail Nikolaev <
michail(dot)nikolaev(at)gmail(dot)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