[PATCH] D51611: Set console mode when -fansi-escape-codes is enabled

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: lib/Support/Windows/Process.inc:331-337
+  if (enable) {
+    HANDLE hConsole = GetStdHandle(STD_OUTPUT_HANDLE);
+    DWORD consoleMode;
+    GetConsoleMode(hConsole, &consoleMode);
+    consoleMode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
+    SetConsoleMode(hConsole, consoleMode);
+  }
----------------
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.


================
Comment at: lib/Support/Windows/WindowsSupport.h:37-39
+#ifndef ENABLE_VIRTUAL_TERMINAL_PROCESSING
+#define ENABLE_VIRTUAL_TERMINAL_PROCESSING 0x0004
+#endif
----------------
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`.


https://reviews.llvm.org/D51611