On Tue, Feb 18, 2020 at 11:39 PM Michail Nikolaev < michail.nikol...@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
v3-0001-command-line-colorization-on-windows.patch
Description: Binary data
