owenpan added inline comments.
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3398
+**RemoveBracesLLVM** (``Boolean``) :versionbadge:`clang-format 14`
+ Remove optional braces of control statements (``if``, ``else``, ``for``,
----------------
MyDeveloperDay wrote:
> Can we agree on one set of options that can be used for both insertion and
> removal even if this patch only does removal
> Can we agree on one set of options that can be used for both insertion and
> removal even if this patch only does removal
One idea is to run insertion for all missing braces and then this option to
remove the optional ones for LLVM, i.e., an `AutomaticBraces` if you will.
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3398
+**RemoveBracesLLVM** (``Boolean``) :versionbadge:`clang-format 14`
+ Remove optional braces of control statements (``if``, ``else``, ``for``,
----------------
curdeius wrote:
> I don't think it's a good idea to make this option dependent on a particular
> style. Even if it works for LLVM style only for the moment.
> I don't think it's a good idea to make this option dependent on a particular
> style. Even if it works for LLVM style only for the moment.
As @MyDeveloperDay said before, eliding braces was (by far?) the most frequent
review comments, and the community would appreciate something like this. So
it's likely that this option will get used and experimented, and we will get
bug reports and other feedback to improve the functionalities. Once we think
it's ready, we can easily configure/package the functionalities for other
styles.
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2338
+ KeepAncestorBraces();
+ NestedTooDeep.push_back(false);
if (FormatTok->is(tok::l_brace)) {
----------------
HazardyKnusperkeks wrote:
> owenpan wrote:
> > HazardyKnusperkeks wrote:
> > > I think it is worth to create a RAII type for that.
> > Can you explain why an RAII type would be beneficial here?
> You have a lot of push/pop pairs. There is the risk someone (in the future)
> may misuse it.
I see. I will think about it.
================
Comment at: clang/unittests/Format/FormatTest.cpp:23224
+
+ EXPECT_EQ("if (a)\n"
+ " if (b)\n"
----------------
MyDeveloperDay wrote:
> any reason these can't be verifyFormats?
Did you mean to add the expected part as a separate case? I don't think it
would add any value if there are no braces to remove in the first place?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116316/new/
https://reviews.llvm.org/D116316
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
