[PATCH] D75791: [clang-format] Added new option IndentExternBlock

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

All this confusion over the defaults is because we don't have the unit tests to check the default of each style. If we had that we could have a before/after conversation more easily

Nit: also please mark comments done once you have addressed them.



================
Comment at: clang/lib/Format/Format.cpp:731
   case FormatStyle::BS_Allman:
+    Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
     Expanded.BraceWrapping.AfterCaseLabel = true;
----------------
MarcusJohnson91 wrote:
> MyDeveloperDay wrote:
> > Isn't this changing the default?
> Expanded.BraceWrapping.AfterExternBlock = true; is a few lines lower, so it will use that value.
> 
> Maybe I should move this option to right below `Expanded.BraceWrapping.AfterExternBlock = true;`?
totally didn't see that.. yep thats correct


================
Comment at: clang/lib/Format/Format.cpp:719
     Expanded.BraceWrapping.AfterExternBlock = true;
+    Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
     Expanded.BraceWrapping.SplitEmptyFunction = true;
----------------
didn't see that either yep thats correct


================
Comment at: clang/lib/Format/Format.cpp:739
     Expanded.BraceWrapping.AfterExternBlock = true;
+    Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
     Expanded.BraceWrapping.BeforeCatch = true;
----------------
thats also correct, sorry for not seeing that


================
Comment at: clang/lib/Format/Format.cpp:753
     Expanded.BraceWrapping.AfterExternBlock = true;
+    Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
     Expanded.BraceWrapping.BeforeCatch = true;
----------------
ok thats correct too


================
Comment at: clang/lib/Format/Format.cpp:812
                              true,  true};
+  LLVMStyle.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
----------------
is this one correct? AfterExternBLock is false in this case correct? should this be NoIndent?


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

https://reviews.llvm.org/D75791