MarcusJohnson91 marked 4 inline comments as done.
MarcusJohnson91 added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:714
   case FormatStyle::BS_Mozilla:
+    Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
     Expanded.BraceWrapping.AfterClass = true;
----------------
MyDeveloperDay wrote:
> I'm sorry but I feel these are changing the previous default which as I 
> understood would indent ONLY if Style.BraceWrapping.AfterExternBlock == true
> 
> I think in all cases other than GNU this was false, isn't that correct?
I chose IEBS_AfterExternBlock here, because it already uses the 
BraceWrapping.AfterExternBlock style, that way it will still use 
AfterExternBlock: true value a few lines lower.


================
Comment at: clang/lib/Format/Format.cpp:725
   case FormatStyle::BS_Stroustrup:
+    Expanded.IndentExternBlock = FormatStyle::IEBS_NoIndent;
     Expanded.BraceWrapping.AfterFunction = true;
----------------
MyDeveloperDay wrote:
> I think you can remove this to avoid confusion that you are changing from the 
> default LLVM style
k, that makes sense; I figured nothing was specified so it should be set to no, 
but I can remove it also.


================
Comment at: clang/lib/Format/Format.cpp:731
   case FormatStyle::BS_Allman:
+    Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
     Expanded.BraceWrapping.AfterCaseLabel = true;
----------------
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;`?


================
Comment at: clang/lib/Format/Format.cpp:931
   GoogleStyle.IndentCaseLabels = true;
+  GoogleStyle.IndentExternBlock = FormatStyle::IEBS_NoIndent;
   GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
----------------
MyDeveloperDay wrote:
> everyone inherits from LLVM so no need for this it only makes people think 
> its different from the base style
ok, I can remove the inherited ones too.


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

https://reviews.llvm.org/D75791



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to