[Lldb-commits] [lldb] Colorize output when searching for symbols in lldb (PR #69422)

[David Spickett via lldb-commits]

DavidSpickett wrote:

> Hello again, David. We've done these. When I fetched the most recent changes in LLVM project today, the ninja check-lldb was okay again. Just FYI, I did not update any dependency.

Nice, there was a change around there recently but I didn't feel like making you update if it wasn't necessary.

> I tried commenting on the more recent suggestions you made that were addressed.

Yes thank you this is looking *very* clean now!

> Should we add some tests to the file with settings set use-color false?

As long as that file has 1 regex search in it, then no. All you need to be sure of is that when colours are disabled, none are emitted. All the multiple match, end of string match, etc, that can stay in the test file you've got.

> Right now, the colorize only works for symbols, i.e., the -s flag. Should this also work for the -F and -n flags? If so, is it better that we add this on this PR or in another patch?

That would be fantastic, but you should concentrate on symbols for this PR. It's enough to stand on its own as a change.

Finally, one of you asked about where `Address::DumpName` should live. Now the code is cleaned up, I see more clearly what you mean.

Looking at the signature `Address::DumpName(Stream *strm, llvm::StringRef text, const char *pattern)` you could imagine this as a member of Stream instead. Perhaps `.PutCStringHighlighted(`? Then you can do `strm.PutCStringHighlighted(text, pattern)`.

Maybe having "color" in the name is good. `PutCStringColorHighlighted`?

Then 1. You can have standalone tests and do less testing in the shell tests and 2. If you do want to do files and function highlighting, it's in a nice central place.

https://github.com/llvm/llvm-project/pull/69422