[PATCH] clang-format: Add SpacesInParenthesesStyle

Daniel Jasper djasper at google.com
Wed May 7 10:13:42 PDT 2014


Sorry that this slipped by for so long.

I think generally we can support this options.

I have two higher-level comments to start with:
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.

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.

So, can you make change #1 and upload your patch to phabricator (
http://reviews.llvm.org/). I have another few comments, but they are easier
to make and track in a proper code review system. I'll address #2 tomorrow.

Cheers,
Daniel


On Wed, Apr 30, 2014 at 6:01 PM, Aaron Wishnick
<aaron.s.wishnick at gmail.com>wrote:

> Ping! Please let me know if there are any changes you would like me to
> make, or if this is ready to commit.
>
> Thanks,
> Aaron
>
>
> On Thu, Apr 3, 2014 at 9:25 AM, Aaron Wishnick <aaron.s.wishnick at gmail.com
> > wrote:
>
>> 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:
>>
>> if( someFunction(a, b, c) ) {
>>   doThing( f(x), f(g(x)) );
>> }
>>
>> 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.
>>
>> 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.
>>
>> Thanks,
>> Aaron
>>
>> N.B. I originally sent this to the cfe-dev list by mistake, sorry for the
>> noise!
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140507/30a80051/attachment.html>


More information about the cfe-commits mailing list