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

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Thank you for working on this! I've been using clangd with this patch applied for a week or so, and it seems to be working well, I haven't run into any issues.

Could you please upload a patch with context, to make it easier to review? (Currently, above each hunk it says "Context not available", which makes it harder to find what code is being changed.)

> Should we present macro expansion before definition in the hover card?

Yes, I think putting the expansion first would make sense, because it's more relevant (since it's specific to the invocation).

> Should we truncate/ignore macro expansion if it's too long? For performance and presentation reason, it might not be a good idea to expand pages worth of tokens in hover card. If so, what's the preferred threshold?

I think a good client will manage longer content by e.g. making the hover scrollable, so we probably don't need a very restrictive limit, but I guess should have //some// limit. Maybe something like 100 lines?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127082