[Lldb-commits] [PATCH] D51615: Set Windows console mode to enable support for ansi escape codes

[Zachary Turner via Phabricator via lldb-commits]

zturner accepted this revision.
zturner added a comment.

lgtm after the one suggested change to the pre-processor conditional.



================
Comment at: source/Core/Debugger.cpp:815
+#if defined(_WIN32)
+  HANDLE hConsole = GetStdHandle(STD_OUTPUT_HANDLE);
+  DWORD consoleMode;
----------------
xbolva00 wrote:
> teemperor wrote:
> > A comment here would be nice. E.g. `// Enabling use of ANSI color codes because LLDB is using them to highlight text.
> Alright, will do.
As a future cleanup we should probably add a function to LLVM called `AreAnsiEscapeCodesSupported()`.  Then you could just write:

```
if (llvm::sys::Process::AreAnsiEscapeCodesSupported())
  llvm::sys::Process::UseAnsiEscapeCodes(true);
```

and get rid of the preprocessor comment.  Don't need to do it in this patch though.


================
Comment at: source/Core/Debugger.cpp:809
+
+#if defined(ENABLE_VIRTUAL_TERMINAL_PROCESSING)
+  // Enabling use of ANSI color codes because LLDB is using them to highlight
----------------
Can you also add `defined(_WIN32)` to this condition?


https://reviews.llvm.org/D51615