<div dir="ltr">Hi Daniel,<div>thanks for your feedback and sorry for the late response.</div><div><br></div><div>I attach the patch.</div><div><br></div><div>My comments to your remarks below.</div><div class="gmail_extra">
<br><div class="gmail_quote"><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"><div dir="ltr"><div>* How is the new test in FormatsWithWebKitStyle related?<br>
</div></div></blockquote><div><br></div><div>I removed the unnecessary mess.</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">
<div dir="ltr"><div>* Either collapse both parameters into a single parameter or add test cases for all combinations.</div></div></blockquote><div><br></div><div>I did so.</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">
<div dir="ltr">
<div>* The parameters themselves have tests (ParsesConfiguration).</div></div></blockquote><div><br></div><div>Ok, done.</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">
<div dir="ltr"><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></blockquote>
<div><br></div><div>You're right. It wasn't necessary</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">
<div dir="ltr"><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></div></blockquote><div><br></div><div>You're right again. It wasn't necessary.</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"><div dir="ltr"><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></blockquote><div><br></div><div>I removed this part.</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">
<div dir="ltr"><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></blockquote><div>
<br></div><div>Well, I couldn't remove it completely.</div><div>If you have any idea how to do it in a cleaner way, I'll be glad to know.</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">
<div dir="ltr"><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></blockquote><div><br></div><div>Done.<br></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">
<div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">
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></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">
<div><div class="h5">
<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></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">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>
</blockquote></div></div><br><div class="gmail_extra"><br></div><div class="gmail_extra"><div>Regards,<br>Marek Kurdej</div><div><br></div></div></div>