[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
CHANGES SINCE LAST ACTION
More information about the cfe-commits