[Lldb-commits] [PATCH] D134516: [lldb] Improve display of absolute symbol lookup

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added inline comments.


================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1552
           strm.IndentMore();
+          strm.Indent("    Name: ");
+          strm.PutCString(symbol->GetDisplayName().GetStringRef());
----------------
DavidSpickett wrote:
> Could you use the stream's indent here instead of `"     Foo:"`? You can pass a number of spaces to pad `void IndentMore(unsigned amount = 2);`.
I was just following the pre-existing code below and also in `DumpAddress`. I do have some issues with the output of the lookup command when there are multiple results -- there is no separation between different results which makes the output a bit hard to parse (and is why I made a semi-related patch in D134111), perhaps we can try to improve it more in a separate change.


================
Comment at: lldb/test/Shell/SymbolFile/absolute-symbol.test:4
 # CHECK: 1 symbols match 'absolute_symbol'
+# CHECK:   Name: absolute_symbol
 # CHECK:   Value: 0x0000000012345678
----------------
DavidSpickett wrote:
> This is my ignorance talking, but isn't it redundant to print the name given that it's shown in the `match` line?
> 
> Perhaps that lookup isn't exact and mangling or partial matches can lead to multiple matches in which case printing the line is very useful.
The lookup command can use regex filters which does usually return multiple matches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134516