nridge added inline comments.
================
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)
Phabricator's inter-diff seems to be broken, but glancing through the new
revision, it seems like this comment about adding a couple of more test cases
hasn't been addressed yet, are you planning to do that in another update?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127082/new/
https://reviews.llvm.org/D127082
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
