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