[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
kadir çetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 22 08:08:22 PDT 2024
kadircet wrote:
So my 2cents;
> 1. I know a [previous comment on the bug](https://github.com/clangd/clangd/issues/959#issuecomment-998927030) stated "We don't show bodies of classes/enums/functions etc by policy", but can we consider changing this policy in the more limited case of public struct fields and enum members? Like I mentioned in [this comment](https://github.com/clangd/clangd/issues/959#issuecomment-2068307365), the usefulness/verbosity tradeoff might be different in this subset of cases. Also cc @colin-grant-work in case you'd like to weigh in on this further.
I feel like a more focused approach could still be preferred. E.g. what about showing this kind of "summary" only when hovering over the declaration of the symbol, rather than references? (similar to the way we show size/alignment info). As I believe showing `struct Foo {}` in hover response, when the cursor is already at a struct's definition, isn't really useful.
For enabling it more generally, I still have some hesitations, as I am not sure how useful that interaction is in general. As not all editors let you interact with hover cards, and just seeing a bunch of field names, without any extra info, sounds suboptimal to me. I'd still prefer going over those fields via `Foo{}.^` code completion, which shows all the "public" info with extra context like "documentation".
> 2. If we're willing to make this change, do we want it unconditionally, or behind a new config option in the `Hover` section? My personal opinion is that even in the case of a struct/enum with many members this is not particularly bothersome and would lean towards making it unconditional, but I am of course happy to put it behind a config option if that helps build a consensus for this.
As you might've sensed from the previous response, i'd definitely prefer a config option. As most terminal-based editors don't really have "rich" hovercard support, and even if we set the default to "verbose", i'd like to have a way for such people to keep using hover cards without definition eating up the whole screen estate.
> 3. Regarding the implementation approach, is it fine to add a flag to `PrintingPolicy` (which is a clang utility class used in a variety of places) for a clangd-specific use case like this? I did it this way because the alternative seemed to involve duplicating a bunch of code related to decl-printing, but I'm happy to explore alternatives if this is an issue.
I feel like "print only public fields" is too specific for clangd use case, and probably won't generalize to other callers at all. But I definitely agree with all the concerns around duplicating code. Looks like we have some `PrintingCallbacks`, maybe we can have something like `SummarizeTagDecl`, which enables customizing what to put into the body, when printing a TagDecl in terse mode?
https://github.com/llvm/llvm-project/pull/89557
More information about the cfe-commits
mailing list