[PATCH] [clang-extra-tools] [clang-format]

[Curdeius Curdeius]

Hi Daniel,
thanks for your feedback and sorry for the late response.

I attach the patch.

My comments to your remarks below.

* How is the new test in FormatsWithWebKitStyle related?
>

I removed the unnecessary mess.


> * Either collapse both parameters into a single parameter or add test
> cases for all combinations.
>

I did so.


>  * The parameters themselves have tests (ParsesConfiguration).
>

Ok, done.


>
> +    case tok::semi:
>> +      Contexts.back().CanBeInInheritanceList = false;
>> +      break;
>
>
> Is this necessary? Can you create a test that breaks without it?
>

You're right. It wasn't necessary


>
>      case tok::l_brace:
>> +      Contexts.back().CanBeInInheritanceList = false;
>>        if (!parseBrace())
>
>
> Is this necessary? Can you create a test that breaks without it?
>

You're right again. It wasn't necessary.


>
>  +        //} else if (Contexts.size() == 1) {
>> +        // FIXME: annotates switch-case colon as inheritance colon
>> +        // FIXME: annotates access specifiers as inheritance colon
>> +        // Tok->Type = TT_OtherColon;
>
>
> Is this intentional?
>

I removed this part.


>
>        else if (Previous.Type == TT_InheritanceColon)
>
> -        State.Stack.back().Indent = State.Column;
>> +        // We had a colon on a newline. Now, if we want to align commas
>> to the
>> +        // colon, we indent less by -2 == length(", ")
>
> +        if (Style.BreakClassInheritanceListsBeforeComma)
>> +          State.Stack.back().Indent = State.Column - 2;
>> +        else
>> +          State.Stack.back().Indent = State.Column;
>
>
> Remove this completely.
>

Well, I couldn't remove it completely.
If you have any idea how to do it in a cleaner way, I'll be glad to know.


> +    if (Current.Type == TT_InheritanceColon) {
>>        State.Stack.back().AvoidBinPacking = true;
>> +      if (!Style.BreakClassInheritanceListsBeforeComma)
>> +        State.Stack.back().Indent = State.Column + 2;
>> +    }
>
>
> Change this to
>
> if (Current.Type == TT_InheritanceColon) {
>   State.Stack.back().AvoidBinPacking = true;
>   State.Stack.back().Indent = State.Column;
>   if (!Style.BreakClassInheritanceListsBeforeComma)
>     State.Stack.back().Indent += 2;
> }
>

Done.


>  On Fri, Aug 2, 2013 at 10:59 AM, Curdeius Curdeius <curdeius at gmail.com>wrote:
>
>>  Hi,
>>
>> I've added two options to clang-format:
>> * BreakClassInheritanceListsBeforeColon
>> and
>> * BreakClassInheritanceListsBeforeComma.
>>
>> They allow us to break on class inheritance lists similarly to
>> constructor initializer lists.
>>
>> I've corrected as well the TokenAnnotator, which mistakenly annotated
>> colons from switch statements ("case:") and from access modifiers
>> ("public:") as TT_InheritanceColon.
>>
>> The patch in the attachment.
>>
>> Cheers,
>> Marek Kurdej
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>

Regards,
Marek Kurdej
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131023/ec10cfd8/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang-format-break-class-inheritance-lists-before-comma-and-colon.patch
Type: application/octet-stream
Size: 11787 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131023/ec10cfd8/attachment.obj>