[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers

[Reuben Thomas via Phabricator via cfe-commits]

reuk added inline comments.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2009
+  // classes case
+  if (Style.AccessModifierIndentation && Line->Level % 2 == 0)
+    --Line->Level;
----------------
klimek wrote:
> What if the class starts at level 1? (for example, inside a function or due to namespace indentation)
> 
> namespace A {
>   class B {
>     public:
>       ..
>   };
> }
Probably worth adding a test or two that format nested classes, classes inside namespaces, classes defined inside functions etc.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2253
       parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true,
-                 /*MunchSemi=*/false);
+                 /*MunchSemi=*/false,
+                 /*IndentedAccessModifiers=*/ContainsAccessModifier);
----------------
Yeah, this would look way nicer using a settings struct.


================
Comment at: clang/lib/Format/UnwrappedLineParser.h:89
   void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
-                  bool MunchSemi = true);
+                  bool MunchSemi = true, bool IndentedAccessModifiers = false);
   void parseChildBlock();
----------------
Adjacent `bool` parameters is a bit nasty. Maybe pass a struct of settings instead? https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i4-make-interfaces-precisely-and-strongly-typed


Repository:
  rC Clang

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

https://reviews.llvm.org/D60225