tom-anders added inline comments.
================
Comment at: clang-tools-extra/clangd/Hover.cpp:785
+
+ // TODO: Show string literal's contents
+ HI.Name = "String Literal";
----------------
Is it really useful to show the contents inside the Hover? You already see the
contents of the string literal anyway, so I don't think this adds any value
================
Comment at: clang-tools-extra/clangd/Hover.cpp:788
+
+ // In C++ string literals have `const` qualifier, but in C it don't.
+ std::string Qualifier;
----------------
(nit)
================
Comment at: clang-tools-extra/clangd/Hover.cpp:814
+ } else {
+ // TODO: For other languages
+ return llvm::None;
----------------
Hmm so what's stopping us from adding support for C here?
================
Comment at: clang-tools-extra/clangd/Hover.cpp:820
+ Type.Type =
+ Qualifier + CharType + "[" + std::to_string(SL->getLength() + 1) + "]";
+ HI.Type = Type;
----------------
I think you should prefer `llvm::formatv` here
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137650/new/
https://reviews.llvm.org/D137650
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
