<div dir="ltr">Turns out, FormatToken already has "NestingLevel" which you should be able to use. Cleanup to also use that in the ContinuationIndenter in r208304.</div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Wed, May 7, 2014 at 7:13 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.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">Sorry that this slipped by for so long.<div><br></div><div>I think generally we can support this options.</div><div><br></div><div>I have two higher-level comments to start with:</div><div>1. Lets directly reuse SpacesInParentheses and turn that into an enum SpacesInParenthesesStyle with the three values (SIPS_None, SIPS_Outermost, SIPS_Always). For backwards compatibility, the JSON parsing should evaluate false to SIPS_None and true to SIPS_Always. See TabStyle for an example of how to write the ScalarEnumerationTraits.</div>
<div><br></div><div>2. I think it is bad to have the ParenNestingLevel that you are introducing as well as the LineState's ParenLevel, which are kind of similar but contain slightly different things. The differences are currently that ParenLevel counts [] and <> as well, which probably shouldn't matter much for your style (haven't spent much time on thinking about corner cases) and that ParenLevel is calculated on the fly while indenting (which actually doesn't help much as it is always the same for the same token). Let me try to move ParenLevel to where your ParenNestingLevel currently is and see whether we can reuse it then.</div>
<div><br></div><div>So, can you make change #1 and upload your patch to phabricator (<a href="http://reviews.llvm.org/" target="_blank">http://reviews.llvm.org/</a>). I have another few comments, but they are easier to make and track in a proper code review system. I'll address #2 tomorrow.</div>
<div><br></div><div>Cheers,</div><div>Daniel</div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">On Wed, Apr 30, 2014 at 6:01 PM, Aaron Wishnick <span dir="ltr"><<a href="mailto:aaron.s.wishnick@gmail.com" target="_blank">aaron.s.wishnick@gmail.com</a>></span> wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"><div dir="ltr">Ping! Please let me know if there are any changes you would like me to make, or if this is ready to commit.<div>
<br></div><div>Thanks,</div><div>Aaron</div></div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Thu, Apr 3, 2014 at 9:25 AM, Aaron Wishnick <span dir="ltr"><<a href="mailto:aaron.s.wishnick@gmail.com" target="_blank">aaron.s.wishnick@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"><span style="font-family:arial,sans-serif;font-size:13px">This patch adds an option to control when spaces are added inside of parentheses. My employer's style guide mandates that spaces go inside the outermost pair of parentheses, but not the rest. For example:</span><div style="font-family:arial,sans-serif;font-size:13px">
<br></div><div style="font-family:arial,sans-serif;font-size:13px">if( someFunction(a, b, c) ) {</div><div style="font-family:arial,sans-serif;font-size:13px"> doThing( f(x), f(g(x)) );</div><div style="font-family:arial,sans-serif;font-size:13px">
}</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">My attached patch implements this feature with a new option, SpacesInParenthesesStyle, which can either be "Always" (the previous behavior, and the default), or "Outermost", the new behavior used by my organization.</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">Does this seem like a reasonable strategy? The new option defaults to the previous behavior. I see that a different approach was taken with SpaceBeforeParensOptions. I went with this approach, because the option applies to spaces inserted inside of parentheses due to SpacesInParentheses, SpaceInEmptyParentheses, SpacesInCStyleCastParentheses, etc.</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">Thanks,<br>Aaron</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">
N.B. I originally sent this to the cfe-dev list by mistake, sorry for the noise!</div></div>
</blockquote></div><br></div>
</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><br></div>