[PATCH] clang-format: options for spacing template argument lists

Mon Oct 28 11:01:27 PDT 2013

I didn't realize there were tests for parsing options.  Added and attached.

If you can submit the change that would be great.

Chris

On Mon, Oct 28, 2013 at 11:59 AM, Daniel Jasper <djasper at google.com> wrote:
>
>
>
> On Mon, Oct 28, 2013 at 5:29 PM, Christopher Olsen <chrisaolsen at gmail.com>
> wrote:
>>
>> Thanks for the feedback.  I've updated the patch:
>>
>> Added the flag SpacesInAngles - A<int> vs A< int >
>> Note that LanguageStandard=Cpp03 overrides SpacesInAngles=false in the
>> case of '>>' as in A<A<int> >
>>
>> While I tend to agree with you on spacing in template arguments I have
>> 5+ million lines of code developed over 12 years in strict "pad
>> everything" style to contend with.  Honestly, I'm hoping clang-format
>> will eventually allow us to re-evaluate certain style choices if it
>> gets adopted on our project.
>
>
> As I said, I can perfectly understand the necessity for SpacesInAngles (both
> because there are large codebase where consistency matters and that it is a
> strong matter of what one is used to). So this option is totally reasonable.
> Sorry if I was unclear.
>
> The patch looks fine except that there doesn't seem to be a test for the
> parsing of the configuration option. That should also be done in
> unittests/Format/FormatTests.cpp along with the tests for parsing all the
> other options. Do you have commit access or should I submit this patch for
> you?
>
> Cheers,
> Daniel
>
>> Chris
>>
>> On Sun, Oct 27, 2013 at 4:32 AM, Daniel Jasper <djasper at google.com> wrote:
>> > The patch itself looks fine. However, I am also slightly skeptical that
>> > it
>> > is a good general direction.
>> >
>> > Specifically:
>> > - SpaceInEmptyAngles: Quite frankly, it doesn't matter enough. Nobody
>> > really
>> > cares (or should care about this). I'd make a decision and get on with
>> > life.
>> > If you need that space for your codebase, make it dependent on
>> > SpacesInAngles.
>> > - SpaceAfterTemplateKeyword: Same as above, it doesn't matter enough. We
>> > actually had a discussion about this very early into the clang-format
>> > development. People don't agree, but it really does not matter (no
>> > version
>> > is more or less readable, ..). We then actually went ahead (IIRC) to
>> > change
>> > all instances in the C++11 standard to do exactly what clang-format does
>> > now. I'd rather have clang-format force people into a consistent
>> > behavior
>> > for such non-issues than add additional technical complexity to it.
>> > - SpacesInAngles: I would personally be appalled by having to work in a
>> > codebase that uses this. Constructs like "template < class T..." simply
>> > look
>> > too much like comparisons to me. However, I can see that, if you are
>> > used to
>> > it, switching might be hard. The overlapping of the language standard is
>> > unfortunate, but I don't really see what we can do here.
>> >
>> > So, I am more or less fine with introducing SpacesInAngles (goes well
>> > with
>> > SpacesInParentheses), but I'd rather not introduce the other two (I know
>> > that we have SpaceInEmptyParentheses - I also don't like that one, but
>> > "()"
>> > is way more common than "<>", so it matters a bit more).
>> >
>> > I also agree that, at the very least, we need some grouping of the
>> > options
>> > soon. I have not found a way to specify style options more intuitively.
>> > And
>> > to some extend, I think people having to carefully look through the
>> > options
>> > or accept a default is reasonable.
>> >
>> > Cheers,
>> > Daniel
>> >
>> >
>> > On Fri, Oct 25, 2013 at 11:26 PM, Sean Silva <silvas at purdue.edu> wrote:
>> >>
>> >>
>> >>
>> >>
>> >> On Fri, Oct 25, 2013 at 4:00 PM, Christopher Olsen
>> >> <chrisaolsen at gmail.com>
>> >> wrote:
>> >>>
>> >>> I am evaluating clang-format and need control over spacing in template
>> >>> argument lists to conform to our coding standards.
>> >>>
>> >>> I've attached a patch that adds new flags to control this spacing.
>> >>> Unittests are also included.
>> >>>
>> >>> - SpacesInAngles - A<int> vs A< int >
>> >>> - SpaceInEmptyAngles - template <> vs template < >
>> >>> - SpaceAfterTemplateKeyword - template<typename T> vs template
>> >>> <typename
>> >>> T>
>> >>>
>> >>> Note that LanguageStandard=Cpp03 overrides SpacesInAngles=false in the
>> >>> case of '>>' as in A<A<int> >
>> >>>
>> >>> Please let me know if there is anything I need to fix for submission.
>> >>
>> >>
>> >> I foresee (and have already run into multiple real use cases) where the
>> >> spacing before, after, in between, etc. of many different kinds of
>> >> tokens
>> >> needs to be tweaked. For example:
>> >>
>> >> * multiplicative operators don't have spaces, but additive operators do
>> >> `a
>> >> + b*c`
>> >> * spaces before parenthesized lists in function calls `foo (bar)`
>> >> * spaces inside the parentheses of an `if`: `if ( cond ) {`
>> >> * "function-like" return: `return(3)`
>> >>
>> >> I think we should try (though it may not be realistic) to integrate
>> >> this
>> >> functionality in a way that covers the different use cases and makes
>> >> them
>> >> interact in a consistent and understandable way; otherwise we will just
>> >> end
>> >> up growing a forest of not-easy-to-discover options that don't have
>> >> very
>> >> good coverage of the configuration space for the next project. For
>> >> example,
>> >> adapting clang-format to the OP's project coding standards requires
>> >> adding 3
>> >> new options; is there a realistic upper bound on the number of such
>> >> options
>> >> that we will need in order for clang-format to support, say, 1000
>> >> different
>> >> projects from 50 different companies/open-source communities?
>> >>
>> >> One possibility that I can imagine (although I don't know how feasible
>> >> it
>> >> is) is to ship another tool (or more likely keep it under an option to
>> >> clang-format) which does a "one time" analysis to determine a set of
>> >> parameters that will conform with a given sample source file (or files)
>> >> and
>> >> emits a configuration file. This analysis could work with a larger (but
>> >> very
>> >> consistent and well-defined) "plumbing" configuration space that
>> >> essentially
>> >> parameterizes the "guts" of clang-format (such as
>> >> `spaceRequiredBefore`,
>> >> `spaceRequiredBetween`, `splitPenalty`, etc.) in a data-driven way.
>> >>
>> >> On the other hand, I think it has been put out there before is that one
>> >> of
>> >> the benefits of clang-format is to help "standardize" to some extent on
>> >> a
>> >> common subset of style options, and so providing too-fine-grained
>> >> support
>> >> for "tweaking" the output might be undesirable from such a perspective.
>> >> On
>> >> the other hand, if clang-format wants to "dominate the world", it can't
>> >> impose arbitrary changes on a project's coding style. A poignant
>> >> question is
>> >> "is it a goal for clang-format be able to conform with
>> >> more-or-less-arbitrary styles without requiring requiring the users to
>> >> get
>> >> involved with clang-format development?", but I can't speak as to the
>> >> answer.
>> >>
>> >> Daniel, what do you think?
>> >>
>> >> -- Sean Silva
>> >>
>> >>>
>> >>>
>> >>> Thanks!
>> >>> Chris
>> >>>
>> >>>
>> >>> _______________________________________________
>> >>> cfe-commits mailing list
>> >>> cfe-commits at cs.uiuc.edu
>> >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> >>>
>> >>
>> >
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang-format_spaces_in_angles.patch
Type: application/octet-stream
Size: 4697 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131028/7a079097/attachment.obj>