[PATCH] D120188: Fix extraneous whitespace addition in line comments on clang-format directives
Luis Penagos via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Feb 19 16:24:50 PST 2022
penagos marked 3 inline comments as done.
penagos added inline comments.
================
Comment at: clang/lib/Format/BreakableToken.cpp:818-822
const auto AllowsSpaceChange =
- SpacesInPrefix != 0 ||
- (!NoSpaceBeforeFirstCommentChar() ||
- (FirstNonSpace == '}' && FirstLineSpaceChange != 0));
+ (!LineTok || !switchesFormatting(*LineTok)) &&
+ (SpacesInPrefix != 0 ||
+ (!NoSpaceBeforeFirstCommentChar() ||
+ (FirstNonSpace == '}' && FirstLineSpaceChange != 0)));
----------------
curdeius wrote:
> curdeius wrote:
> > HazardyKnusperkeks wrote:
> > > This slowly gets hard to read. Could you maybe split it up? Either in giving the multiple parts their own name. Or to keep the short circuiting have something like:
> > > ```
> > > auto AllowsSpaceChange = ...;
> > > AllowsSpaceChange = AllowsSpaceChange || ...;
> > > AllowsSpaceChange = AllowsSpaceChange || ...;
> > > ```
> > Or use a lambda (with a series of ifs for instance) to initialize it.
> While here, please change `auto` to `bool`.
I've split the conditional over a couple of smaller boolean conditionals with the intent of enhancing readability.
================
Comment at: clang/unittests/Format/FormatTestComments.cpp:98
+ "// clang-format on\n",
+ getLLVMStyleWithColumns(20)));
+
----------------
HazardyKnusperkeks wrote:
> Why the limit?
This was a copy-paste artifact and wasn't intended to be included in the original diff; the column limit has no bearing on this specific test addition.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120188/new/
https://reviews.llvm.org/D120188
More information about the cfe-commits
mailing list