Add color to the default thread and frame format.

[JDevlieghere]

Summary

Now that we correctly ignore ASCII escape sequences when colors are disabled (rL362240), I'd like to change the the default frame and thread format to include color in their output, in line with the syntax highlighting that Raphael added a while ago. More specifically, I want to highlight the stop reason and the file + line/column number. With colors disabled, this of course is a no-op.

Please see the following screenshot for example output: https://i.imgur.com/KRZhxSz.png

[teemperor]

+1. I'm (obviously) in favor of this.

[aprantl]

Let the bike-shedding begin!
Feel free to ignore this, but personally, I'd only highlight the filename (or highlight the the line number in a different color).
Feel free to ignore this even more, but we should probably have symbolic color names instead of hardcoding red and such, for users with black-on-white vs. white-on-black (or green-on-black) terminals.

[JDevlieghere]

Adrian's feedback

[clayborg]

Attaching my versions in case you like anything that I did:

settings set frame-format frame #${frame.index}: ${ansi.fg.yellow}${frame.pc}${ansi.normal}{ ${ansi.bold}${ansi.underline}${ansi.fg.cyan}${module.file.basename}${ansi.normal}${ansi.fg.cyan} ${function.name-with-args}{${function.pc-offset}}${ansi.normal}}{ ${ansi.fg.purple}at ${ansi.bold}${ansi.fg.purple}${line.file.basename}:${line.number}}${ansi.normal}\n
settings set thread-format thread #${thread.index}: ${ansi.bold}${ansi.fg.yellow}tid${ansi.normal}${ansi.fg.yellow} = ${thread.id}${ansi.normal}{, ${ansi.fg.yellow}${frame.pc}${ansi.normal}}{ ${ansi.bold}${ansi.underline}${ansi.fg.cyan}${module.file.basename}${ansi.normal}${ansi.fg.cyan} ${function.name-with-args}{${function.pc-offset}}${ansi.normal}}{, ${ansi.bold}${ansi.fg.purple}stop reason${ansi.normal}${ansi.fg.purple} = ${thread.stop-reason}${ansi.normal}}{, ${ansi.bold}${ansi.fg.cyan}name = ${ansi.normal}${ansi.fg.cyan}${thread.name}${ansi.normal}}{, ${ansi.bold}${ansi.fg.green}queue = ${ansi.normal}${ansi.fg.green}${thread.queue}${ansi.normal}}\n{Return value: ${thread.return-value}}

[aprantl]

Sorry, I got more comments :-)

lldb/source/Core/Debugger.cpp

124 ↗

