<div dir="ltr"><div>Thanks for working on this.</div><div><br></div>A couple of thoughts:<div>* How is the new test in FormatsWithWebKitStyle related?<br>* Either collapse both parameters into a single parameter or add test cases for all combinations.</div>

<div>* The parameters themselves have tests (ParsesConfiguration).</div><div><br></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

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

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">     case tok::l_brace:<br>+      Contexts.back().CanBeInInheritanceList = false;<br>

       if (!parseBrace())</blockquote></div><div><br></div><div><div>Is this necessary? Can you create a test that breaks without it?</div><div><br></div><div></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

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

<div><br></div><div>Is this intentional?</div><div><br></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

       else if (Previous.Type == TT_InheritanceColon) </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
-        State.Stack.back().Indent = State.Column;<br>+        // We had a colon on a newline. Now, if we want to align commas to the<br>+        // colon, we indent less by -2 == length(", ")</blockquote></div>

<div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">+        if (Style.BreakClassInheritanceListsBeforeComma)<br>

+          State.Stack.back().Indent = State.Column - 2;<br>+        else<br>+          State.Stack.back().Indent = State.Column;</blockquote><div><br></div><div>Remove this completely. </div></div><div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

+    if (Current.Type == TT_InheritanceColon) {<br>       State.Stack.back().AvoidBinPacking = true;<br>+      if (!Style.BreakClassInheritanceListsBeforeComma)<br>+        State.Stack.back().Indent = State.Column + 2;<br>

+    } </blockquote><div><br></div><div>Change this to</div></div><div><br></div><div>if (Current.Type == TT_InheritanceColon) {</div><div>  State.Stack.back().AvoidBinPacking = true;<br></div><div>  State.Stack.back().Indent = State.Column;</div>
<div>  if (!Style.BreakClassInheritanceListsBeforeComma)</div><div>    State.Stack.back().Indent += 2;</div><div>}</div><div><br></div><div>Cheers,</div><div>Daniel</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Fri, Aug 2, 2013 at 10:59 AM, Curdeius Curdeius <span dir="ltr"><<a href="mailto:curdeius@gmail.com" target="_blank">curdeius@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div>Hi,</div><div><br></div><div>I've added two options to clang-format:</div><div>* BreakClassInheritanceListsBeforeColon</div><div>and</div><div>* BreakClassInheritanceListsBeforeComma.<br>

</div><div><br></div><div>They allow us to break on class inheritance lists similarly to constructor initializer lists.</div><div><br></div><div>I've corrected as well the TokenAnnotator, which mistakenly annotated colons from switch statements ("case:") and from access modifiers ("public:") as TT_InheritanceColon.</div>


<div><br></div><div>The patch in the attachment.</div><br clear="all"><div>Cheers,<br>Marek Kurdej</div>
</div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>