[Lldb-commits] [PATCH] D136462: [LLDB] Add color to output text when searching for symbols

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I agree that this change names - print - restore names is going to cause problems. Pavel is right that this should happen in some dump function somewhere.

But, assuming what you've got works to an extent, you should add some testing first. That will allow you to refactor with less risk. You'll be able to reuse a lot of the tests later, so it's not wasted effort and it'll flush out some corner cases you haven't thought of.

For how to test this I would look to the other commands that have colour (`frame` was one I think) and anything interacting with the colour settings.
(if you want a cheeky way to find the tests, delete some of the colouring code and run the test suite - whatever fails is going to be interesting to you)



================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1514
+  for (size_t i = 0; i < positions.size(); ++i) {
+    if (pos >= positions[i].first && pos <= positions[i].second) {
+      return true;
----------------
Doesn't seem like you need to enumerate them, so this could be:
```
for (const auto &p : positions)
   if (pos >= ....)
       return true;
return false;
```
(https://en.cppreference.com/w/cpp/language/range-for)

Or `std::find(..., <lambda>) != positions.end()` but same thing really.

Basically prefer iterators unless `i` is needed for something other than indexing.

If you do need the index but don't want the hassle of handling it yourself, there is an `llvm::enumerate` in `llvm/include/llvm/ADT/STLExtras.h` which acts like Python's `enumerate`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136462