[PATCH] D55250: [clangd] Enhance macro hover to see full definition

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 5 00:21:07 PST 2018


hokein added inline comments.


================
Comment at: clangd/XRefs.cpp:567
 
 /// Generate a \p Hover object given the macro \p MacroInf.
+static Hover getHoverContents(MacroDecl Decl, ASTContext &ASTCtx) {
----------------
The comment seems stale.


================
Comment at: clangd/XRefs.cpp:572
+
+  // Try to get the full definition, not just the name
+  SourceLocation StartLoc = Decl.Info->getDefinitionLoc();
----------------
if this is a complicated macro (like `AST_MATCHER`), do we still want to return all the content? they might be less useful than the simple macros.


================
Comment at: clangd/XRefs.cpp:589
+  H.contents.kind = MarkupKind::Markdown;
+  H.contents.value = formatv("```C++\n#define {0}\n```", Definition);
   return H;
----------------
We are now returning `markdown` as default, but some LSP clients might not support `markdown`. I think we should respect the `ClientCapabilities` -- there is a `hover.contentFormat` in `TextDocumentClientCapabilities`. 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55250





More information about the cfe-commits mailing list