daiyousei-qz added a comment.
It looks like my inline comment wasn't submitted (didn't click the submit
button in the bottom). Here's my old comment.
================
Comment at: clang-tools-extra/clangd/Hover.cpp:1091
+
+ // Reformat Macro Expansion
+ if (!HI->MacroExpansion.empty()) {
----------------
nridge wrote:
> It would be interesting to have a couple of test cases that exercise the
> reformatting in non-trivial ways, e.g. long expansions that need to be
> wrapped onto multiple lines
>
> I would suggest two such test cases, one with the expansion being in a
> declaration context, and the second an expression context (for this one, to
> make it long enough, the expansion could contain e.g. an `a ? b : c`
> expression)
>
> (I'm suggesting the expression-context testcase in part as a sanity check to
> make sure that `format::reformat()` handles such code reasonably in the first
> place)
Somehow, this comment goes out of the position.
In my opinion, such test should be written against `format::reformat()`
directly instead of hover message in clangd. One problem is that we are using
the current style in users' workspace to reformat the definition/expansion,
which means the same tokens might present differently given different
`.clang-format` or fallback style that the user has specified. I do agree that,
if tokens don't conform a regular c++ expression, like `) . field`, the
presentation could be bad. But I suppose there's no obvious solution for that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127082/new/
https://reviews.llvm.org/D127082
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
