djasper added inline comments.
================
Comment at: lib/Format/ContinuationIndenter.cpp:196
+ FormatStyle::BCIS_AfterColonAndComma) &&
+ (State.Column + State.Line->Last->TotalLength - Previous.TotalLength >
+ getColumnLimit(State) ||
----------------
Typz wrote:
> Typz wrote:
> > djasper wrote:
> > > Why can you drop the "+2" here?
> > >
> > > Also, I'd like to structure this so we have to duplicate less of the
> > > logic. But I am not really sure it's possible.
> > the +2 here was needed to keep identifiers aligned when breaking after
> > colon but before the command (e.g. the 4th combination, not defined
> > anymore):
> >
> > Foo() :
> > field(1)
> > , field(2)
> I can avoid some duplication like this,m but i am not convinced it helps :
>
> const FormatToken &ColonToken =
> Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon
> ? Current : Previous;
> if (ColonToken.is(TT_CtorInitializerColon) &&
> (State.Column + State.Line->Last->TotalLength - ColonToken.TotalLength +
> (Style.BreakConstructorInitializers !=
> FormatStyle::BCIS_AfterColon ? 2 : 0) >
> getColumnLimit(State) ||
> State.Stack.back().BreakBeforeParameter) &&
> (Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All ||
> Style.BreakConstructorInitializers != FormatStyle::BCIS_BeforeColon ||
> Style.ColumnLimit != 0))
> return true;
>
> what do you think?
The +2 here has nothing todo with how the things are aligned. This is about
whether the entire constructor with initializer fits on one line. Can you try
out (or even add tests) for cases where the entire constructor is 80 and 81
columns long?
I think I like the condensed version a bit better, but yeah, it's not beautiful
either ;).
================
Comment at: lib/Format/ContinuationIndenter.cpp:468
+ } else if (Previous.is(TT_CtorInitializerColon) &&
+ Style.BreakConstructorInitializers ==
+ FormatStyle::BCIS_AfterColon) {
----------------
Does this fit on one line now?
================
Comment at: lib/Format/Format.cpp:309
+ // If BreakConstructorInitializersBeforeComma was specified but
+ // BreakConstructorInitializers was not, initialize the latter from the
+ // former for backwards compatibility.
----------------
How do you know that BreakConstructorInitializers was not specified?
https://reviews.llvm.org/D32479
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
