Re: [PATCH] D12921: clang-format: Support 'template<>' (no space).

[Daniel Jasper via cfe-commits]

djasper added a comment.

This has come up before and the decision was that this is not important enough 
to meet the bar for an additional clang-format option. clang-format options 
have a certain cost and this specific space is so entirely unimportant that we 
don't want to pay it.

We have tried to eradicate this style wherever we find it (e.g. in the C++ 
standard) and encourage people to just change their style here if they want to 
use clang-format.


http://reviews.llvm.org/D12921



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12921: clang-format: Support 'template<>' (no space).

[Paul Hoad via cfe-commits]

MyDeveloperDay added a subscriber: MyDeveloperDay.
MyDeveloperDay added a comment.

This looks like a good small change.

What is interesting to me is that even the twiki page about template is 
inconsistent in its style (https://en.wikipedia.org/wiki/Template_(C%2B%2B)), 
so its not like there is "one style to rule them all"  (some have spaces, some 
do not)

@djasper, I don't understand here what we mean by "cost" in your review comment 
 (performance, maintainability?), the change has unit tests to keep it honest 
and its just an additional expression on an 'if', is there something else we 
should be concerned about when submitting patches? (long term maintainability?)

It does sound a little to me like we might be want to reject it because we 
don't like this style and don't want to proliferate its continued use, and 
can't understand why anyone would do it that way.  Unfortunately that is not 
very realistic for those of us who want to introduce clang-format into a code 
base that predates clang, google and for some of us even the internet! which 
might have it own style guide which doesn't quite match clang (not quite)

In my view this finite control over every aspect of a style is what 
clang-format needs, we need clang-format to allow the individuals who maintain 
large code bases to be able to fine tune the style to meet our companies own 
documented style, then we can introduce it to automate that style. But the 
concept that we might be able to alter that style guide of our company and 
inflict a tidal wave of changes onto previously conforming code is 
unfortunately not possible.

I know this may not match llvm,clangs requirements but they are possibly the 
requirements to ensure clang-format becomes completely ubiquitous in the 
industry.

This review like my own review for similar finite control over else and catch 
(http://reviews.llvm.org/D12492) is obviously not the way that clang-format 
wants to go, we want to contribute but can't do it if our patches aren't 
included, do you have a recommendation?

Could I ask you to reconsider this review, I'm willing to help.


http://reviews.llvm.org/D12921



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12921: clang-format: Support 'template<>' (no space).

[Paul Hoad via cfe-commits]

MyDeveloperDay added a comment.

Oh and we might want to run clang-format by the guys at http://cppreference.com 
seems they don't like the space either!   just saying


http://reviews.llvm.org/D12921



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12921: clang-format: Support 'template<>' (no space).

[Daniel Jasper via cfe-commits]

The most important costs are: Maintenance and discoverability of options.

The point here that it this space is utterly irrelevant, it doesn't make a
readability difference ever. Most codebases are inconsistent about it
anyway and I haven't seen a style guide even talking about this.

We never intended clang-format to provide control over every aspect of
formatting. We'd rather have it support a limited set of (important) style
options really well.

This simply isn't worth it, not even the time arguing about it.

On Thu, Sep 17, 2015 at 9:37 AM, Paul Hoad  wrote:

> MyDeveloperDay added a comment.
>
> Oh and we might want to run clang-format by the guys at
> http://cppreference.com seems they don't like the space either!   just
> saying
>
>
> http://reviews.llvm.org/D12921
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12921: clang-format: Support 'template<>' (no space).

[Paul Hoad via cfe-commits]

MyDeveloperDay added a comment.

> The point here that it this space is utterly irrelevant


So I totally take the point but if it doesn't matter then why does clang-format 
automatically add one to my "template<>" why not simply leave it alone?

> We never intended clang-format to provide control over every aspect of 
> formatting


I know that when new people come along on an open source project they want to 
change the world, for that I apologize, we weren't here for the previous 
decisions, but maybe clang-format is turning into something that is different 
from what was originally intended, maybe it CAN become the code formatting tool 
for those teams out there that are not quite/can't  do it the google way.

> We'd rather have it support a limited set of (important) style options really 
> well


And that I understand, but for those of us whose style does not exactly meet 
one of the 5 what can WE do other than submit patches to give finer control.

I don't want to argue but I would like you (who seems to be the gatekeeper for 
clang-format) to consider.


http://reviews.llvm.org/D12921



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12921: clang-format: Support 'template<>' (no space).

[Daniel Jasper via cfe-commits]

On Thu, Sep 17, 2015 at 12:17 PM, Paul Hoad 
wrote:

> MyDeveloperDay added a comment.
>
> > The point here that it this space is utterly irrelevant
>
>
> So I totally take the point but if it doesn't matter then why does
> clang-format automatically add one to my "template<>" why not simply leave
> it alone?
>

Because that is something else clang-format (almost) never does.
Consistency is beneficial.

> We never intended clang-format to provide control over every aspect of
> formatting
>
>
> I know that when new people come along on an open source project they want
> to change the world, for that I apologize, we weren't here for the previous
> decisions, but maybe clang-format is turning into something that is
> different from what was originally intended, maybe it CAN become the code
> formatting tool for those teams out there that are not quite/can't  do it
> the google way.
>

I generally agree and will gladly accept any useful formatting option. I
know I have been slow at reviewing your patches and I am sorry about that.
I was on vacation and had to catch on other stuff, but I hope we can make
progress on those.

> We'd rather have it support a limited set of (important) style options
> really well
>
>
> And that I understand, but for those of us whose style does not exactly
> meet one of the 5 what can WE do other than submit patches to give finer
> control.
>

One of the 5?? Clang-format has *many* options by now and supports many
large open-source projects and publicly available style guides. I have
personally spent a lot of time on and implemented many options that are
entirely irrelevant to both Google and LLVM.

And whether or not to add the space is something that nobody really had an
opinion. Basically every project I sampled was inconsistent, not a single
style guide said what to do. We looked around at the sources we could find
(the style guide most heavily) and made a decision. If you had brought up
good arguments that the space hurts readability (I really doubt it), you
could have made us turn the other way. The point is that this space matter
so little that it is not worth having the option.

I don't want to argue but I would like you (who seems to be the gatekeeper
> for clang-format) to consider.
>

I have considered and my opinion stands. It is not worth having this option
and it is also not worth changing that clang-format adds the space now that
it has been in use by many for 2+ years.

I am sorry if this means you cannot use clang-format for a significant
project.

http://reviews.llvm.org/D12921
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12921: clang-format: Support 'template<>' (no space).

[Daniel Jasper via cfe-commits]

Lets says this: I am not happy about it and wouldn't allow it if it was
added now. However, *removing* it actually has additional costs, so I am
not inclined to do so now.

On Thu, Sep 17, 2015 at 9:52 PM, strager  wrote:

> strager added a comment.
>
> Should we remove `ObjCSpaceBeforeProtocolList`? It has the same problem as
> the `SpaceAfterTemplateKeyword` I am introducing.
>
>
> http://reviews.llvm.org/D12921
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12921: clang-format: Support 'template<>' (no space).

[strager via cfe-commits]

strager added a comment.

Should we remove `ObjCSpaceBeforeProtocolList`? It has the same problem as the 
`SpaceAfterTemplateKeyword` I am introducing.


http://reviews.llvm.org/D12921



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits