Hi,
On Sun, 2013-12-08 at 21:32 +0100, Daniel Jasper wrote:
> I think we might just want to change the existing
> option BreakConstructorInitializersBeforeComma to do exactly this. WebKit
> style is the only style using it and I agree that the style guide is
> explicit in this regard.
that seems reasonable to me. Please find attached a patch which
implements that behavior.
Unlike the old one, the new patch also works if ColumnLimit is 0, so it
does work for WebKit, too.
Florian Sowade
> On Thu, Dec 5, 2013 at 10:50 PM, Nico Weber <[email protected]> wrote:
>
> > (This is what WebKit style asks for too:
> > http://www.webkit.org/coding/coding-style.html , "Other Punctuation"
> > point 1. So if this goes in, maybe you can set to true for webkit style.)
> >
> >
> > On Thu, Dec 5, 2013 at 10:23 AM, Florian Sowade <[email protected]> wrote:
> >
> >> Hi,
> >>
> >> the attached patch adds a clang-format option to always break
> >> constructor initializers before the initial colon.
> >>
> >> So this patch adds an option to turn
> >>
> >> Constructor : foo(bar)
> >> {}
> >>
> >> into
> >>
> >> Constructor
> >> : foo(bar)
> >> {}
> >>
> >> I don't know if this style is widely distributed, but I use it to
> >> minimize the diff when I add new members to the list of constructor
> >> initializers, since I can add a second member to the list without
> >> modifying any of the existing lines:
> >>
> >> Constructor
> >> : foo(bar)
> >> , baz()
> >> {}
> >>
> >> From my understanding this is not possible with the current set of
> >> options.
> >>
> >> Florian Sowade
diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp
index e63f72d..0c5a94b 100644
--- a/lib/Format/ContinuationIndenter.cpp
+++ b/lib/Format/ContinuationIndenter.cpp
@@ -183,9 +183,13 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
Current.LongestObjCSelectorName == 0 &&
State.Stack.back().BreakBeforeParameter)
return true;
- if ((Current.Type == TT_CtorInitializerColon ||
- (Previous.ClosesTemplateDeclaration && State.ParenLevel == 0 &&
- !Current.isTrailingComment())))
+ if (Current.Type == TT_CtorInitializerColon &&
+ !(Style.AllowShortFunctionsOnASingleLine &&
+ !Style.BreakConstructorInitializersBeforeComma &&
+ Style.ColumnLimit == 0))
+ return true;
+ if ((Previous.ClosesTemplateDeclaration && State.ParenLevel == 0 &&
+ !Current.isTrailingComment()))
return true;
if ((Current.Type == TT_StartOfName || Current.is(tok::kw_operator)) &&
diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 87c2fd7..8661927 100644
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -455,13 +455,12 @@ public:
/// \brief Formats the line starting at \p State, simply keeping all of the
/// input's line breaking decisions.
- void format(unsigned FirstIndent, const AnnotatedLine *Line,
- bool LineIsMerged) {
+ void format(unsigned FirstIndent, const AnnotatedLine *Line) {
LineState State =
Indenter->getInitialState(FirstIndent, Line, /*DryRun=*/false);
while (State.NextToken != NULL) {
bool Newline =
- (!LineIsMerged && Indenter->mustBreak(State)) ||
+ Indenter->mustBreak(State) ||
(Indenter->canBreak(State) && State.NextToken->NewlinesBefore > 0);
Indenter->addTokenToState(State, Newline, /*DryRun=*/false);
}
@@ -728,8 +727,7 @@ public:
// FIXME: Implement nested blocks for ColumnLimit = 0.
NoColumnLimitFormatter Formatter(Indenter);
if (!DryRun)
- Formatter.format(Indent, &TheLine,
- /*LineIsMerged=*/MergedLines > 0);
+ Formatter.format(Indent, &TheLine);
} else {
Penalty += format(TheLine, Indent, DryRun);
}
diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index 19759af..e2615db 100644
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -1407,7 +1407,8 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
Right.Previous->MatchingParen->NestingLevel == 0 &&
Style.AlwaysBreakTemplateDeclarations) {
return true;
- } else if (Right.Type == TT_CtorInitializerComma &&
+ } else if ((Right.Type == TT_CtorInitializerComma ||
+ Right.Type == TT_CtorInitializerColon) &&
Style.BreakConstructorInitializersBeforeComma &&
!Style.ConstructorInitializerAllOnOneLineOrOnePerLine) {
return true;
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index bd9d108..5e38f59 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -7524,7 +7524,38 @@ TEST_F(FormatTest, ConstructorInitializerIndentWidth) {
" , b(b)\n"
" , c(c) {}",
Style);
+ verifyFormat("SomeClass::Constructor()\n"
+ " : a(a) {}",
+ Style);
+
+ Style.ColumnLimit = 0;
+ verifyFormat("SomeClass::Constructor()\n"
+ " : a(a) {}",
+ Style);
+ verifyFormat("SomeClass::Constructor()\n"
+ " : a(a)\n"
+ " , b(b)\n"
+ " , c(c) {}",
+ Style);
+ verifyFormat("SomeClass::Constructor()\n"
+ " : a(a) {\n"
+ " foo();\n"
+ " bar();\n"
+ "}",
+ Style);
+
+ Style.AllowShortFunctionsOnASingleLine = false;
+ verifyFormat("SomeClass::Constructor()\n"
+ " : a(a)\n"
+ " , b(b)\n"
+ " , c(c) {\n}",
+ Style);
+ verifyFormat("SomeClass::Constructor()\n"
+ " : a(a) {\n}",
+ Style);
+ Style.ColumnLimit = 80;
+ Style.AllowShortFunctionsOnASingleLine = true;
Style.ConstructorInitializerIndentWidth = 2;
verifyFormat("SomeClass::Constructor()\n"
" : a(a)\n"
@@ -7541,6 +7572,10 @@ TEST_F(FormatTest, ConstructorInitializerIndentWidth) {
Style.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
Style.ConstructorInitializerIndentWidth = 4;
+ verifyFormat("SomeClass::Constructor() : aaaaaaaa(aaaaaaaa) {}", Style);
+ verifyFormat(
+ "SomeClass::Constructor() : aaaaa(aaaaa), aaaaa(aaaaa), aaaaa(aaaaa)\n",
+ Style);
verifyFormat(
"SomeClass::Constructor()\n"
" : aaaaaaaa(aaaaaaaa), aaaaaaaa(aaaaaaaa), aaaaaaaa(aaaaaaaa) {}",
@@ -7600,18 +7635,27 @@ TEST_F(FormatTest, FormatsWithWebKitStyle) {
// Constructor initializers are formatted one per line with the "," on the
// new line.
- EXPECT_EQ(
- "Constructor()\n"
- " : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
- " , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, // break\n"
- " aaaaaaaaaaaaaa)\n"
- " , aaaaaaaaaaaaaaaaaaaaaaa() {}",
- format("Constructor()\n"
- " : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
- " , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, // break\n"
- " aaaaaaaaaaaaaa)\n"
- " , aaaaaaaaaaaaaaaaaaaaaaa() {}",
- Style));
+ verifyFormat("Constructor()\n"
+ " : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
+ " , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, // break\n"
+ " aaaaaaaaaaaaaa)\n"
+ " , aaaaaaaaaaaaaaaaaaaaaaa() {}",
+ Style);
+ verifyFormat("SomeClass::Constructor()\n"
+ " : a(a) {}",
+ Style);
+ verifyFormat("SomeClass::Constructor()\n"
+ " : a(a)\n"
+ " , b(b)\n"
+ " , c(c) {}",
+ Style);
+ verifyFormat("SomeClass::Constructor()\n"
+ " : a(a)\n"
+ "{\n"
+ " foo();\n"
+ " bar();\n"
+ "}",
+ Style);
// Access specifiers should be aligned left.
verifyFormat("class C {\n"
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits