From: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>
To: Michail Nikolaev <michail.nikolaev@gmail.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Tang, Haiying <tanghy.fnst@cn.fujitsu.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: BUG #16108: Colorization to the output of command-line has unproperly behaviors at Windows platform
Date: Mon, 24 Feb 2020 18:56:05 +0100
Message-ID: <CAC+AXB0LSzuE7veUrw0-N=cxKEV_hfZTT_RaPMfMeRY3co-RQA@mail.gmail.com> (raw)
In-Reply-To: <CANtu0oic8bkGZ6uSA6NPp=dKCwpBd22yk2tJmUKCX5PVVAzMDA@mail.gmail.com>
References: <16108-134692e97146b7bc@postgresql.org>
<CA+hUKG+Em5CO27qk4PC9syK+p2n0a0ULP_Vm3OqpvdXGQTn0=A@mail.gmail.com>
<CAC+AXB2LDsO_vB-yNW8b9VBW=onGPwJiUuxAQ23TDcm8JbJ8NQ@mail.gmail.com>
<43AA0560298D1A4893896FDC52840EA1D9C74F7D@G08CNEXMBPEKD03.g08.fujitsu.local>
<CAC+AXB1yZXb_=DzRoPV8jY0cUX5EFYLHtmOgu7is3d3M31GppQ@mail.gmail.com>
<CANtu0oj2z_t9d5SrTLqgZ_mOoiiAheTFp3spP7urUFiuK90=uA@mail.gmail.com>
<CAC+AXB3cTZKR8ry-T6-ui0qLMij+K_auxO9Lq9dHy1DBj+HDHQ@mail.gmail.com>
<CANtu0oic8bkGZ6uSA6NPp=dKCwpBd22yk2tJmUKCX5PVVAzMDA@mail.gmail.com>
On Sat, Feb 22, 2020 at 9:09 PM Michail Nikolaev <michail.nikolaev@gmail.com>
wrote:
>
> I am not sure it is good idea to mix both patches because it adds some
> confusion and makes it harder to merge each.
> Maybe is it better to update current patch the way to reuse some
> function later in [1]?
>
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.
> Also, regarding comment
> > It is disabled by default, so it must be enabled to use color outpout.
>
> It is not true for new terminal, for example. Maybe it is better to
> rephrase it to something like: "Check if TV100 support if enabled and
> attempt to enable if not".
>
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.
Regards,
Juan José Santamaría Flecha
Attachments:v4-0001-command-line-colorization-on-windows.patch (application/octet-stream, 1.6K) downloaddiff --git a/src/common/logging.c b/src/common/logging.cindex c78ae79..d59ec88 100644--- a/src/common/logging.c+++ b/src/common/logging.c@@ -32,6 +32,35 @@ static const char *sgr_locus = NULL;
#define ANSI_ESCAPE_FMT "\x1b[%sm"
#define ANSI_ESCAPE_RESET "\x1b[0m"
+#ifdef WIN32+#ifndef ENABLE_VIRTUAL_TERMINAL_PROCESSING+#define ENABLE_VIRTUAL_TERMINAL_PROCESSING 0x0004+#endif++/*+ * Attempt to enable VT100 sequence processing.+ * If it is not possible consider it as unsupported.+ */+static bool+enable_vt_processing(void)+{+ /* Check stderr */+ HANDLE hOut = GetStdHandle(STD_ERROR_HANDLE);+ DWORD dwMode = 0;++ if (hOut == INVALID_HANDLE_VALUE)+ return false;+ if (!GetConsoleMode(hOut, &dwMode))+ return false;+ if (dwMode & ENABLE_VIRTUAL_TERMINAL_PROCESSING)+ return true;+ dwMode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;+ if (!SetConsoleMode(hOut, dwMode))+ return false;+ return true;+}+#endif /* WIN32 */+
/*
* This should be called before any output happens.
*/
@@ -40,6 +69,10 @@ pg_logging_init(const char *argv0)
{
const char *pg_color_env = getenv("PG_COLOR");
bool log_color = false;
+ bool color_terminal = isatty(fileno(stderr));+#ifdef WIN32+ color_terminal = color_terminal && enable_vt_processing();+#endif
/* usually the default, but not on Windows */
setvbuf(stderr, NULL, _IONBF, 0);
@@ -50,7 +83,7 @@ pg_logging_init(const char *argv0)
if (pg_color_env)
{
if (strcmp(pg_color_env, "always") == 0 ||
- (strcmp(pg_color_env, "auto") == 0 && isatty(fileno(stderr))))+ (strcmp(pg_color_env, "auto") == 0 && color_terminal))
log_color = true;
}
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: juanjo.santamaria@gmail.com, michail.nikolaev@gmail.com, peter.eisentraut@2ndquadrant.com, tanghy.fnst@cn.fujitsu.com, thomas.munro@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: BUG #16108: Colorization to the output of command-line has unproperly behaviors at Windows platform
In-Reply-To: <CAC+AXB0LSzuE7veUrw0-N=cxKEV_hfZTT_RaPMfMeRY3co-RQA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox