[PATCH] D127082: [clangd] Add Macro Expansion to Hover

[Qingyuan Zheng via Phabricator via cfe-commits]

daiyousei-qz added inline comments.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:1187
 
-  if (!Definition.empty()) {
+  if (!Definition.empty() || !MacroExpansion.empty()) {
     Output.addRuler();
----------------
sammccall wrote:
> Sorry, I think I wasn't clear about what I was asking for...
> Rather than merging `HoverInfo::MacroExpansion` and `HoverInfo::Definition` during `present()`, I'd really like to eliminate `HoverInfo::MacroExpansion` entirely, and have `HoverInfo::Definition` be:
> ```
> #define FOO BAR
> 
> // Expands to
> BAR
> ```
> 
> This isn't completely consistent with the way scope/access are handled, but those are somewhat more generic features and I'd rather avoid adding new HoverInfo members specific to macros.
> 
> (The final presentation does look good, so this is just about the intermediate HoverInfo struct)
I see. Though I'm not sure if putting both of them in a single string will cause problems when doing the format. That is:
```
#define MACRO token token token token token

// Expands to
token token token token token token token token token
```
as a whole will be reformatted by clang-format and I'm not sure if the overall layout could always be preserved. Let me do some expriment tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127082