MyDeveloperDay added subscribers: Abpostelnicu, sylvestre.ledru.
MyDeveloperDay added a comment.
This is totally fine, now I'm just concerned by the choice of defaults, I
really don't know if we want to change the defaults for all the styles, I don't
want to break all those people using it
One way might be if we canvas opinion from those developers who work on some of
those proejcts for example @sylvestre.ledru, @Abpostelnicu what impact might
this have on the Mozilla sources (if any?)
================
Comment at: clang/lib/Format/Format.cpp:714
case FormatStyle::BS_Mozilla:
+ Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
Expanded.BraceWrapping.AfterClass = true;
----------------
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?
================
Comment at: clang/lib/Format/Format.cpp:725
case FormatStyle::BS_Stroustrup:
+ Expanded.IndentExternBlock = FormatStyle::IEBS_NoIndent;
Expanded.BraceWrapping.AfterFunction = true;
----------------
I think you can remove this to avoid confusion that you are changing from the
default LLVM style
================
Comment at: clang/lib/Format/Format.cpp:731
case FormatStyle::BS_Allman:
+ Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
Expanded.BraceWrapping.AfterCaseLabel = true;
----------------
Isn't this changing the default?
================
Comment at: clang/lib/Format/Format.cpp:746
case FormatStyle::BS_Whitesmiths:
+ Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
Expanded.BraceWrapping.AfterCaseLabel = true;
----------------
Isn't this changing the default?
================
Comment at: clang/lib/Format/Format.cpp:761
case FormatStyle::BS_GNU:
+ Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
Expanded.BraceWrapping = {true, true, FormatStyle::BWACS_Always,
----------------
Ok this one feel correct.
================
Comment at: clang/lib/Format/Format.cpp:931
GoogleStyle.IndentCaseLabels = true;
+ GoogleStyle.IndentExternBlock = FormatStyle::IEBS_NoIndent;
GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
----------------
everyone inherits from LLVM so no need for this it only makes people think its
different from the base style
================
Comment at: clang/lib/Format/Format.cpp:1067
ChromiumStyle.IndentWidth = 4;
+ ChromiumStyle.IndentExternBlock = FormatStyle::IEBS_NoIndent;
// See styleguide for import groups:
----------------
everyone inherits from LLVM so no need for this it only makes people think its
different from the base style
================
Comment at: clang/lib/Format/Format.cpp:1117
MozillaStyle.IndentCaseLabels = true;
+ MozillaStyle.IndentExternBlock = FormatStyle::IEBS_NoIndent;
MozillaStyle.ObjCSpaceAfterProperty = true;
----------------
everyone inherits from LLVM so no need for this it only makes people think its
different from the base style
================
Comment at: clang/lib/Format/Format.cpp:1140
Style.IndentWidth = 4;
+ Style.IndentExternBlock = FormatStyle::IEBS_NoIndent;
Style.NamespaceIndentation = FormatStyle::NI_Inner;
----------------
everyone inherits from LLVM so no need for this it only makes people think its
different from the base style
================
Comment at: clang/lib/Format/Format.cpp:1159
Style.ColumnLimit = 79;
+ Style.IndentExternBlock = FormatStyle::IEBS_NoIndent;
Style.FixNamespaceComments = false;
----------------
everyone inherits from LLVM so no need for this it only makes people think its
different from the base style
================
Comment at: clang/lib/Format/Format.cpp:1200
NoStyle.SortUsingDeclarations = false;
+ NoStyle.IndentExternBlock = FormatStyle::IEBS_NoIndent;
return NoStyle;
----------------
everyone inherits from LLVM so no need for this it only makes people think its
different from the base style
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
