This is an archive of the discontinued LLVM Phabricator instance.

Differential D51611

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

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

Details

Reviewers
zturner
chandlerc
Commits
rGecf0c55b6352: Set console mode when -fansi-escape-codes is enabled
rL341396: Set console mode when -fansi-escape-codes is enabled
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

Repository
rL LLVM

Event Timeline

xbolva00 created this revision.Sep 4 2018, 1:59 AM
Herald added a subscriber: llvm-commits. · View Herald TranscriptSep 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 ↗(On Diff #163778)

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 ↗(On Diff #163778)

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
Closed by commit rL341396: Set console mode when -fansi-escape-codes is enabled (authored by xbolva00). · Explain WhySep 4 2018, 12:24 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
trunk/
lib/
Support/
Windows/
9 lines

Diff 163885

llvm/trunk/lib/Support/Windows/Process.inc

Show First 20 Lines • Show All 322 Lines • ▼ Show 20 Lines
} }
bool Process::StandardErrHasColors() { bool Process::StandardErrHasColors() {
return FileDescriptorHasColors(2); return FileDescriptorHasColors(2);
} }
static bool UseANSI = false; static bool UseANSI = false;
void Process::UseANSIEscapeCodes(bool enable) { void Process::UseANSIEscapeCodes(bool enable) {
#if defined(ENABLE_VIRTUAL_TERMINAL_PROCESSING)
if (enable) {
HANDLE Console = GetStdHandle(STD_OUTPUT_HANDLE);
DWORD Mode;
GetConsoleMode(Console, &Mode);
Mode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
SetConsoleMode(Console, Mode);
}
#endif
UseANSI = enable; UseANSI = enable;
} }
namespace { namespace {
class DefaultColors class DefaultColors
{ {
private: private:
WORD defaultColor; WORD defaultColor;
▲ Show 20 Lines • Show All 125 Lines • Show Last 20 Lines