This is an archive of the discontinued LLVM Phabricator instance.

Set console mode when -fansi-escape-codes is enabled
ClosedPublic

Authored by xbolva00 on Sep 4 2018, 1:59 AM.

Details

Summary

Windows console now supports supports ANSI escape codes, but we need to enable it using SetConsoleMode with ENABLE_VIRTUAL_TERMINAL_PROCESSING flag.

Fixes https://bugs.llvm.org/show_bug.cgi?id=38817

Tested on Windows 10, screenshot:
https://i.imgur.com/bqYq0Uy.png

Diff Detail

Event Timeline

xbolva00 created this revision.Sep 4 2018, 1:59 AM
xbolva00 updated this revision to Diff 163778.Sep 4 2018, 3:44 AM
  • Addressed comments from PR38817
zturner added inline comments.Sep 4 2018, 10:58 AM
lib/Support/Windows/Process.inc
331–337

I was going to comment how this looks all wrong because it only operates on stdout (e.g. not stderr) and doesn't seem to notice the case when stdout is not a terminal. But I looked the rest of the code in this file and it's all broken as well. Anyway, I would write this as:

#if defined(ENABLE_VIRTUAL_TERMINAL_PROCESSING)
  if (enable) {
    HANDLE hConsole = GetStdHandle(STD_OUTPUT_HANDLE);
    DWORD consoleMode;
    GetConsoleMode(hConsole, &consoleMode);
    consoleMode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
    SetConsoleMode(hConsole, consoleMode);
  }
#endif

If they aren't using the appropriate Windows SDK, they don't get the feature.

lib/Support/Windows/WindowsSupport.h
37–39 ↗(On Diff #163778)

If someone wants a copy of LLDB that supports features specific to Windows 10, the supported workflow is that they should be building using the Windows SDK that has those features. So I'd rather remove this, and guard the code that uses this flag with an #ifdef.

zturner added inline comments.Sep 4 2018, 10:59 AM
lib/Support/Windows/Process.inc
332

We don't use hungarian notation. Also all variables begin with capital letters.

HANDLE Console = GetStdHandle(STD_OUTPUT_HANDLE);
DWORD Mode;
GetConsoleMode(Console, &Mode);
Mode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
SetConsoleMode(Console, Mode).
xbolva00 updated this revision to Diff 163879.Sep 4 2018, 11:33 AM
xbolva00 marked 2 inline comments as done.Sep 4 2018, 11:35 AM
zturner accepted this revision.Sep 4 2018, 11:37 AM
This revision is now accepted and ready to land.Sep 4 2018, 11:37 AM
This revision was automatically updated to reflect the committed changes.