[PATCH] D61497: [clangd] Introduce a structured hover response

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/XRefs.h:52
+struct HoverInfo {
+  LocatedSymbol Sym;
+  /// Name of the context containing the symbol.
----------------
sammccall wrote:
> I'm not sure about reuse of LocatedSymbol - do we want to commit to returning decl/def locations?
> Name might be enough here.
It might be nice to provide editors with enough info to jump to definition(it was brought up during last meeting). But happy to reduce it to just name.


================
Comment at: clang-tools-extra/clangd/XRefs.h:55
+  std::string ContainerName;
+  index::SymbolInfo SI;
+  /// Includes all the qualifiers.
----------------
sammccall wrote:
> SymbolInfo is a bit of a mess. Maybe we just want SymbolInfo::Kind? (LSP SymbolKind is tempting but loses a lot of info for C++).
> 
> I do think we'll need to extend SymbolInfo::Kind or eventually use our own enum, e.g. lambdas may need their own kind (they're variables, but don't have a printable type and need to be displayed more like functions)
As you mentioned we might need to extend LSP's enum, but switching to it anyway. Currently it doesn't support "macro" kind exactly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497