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

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Juan José Santamaría Flecha <juanjo(dot)santamaria(at)gmail(dot)com>
Cc: Michail Nikolaev <michail(dot)nikolaev(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, "Tang, Haiying" <tanghy(dot)fnst(at)cn(dot)fujitsu(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: BUG #16108: Colorization to the output of command-line has unproperly behaviors at Windows platform
Date: 2020-03-02 06:48:42
Message-ID: 20200302064842.GE32059@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Mon, Feb 24, 2020 at 06:56:05PM +0100, Juan José Santamaría Flecha wrote:
> The patch was originaly reported for Windows, but looking into Peter's
> patch, I think this issue affects other systems unless we use stricter
> logic to detect a colorable terminal when using the "auto" option.
> Probably, the way to go is leaving this patch as WIN32 only and thinking
> about a future patch.

It is better to not mix issues. You can actually bump on similar
coloring issues depending on your configuration, with OSX or even
Linux.

> The logic I have seen on new terminals is that VT100 is supported but
> disabled. Would you find clearer? "Attempt to enable VT100 sequence
> processing. If it is not possible consider it as unsupported."
>
> Please find attached a patch addressing these comments.

I was reading the thread for the first time, and got surprised first
with the argument about "always" which gives the possibility to print
incorrect characters even if the environment does not allow coloring.
However, after looking at logging.c, the answer is pretty clear what
always is about as it enforces colorization, so this patch looks
correct to me.

On top of that, and that's a separate issue, I have noticed that we
have exactly zero documentation about PG_COLORS (the plural flavor,
not the singular), but we have code for it in common/logging.c..

Anyway, committed down to 12, after tweaking a few things.
--
Michael

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Arseny Sher 2020-03-02 07:41:54 Re: ERROR: subtransaction logged without previous top-level txn record
Previous Message Amit Kapila 2020-03-02 06:26:33 Re: ERROR: subtransaction logged without previous top-level txn record

Browse pgsql-hackers by date

  From Date Subject
Next Message Arseny Sher 2020-03-02 07:41:54 Re: ERROR: subtransaction logged without previous top-level txn record
Previous Message Amit Kapila 2020-03-02 06:26:33 Re: ERROR: subtransaction logged without previous top-level txn record