[PATCH] D96751: [clangd] Populate detail field in document symbols

This revision was automatically updated to reflect the committed changes. Closed by commit rG2e851c4172a3: [clangd] Populate detail field in document symbols (authored by lightmelodies, committed by sammccall). Changed prior to commit: https://reviews.llvm.org/D96751?vs=324626&id=324639#toc Re

[PATCH] D96751: [clangd] Populate detail field in document symbols

sammccall added a comment. In D96751#2571579 , @lightmelodies wrote: > In D96751#2571563 , @sammccall wrote: > >> Fantastic, thank you! > > Maybe WithoutVoid.consume_front("void "); Oh wow, yes... sizeof() was rea

[PATCH] D96751: [clangd] Populate detail field in document symbols

lightmelodies added a comment. In D96751#2571563 , @sammccall wrote: > Fantastic, thank you! Maybe WithoutVoid.consume_front("void "); CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96751/new/ https://reviews.llvm.org/D96751 ___

[PATCH] D96751: [clangd] Populate detail field in document symbols

sammccall accepted this revision. sammccall added a comment. Fantastic, thank you! Comment at: clang-tools-extra/clangd/FindSymbols.cpp:189 +if (isa(VD)) { + std::string CCD; + llvm::raw_string_ostream OSC(CCD); I think equivalent and a little saf

[PATCH] D96751: [clangd] Populate detail field in document symbols

lightmelodies added a comment. In D96751#2571346 , @sammccall wrote: > Nice! This looks good to land as-is, I just have some suggestions where we > may want to mark behavior to revisit later, and some places where we could > trim the tests a bit. > > Do

[PATCH] D96751: [clangd] Populate detail field in document symbols

lightmelodies updated this revision to Diff 324626. lightmelodies added a comment. Better printing for C++ constructor and destructor. Remove unnecessary test cases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96751/new/ https://reviews.llvm.org/D96751 Files: clang-tools-extra/cla

[PATCH] D96751: [clangd] Populate detail field in document symbols

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Nice! This looks good to land as-is, I just have some suggestions where we may want to mark behavior to revisit later, and some places where we could trim the tests a bit. Do you have c

[PATCH] D96751: [clangd] Populate detail field in document symbols

lightmelodies updated this revision to Diff 324599. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96751/new/ https://reviews.llvm.org/D96751 Files: clang-tools-extra/clangd/FindSymbols.cpp clang-tools-extra/clangd/test/symbols.test clang-tools-extra/clangd/unittests/FindSymbolsTest

[PATCH] D96751: [clangd] Populate detail field in document symbols

lightmelodies updated this revision to Diff 324582. lightmelodies added a comment. Change behavior of template and add unit tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96751/new/ https://reviews.llvm.org/D96751 Files: clang-tools-extra/

[PATCH] D96751: [clangd] Populate detail field in document symbols

sammccall added a comment. This looks really good. Only thing i'd really think about changing in behavior is dropping the template params. Please go ahead and upload the unittests! (And thanks for including the diff context) Comment at: clang-tools-extra/clangd/FindSymbols.cp

[PATCH] D96751: [clangd] Populate detail field in document symbols

lightmelodies added a comment. Thanks for your response. Yes decl is too verbose in many cases. In fact I have tried another concise version, but I can not decide which one is better. Unit test in FindSymbolsTests.cpp is also done, but I would like upload them after we reach an agreement on wh

[PATCH] D96751: [clangd] Populate detail field in document symbols

lightmelodies updated this revision to Diff 324143. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96751/new/ https://reviews.llvm.org/D96751 Files: clang-tools-extra/clangd/FindSymbols.cpp clang-tools-extra/clangd/test/symbols.test Index: clang

[PATCH] D96751: [clangd] Populate detail field in document symbols

sammccall added a comment. Thanks for doing this! I think this behavior is a great starting point. I suspect printing the decl is more verbose than desired in a lot of cases, at least avoiding repeating the identifier would be nice. e.g. x | int x --> x | int foo | void foo() --> foo | void() b

[PATCH] D96751: [clangd] Populate detail field in document symbols

lightmelodies created this revision. lightmelodies added reviewers: sammccall, kadircet. lightmelodies added a project: clang-tools-extra. Herald added subscribers: usaxena95, arphaman. lightmelodies requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. H