[PATCH] D137650: [clangd] Implement hover for C++ string literals

[Tom Praschan via Phabricator via cfe-commits]

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