(On Diff #202587)

Thanks, but I was thinking more about something that could be replaced dynamically to support dark and light terminals in the same LLDB binary. Int hits form, this probably doesn't add much because ...

141 ↗

(On Diff #202587)

... because we probably want a macro for the entire "stop reason" string, not just its colors ...

151 ↗

(On Diff #202587)

... and use it here, too.

[JDevlieghere]

Match dwarfdump color scheme

[JDevlieghere]

lldb/source/Core/Debugger.cpp

124 ↗

(On Diff #202587)

As discussed offline, I think it's better to have fixed colors (but not things like black and white) and let the user's color scheme deal with their interpretation.

[JDevlieghere]

Highlight line and column in different color

[davide]

Can you add a screenshot of the final result for everybody?

[JDevlieghere]

In D62743#1544371, @davide wrote:

Can you add a screenshot of the final result for everybody?

Of course! https://i.imgur.com/1cvIbjB.png

[davide]

LGTM!

lldb/trunk/packages/Python/lldbsuite/test/macosx/nslog/TestDarwinNSLogOutput.py

def run_lldb_to_breakpoint(self, exe, source_file, line,
         # Set the breakpoint, and run to it.
         child.sendline('breakpoint set -f %s -l %d' % (source_file, line))
         child.expect_exact(prompt)
         child.sendline('run')
         child.expect_exact(prompt)
 
         # Ensure we stopped at a breakpoint.
         self.runCmd("thread list")
-        self.expect(re.compile(r"stop reason = breakpoint"))
+        self.expect(re.compile(r"stop reason = .*breakpoint"))
 
     def runCmd(self, cmd):
         if self.child:
             self.child.sendline(cmd)
 
     def expect_prompt(self, exactly=True):
         self.expect(self.child_prompt, exactly=exactly)
 

lldb/trunk/source/Core/Debugger.cpp

#define MODULE_WITH_FUNC                                                       \
   "{${frame.no-debug}${function.pc-offset}}}}"
 
 #define MODULE_WITH_FUNC_NO_ARGS                                               \
   "{ "                                                                         \
   "${module.file.basename}{`${function.name-without-args}"                     \
   "{${frame.no-debug}${function.pc-offset}}}}"
 
 #define FILE_AND_LINE                                                          \
-  "{ at ${line.file.basename}:${line.number}{:${line.column}}}"
+  "{ at ${ansi.fg.cyan}${line.file.basename}${ansi.normal}"                    \
+  ":${ansi.fg.yellow}${line.number}${ansi.normal}"                             \
+  "{:${ansi.fg.yellow}${line.column}${ansi.normal}}}"
+
 #define IS_OPTIMIZED "{${function.is-optimized} [opt]}"
 
 #define IS_ARTIFICIAL "{${frame.is-artificial} [artificial]}"
 
 #define DEFAULT_THREAD_FORMAT                                                  \
   "thread #${thread.index}: tid = ${thread.id%tid}"                            \
   "{, ${frame.pc}}" MODULE_WITH_FUNC FILE_AND_LINE                             \
-  "{, name = '${thread.name}'}"                                                \
-  "{, queue = '${thread.queue}'}"                                              \
-  "{, activity = '${thread.info.activity.name}'}"                              \
+  "{, name = ${ansi.fg.green}'${thread.name}'${ansi.normal}}"                  \
+  "{, queue = ${ansi.fg.green}'${thread.queue}'${ansi.normal}}"                \
+  "{, activity = "                                                             \
+  "${ansi.fg.green}'${thread.info.activity.name}'${ansi.normal}}"              \
   "{, ${thread.info.trace_messages} messages}"                                 \
-  "{, stop reason = ${thread.stop-reason}}"                                    \
+  "{, stop reason = ${ansi.fg.red}${thread.stop-reason}${ansi.normal}}"        \
   "{\\nReturn value: ${thread.return-value}}"                                  \
   "{\\nCompleted expression: ${thread.completed-expression}}"                  \
   "\\n"
 
 #define DEFAULT_THREAD_STOP_FORMAT                                             \
   "thread #${thread.index}{, name = '${thread.name}'}"                         \
-  "{, queue = '${thread.queue}'}"                                              \
-  "{, activity = '${thread.info.activity.name}'}"                              \
+  "{, queue = ${ansi.fg.green}'${thread.queue}'${ansi.normal}}"                \
+  "{, activity = "                                                             \
+  "${ansi.fg.green}'${thread.info.activity.name}'${ansi.normal}}"              \
   "{, ${thread.info.trace_messages} messages}"                                 \
-  "{, stop reason = ${thread.stop-reason}}"                                    \
+  "{, stop reason = ${ansi.fg.red}${thread.stop-reason}${ansi.normal}}"        \
   "{\\nReturn value: ${thread.return-value}}"                                  \
   "{\\nCompleted expression: ${thread.completed-expression}}"                  \
   "\\n"
 
 #define DEFAULT_FRAME_FORMAT                                                   \
-  "frame #${frame.index}: ${frame.pc}" MODULE_WITH_FUNC FILE_AND_LINE          \
+  "frame #${frame.index}: "                                                    \
+  "${ansi.fg.yellow}${frame.pc}${ansi.normal}" MODULE_WITH_FUNC FILE_AND_LINE  \
       IS_OPTIMIZED IS_ARTIFICIAL "\\n"
 
 #define DEFAULT_FRAME_FORMAT_NO_ARGS                                           \
-  "frame #${frame.index}: ${frame.pc}" MODULE_WITH_FUNC_NO_ARGS FILE_AND_LINE  \
-      IS_OPTIMIZED IS_ARTIFICIAL "\\n"
+  "frame #${frame.index}: "                                                    \
+  "${ansi.fg.yellow}${frame.pc}${ansi.normal}" MODULE_WITH_FUNC_NO_ARGS        \
+      FILE_AND_LINE IS_OPTIMIZED IS_ARTIFICIAL "\\n"
 
 // Three parts to this disassembly format specification:
 //   1. If this is a new function/symbol (no previous symbol/function), print
 //      dylib`funcname:\n
 //   2. If this is a symbol context change (different from previous
 //   symbol/function), print
 //      dylib`funcname:\n
 //   3. print

lldb/trunk/source/Core/FormatEntity.cpp

case Entry::Type::ParentString: // Only used for
                                   // FormatEntity::Entry::Definition encoding
     return false;
   case Entry::Type::EscapeCode:
     if (exe_ctx) {
       if (Target *target = exe_ctx->GetTargetPtr()) {
         Debugger &debugger = target->GetDebugger();
         if (debugger.GetUseColor()) {
           s.PutCString(entry.string);
-          return true;
         }
       }
     }
-    return false;
+    // Always return true, so colors being disabled is transparent.
+    return true;
 
   case Entry::Type::Root:
     for (const auto &child : entry.children) {
       if (!Format(child, s, sc, exe_ctx, addr, valobj, function_changed,
                   initial_function)) {
         return false; // If any item of root fails, then the formatting fails
       }
     }