Page MenuHomePhabricator

Add color to the default thread and frame format.
ClosedPublic

Authored by JDevlieghere on May 31 2019, 10:29 AM.

Details

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

Diff Detail

Repository
rL LLVM

Event Timeline

JDevlieghere created this revision.May 31 2019, 10:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 31 2019, 10:29 AM
JDevlieghere edited the summary of this revision. (Show Details)May 31 2019, 10:32 AM

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

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.

Adrian's feedback

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}}

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.

Match dwarfdump color scheme

JDevlieghere marked 4 inline comments as done.Jun 14 2019, 1:47 PM
JDevlieghere added inline comments.
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 marked an inline comment as done.

Highlight line and column in different color

davide added a subscriber: davide.Jun 14 2019, 2:53 PM

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

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

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

davide accepted this revision.Jun 14 2019, 2:55 PM

LGTM!

This revision is now accepted and ready to land.Jun 14 2019, 2:55 PM
aprantl accepted this revision.Jun 14 2019, 3:01 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2019, 12:51 PM