[PATCH] D120217: [clang-format] Add an option to insert braces after control statements

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Nice! A few minor comments though.



================
Comment at: clang/include/clang/Format/Format.h:2572-2573
 
+  /// Insert braces after control statements (``if``, ``else``, ``for``, ``do``,
+  /// and ``while``) in C++.
+  /// \warning
----------------
Please add a comment in which cases the braces are added. If I understand correctly, everywhere except for PP directives, right?


================
Comment at: clang/lib/Format/Format.cpp:1839
+private:
+  // Insert optional braces.
+  void insertBraces(SmallVectorImpl<AnnotatedLine *> &Lines,
----------------
This comment seems not very useful, remove please.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2449
     } else {
-      addUnwrappedLine();
-      ++Line->Level;
-      parseStructuralElement();
-      if (FormatTok->is(tok::eof))
-        addUnwrappedLine();
-      --Line->Level;
+      parseUnbracedBody(/*CheckEOF=*/true);
     }
----------------
Is it possible to get rid of the `CheckEOF` parameter and do `if (FormatTok->is(tok::eof)) addUnwrappedLine();` only here?
(I'm unsure about the dependency between `Line` and `addUnwrappedLine`)


================
Comment at: clang/unittests/Format/FormatTest.cpp:24298
+
+  verifyFormat("if (a) {\n"
+               "  switch (b) {\n"
----------------
Could you add a test with a `// clang-format off` part?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120217/new/

https://reviews.llvm.org/D120217