[PATCH] D98214: [clang-format] Fix aligning with linebreaks
Marek Kurdej via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 10 00:20:26 PST 2021
curdeius added inline comments.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:332-340
if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName) ||
(ScopeStart > Start + 1 &&
Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName)) ||
+ (ScopeStart > Start + 1 &&
+ Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
+ Changes[ScopeStart - 1].Tok->is(tok::l_paren)) ||
Changes[i].Tok->is(TT_ConditionalExpr) ||
----------------
HazardyKnusperkeks wrote:
> curdeius wrote:
> > Would it be possible to break up this condition and name it (or name its parts)? It's getting hard to follow.
> > Suggestion according to my understanding, which might be wrong.
> Can do, but then all those are always checked, there is no short circuit anymore. But I really get your point.
>
> How about a lambda with different returns (and comments), that way we would still short circuit. Or some thing like
> ```
> bool AddShift = /* checks #1 */;
> AddShift = AddShift || /* checks #2 */;
> ...
> AddShift = AddShoft || /* checks #n */;
>
> if (AddShift)
> Changes[i].Spaces += Shift;
> ```
Good point on the short circuiting. A lambda may be a good solution here. But the `bool AddShift ...` that you suggested above (with comments too) is ok for me as well. I let you choose.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98214/new/
https://reviews.llvm.org/D98214
More information about the cfe-commits
mailing list