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

Nathan Ridge via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 20 00:51:44 PDT 2022

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?

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list