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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
