[Lldb-commits] [PATCH] D85145: Use syntax highlighting also in gui mode

[Luboš Luňák via Phabricator via lldb-commits]

llunak added inline comments.


================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:486
+      if (!string.consume_front("[")) {
+        llvm::errs() << "Missing '[' in color escape sequence.\n";
+        continue;
----------------
clayborg wrote:
> So what will happen if we actually get these errors? Will it just print right in the curses view? If so, that doesn't seem optimal.
The idea is that it should ideally never happen, it's technically an internal error of not handling what Highlighter produces. In the discussion above I mentioned that I originally used assert(). So I don't see a real problem, if this actually happens then it shouldn't matter how it shows, and lldb already shows other such messages (which is BTW why Ctrl+L to redraw the screen was one of my first patches).




================
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:4255-4275
+    init_pair(1, COLOR_BLACK, COLOR_BLACK);
+    init_pair(2, COLOR_RED, COLOR_BLACK);
+    init_pair(3, COLOR_GREEN, COLOR_BLACK);
+    init_pair(4, COLOR_YELLOW, COLOR_BLACK);
+    init_pair(5, COLOR_BLUE, COLOR_BLACK);
+    init_pair(6, COLOR_MAGENTA, COLOR_BLACK);
+    init_pair(7, COLOR_CYAN, COLOR_BLACK);
----------------
clayborg wrote:
> Maybe we should make #define for each init_pair to make our code more readable?
> ```
> #define GUI_BLACK_BLACK 1
> #define GUI_RED_BLACK 2
> ...
> init_pair(GUI_BLACK_BLACK, COLOR_BLACK, COLOR_BLACK);
> init_pair(GUI_RED_BLACK, COLOR_BLACK, COLOR_BLACK);
> ...
> ```
> 
> I know it was using magic numbers before.
D85286



Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85145/new/

https://reviews.llvm.org/D85145