kadircet added a comment.
Thanks this looks great, and something i've been longing for some time! But I
got a couple of questions about the how the interaction is designed (sorry if I
missed some high level discussions elsewhere, feel free to just point me in
that direction).
- Why do we just declare methods, rather than defining them too (with dummy
bodies) ? This would allow users to move function bodies out-of-line if they
want to easily. Also it would enforce them to fill in the bodies, rather than
forgetting/skipping some of the methods. (Even more maybe we should take the
extra step and offer another action to declare the function inline and define
it out-of-line, but I think that could be done iteratively if we see the need)
- Implementing all (pure) virtuals vs offering a code action for each possible
method. It is unfortunate that our existing tweak infrastructure doesn't enable
a single tweak to output multiple code actions, but I believe in this case we
might achieve a whole lot better UX if we did offer implementing each method
one by one, while possibly still suggesting implement "all" overrides or "only"
pures. It is still useful in its current form ofcourse, as the user can
manually change the declaration into a pure one, or delete it, but it sounds
cumbersome if they only want to implement a small subset of all possible pure
methods .
- Acting on non-pure virtuals too, i believe this is also a quite common use
case that could benefit a lot of users. but this definitely increases the need
for a code action per override.
- When to trigger? I suppose what you have in the code ATM makes sense (e.g.
just on `[[class X]] : ... [[{]] [[}]]`) but it would be great to spell it out
explicitly so that others have a chance to raise concerns. I don't think
triggering within the body would be useful though, as users are likely to
navigate within class body for various reasons, and spamming them with lots of
codeactions at each cursor move is likely to be annoying.
- When to land this :) we are at the edge of a branch cut, and tweaks are a
feature triggered automatically by editors. so any crashes in there are likely
to make clangd useless (as they'll persist no matter what). so I believe we
should either wait for release cut, and land this afterwards and fixing any
crash reports we get until next release, or include it in current release while
marking the tweak as hidden so that it will only annoy experimental users.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94942/new/
https://reviews.llvm.org/D94942
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
