Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Andrew Trick via swift-dev]

> On Jan 26, 2017, at 4:48 PM, Greg Parker  wrote:
> 
> 
>> On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev 
>>  wrote:
>> 
>> Before I pull in a large PR that "accidentally" reformats a bunch of code, I 
>> want to get feedback on how Swift compiler devs plan to use `clang-format`. 
>> (BTW, here's the PR https://github.com/apple/swift/pull/6922).
>> 
>> During the code review, I ran `git clang-format` as part of being a good 
>> citizen. There's only one problem with the tool. It rewraps long boolean 
>> expressions, hiding those unsightly operators at the end of lines (after all 
>> who wants to see those?).
>> 
>>   if (some_expression->with_calls() ||
>>   another_expression->with(nested_calls()) &&
>>   an_even_longer_expression->making_your_eyes->glaze_over())
>> 
>> I don't get involved in style wars, but this is not a style change, it's an 
>> objective code quality degradation. It's a demonstrably bug-prone thing to 
>> do. It's hurt me too many times in the past, and I'm not happy using a 
>> formatting tool that injects future bugs and harms code comprehension.
> 
> Counterargument: It's not an objective code quality degradation, it's a style 
> choice that you don't like. 

It’s not about what I’m used to or what I find visually pleasing. It’s been a 
source of bugs in my experience. If my experience is unique, and people want to 
standardize on a different style, then I’ll just have to try to be more careful 
reading the code and write fewer complicated if-conditions.

> The first rule of code style is "do as the rest of the code does". 
> 
> Trailing binary operators are fine, as long as they are consistent. Mixing 
> leading and trailing operators in the same code base is a greater harm to 
> code comprehension than either one is on its own. Any difference between 
> consistently leading and consistently trailing is small, and not worth the 
> pain of a mass rewrite.

That’s why I haven’t brought this up with LLVM project. The style has already 
been standardized (although my old code still wraps operators the right way :). 
I’m not sure anyone thought about this and didn’t want to repeat the same 
mistake in Swift.

I’ve seen both styles in the Swift compiler, although looking around the code 
now my preferred convention seems pretty clearly out of style. I also spent 
time in the standard library code, which has very carefully thought out 
conventions and wraps operators the way I’m recommending. Moving toward that 
style seemed reasonable if most developers agreed.

If I am the only person that feels strongly about it, then we’re probably 
better of standardizing on LLVM’s defaults just because most of the code has 
already done that. So here’s the option I intentionally left out:

Option 4: Standardize Swift style based purely on LLVM.

  Let's standardize the Swift compiler using LLVM’s default style configuration.

> I consider this LLVM and Swift project style to be an objective code quality 
> degradation:
>if (condition)
>single_line_body();
> 
> But the first rule of code style is "do as the rest of the code does", so I 
> write my LLVM and Swift code like that.

I agree. If auto-indenting editors hadn’t solved that problem I’d be arguing 
about that too!

-Andy
___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Greg Parker via swift-dev]

> On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev  
> wrote:
> 
> Before I pull in a large PR that "accidentally" reformats a bunch of code, I 
> want to get feedback on how Swift compiler devs plan to use `clang-format`. 
> (BTW, here's the PR https://github.com/apple/swift/pull/6922).
> 
> During the code review, I ran `git clang-format` as part of being a good 
> citizen. There's only one problem with the tool. It rewraps long boolean 
> expressions, hiding those unsightly operators at the end of lines (after all 
> who wants to see those?).
> 
>if (some_expression->with_calls() ||
>another_expression->with(nested_calls()) &&
>an_even_longer_expression->making_your_eyes->glaze_over())
> 
> I don't get involved in style wars, but this is not a style change, it's an 
> objective code quality degradation. It's a demonstrably bug-prone thing to 
> do. It's hurt me too many times in the past, and I'm not happy using a 
> formatting tool that injects future bugs and harms code comprehension.

Counterargument: It's not an objective code quality degradation, it's a style 
choice that you don't like. 

The first rule of code style is "do as the rest of the code does". 

Trailing binary operators are fine, as long as they are consistent. Mixing 
leading and trailing operators in the same code base is a greater harm to code 
comprehension than either one is on its own. Any difference between 
consistently leading and consistently trailing is small, and not worth the pain 
of a mass rewrite.

I consider this LLVM and Swift project style to be an objective code quality 
degradation:
if (condition)
single_line_body();

But the first rule of code style is "do as the rest of the code does", so I 
write my LLVM and Swift code like that.


-- 
Greg Parker gpar...@apple.com Runtime Wrangler


___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Jordan Rose via swift-dev]

> On Jan 26, 2017, at 12:55, Andrew Trick via swift-dev  
> wrote:
> 
>> 
>> On Jan 26, 2017, at 11:38 AM, Graydon Hoare  wrote:
>> 
>> 
>>> On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev 
>>>  wrote:
>>> 
>>> Before I pull in a large PR that "accidentally" reformats a bunch of code, 
>>> I want to get feedback on how Swift compiler devs plan to use 
>>> `clang-format`. (BTW, here's the PR 
>>> https://github.com/apple/swift/pull/6922).
>>> 
>>> During the code review, I ran `git clang-format` as part of being a good 
>>> citizen. There's only one problem with the tool. It rewraps long boolean 
>>> expressions, hiding those unsightly operators at the end of lines (after 
>>> all who wants to see those?).
>>> 
>>>  if (some_expression->with_calls() ||
>>>  another_expression->with(nested_calls()) &&
>>>  an_even_longer_expression->making_your_eyes->glaze_over())
>>> 
>>> I don't get involved in style wars, but this is not a style change, it's an 
>>> objective code quality degradation. It's a demonstrably bug-prone thing to 
>>> do. It's hurt me too many times in the past, and I'm not happy using a 
>>> formatting tool that injects future bugs and harms code comprehension.
>> 
>> It's funny you'd mention this! I often format code that way, not out of any 
>> great love of it, but from muscle-memory of living under an old coding 
>> guideline somewhere in the distant past claiming that the ugliness of 
>> trailing unfinished-binops draws the eye to them and makes the user pay 
>> attention. Doug Crockford recommends this style; but of course Don Knuth 
>> agrees with you. I don't feel strongly about them as such, but I feel ... 
>> anti-strongly, I guess? Like changing that one thing isn't worth a 
>> cross-codebase rewrite / merge collision.
> 
> I’m not sure who’s recommending what. The above style obscures operators. 
> Does anyone disagree with that? A lot of code has been written in that way; I 
> think because developers value aesthetics over clarity, or just don’t think 
> about it. I care because when something doesn’t stand out, my brain fills in 
> the gaps with whatever it expects. For me, that leads to a bunch of silly 
> logic errors.
> 
> I need to see it this way:
> 
>  if (some_expression->with_calls()
>  || another_expression->with(nested_calls())
>  && an_even_longer_expression->making_your_eyes->glaze_over())
> 
> The need for parens now stands out. Sorry this isn’t a good example. Nested 
> expressions would make it much more compelling.
> 
> That’s the coding convention we use for Swift code (at least in the stdlib). 
> The compiler C++ code is just a hodge-podge.
> If anyone actually thinks trailing operators are a good idea for our code 
> base, I won’t argue, but I’ve never heard that argument.
> 
> BTW- I’m not interested at all in doing a mass reformatting or forcing a 
> convention on people. I just don’t want to apply clang-format to every line 
> of code I touch without knowing what settings to use.

I've never had a problem with the trailing operators, and find them mildly more 
aesthetically pleasing (when not in an if, it makes it clearer what I plan to 
do with the next line), but I see how they put it in your face in an if. I can 
change my style.

Jordan

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Andrew Trick via swift-dev]

> On Jan 26, 2017, at 11:38 AM, Graydon Hoare  wrote:
> 
> 
>> On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev 
>>  wrote:
>> 
>> Before I pull in a large PR that "accidentally" reformats a bunch of code, I 
>> want to get feedback on how Swift compiler devs plan to use `clang-format`. 
>> (BTW, here's the PR https://github.com/apple/swift/pull/6922).
>> 
>> During the code review, I ran `git clang-format` as part of being a good 
>> citizen. There's only one problem with the tool. It rewraps long boolean 
>> expressions, hiding those unsightly operators at the end of lines (after all 
>> who wants to see those?).
>> 
>>   if (some_expression->with_calls() ||
>>   another_expression->with(nested_calls()) &&
>>   an_even_longer_expression->making_your_eyes->glaze_over())
>> 
>> I don't get involved in style wars, but this is not a style change, it's an 
>> objective code quality degradation. It's a demonstrably bug-prone thing to 
>> do. It's hurt me too many times in the past, and I'm not happy using a 
>> formatting tool that injects future bugs and harms code comprehension.
> 
> It's funny you'd mention this! I often format code that way, not out of any 
> great love of it, but from muscle-memory of living under an old coding 
> guideline somewhere in the distant past claiming that the ugliness of 
> trailing unfinished-binops draws the eye to them and makes the user pay 
> attention. Doug Crockford recommends this style; but of course Don Knuth 
> agrees with you. I don't feel strongly about them as such, but I feel ... 
> anti-strongly, I guess? Like changing that one thing isn't worth a 
> cross-codebase rewrite / merge collision.

I’m not sure who’s recommending what. The above style obscures operators. Does 
anyone disagree with that? A lot of code has been written in that way; I think 
because developers value aesthetics over clarity, or just don’t think about it. 
I care because when something doesn’t stand out, my brain fills in the gaps 
with whatever it expects. For me, that leads to a bunch of silly logic errors.

I need to see it this way:

  if (some_expression->with_calls()
  || another_expression->with(nested_calls())
  && an_even_longer_expression->making_your_eyes->glaze_over())

The need for parens now stands out. Sorry this isn’t a good example. Nested 
expressions would make it much more compelling.

That’s the coding convention we use for Swift code (at least in the stdlib). 
The compiler C++ code is just a hodge-podge.
If anyone actually thinks trailing operators are a good idea for our code base, 
I won’t argue, but I’ve never heard that argument.

BTW- I’m not interested at all in doing a mass reformatting or forcing a 
convention on people. I just don’t want to apply clang-format to every line of 
code I touch without knowing what settings to use.

-Andy
___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Graydon Hoare via swift-dev]

> On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev  
> wrote:
> 
> Before I pull in a large PR that "accidentally" reformats a bunch of code, I 
> want to get feedback on how Swift compiler devs plan to use `clang-format`. 
> (BTW, here's the PR https://github.com/apple/swift/pull/6922).
> 
> During the code review, I ran `git clang-format` as part of being a good 
> citizen. There's only one problem with the tool. It rewraps long boolean 
> expressions, hiding those unsightly operators at the end of lines (after all 
> who wants to see those?).
> 
>if (some_expression->with_calls() ||
>another_expression->with(nested_calls()) &&
>an_even_longer_expression->making_your_eyes->glaze_over())
> 
> I don't get involved in style wars, but this is not a style change, it's an 
> objective code quality degradation. It's a demonstrably bug-prone thing to 
> do. It's hurt me too many times in the past, and I'm not happy using a 
> formatting tool that injects future bugs and harms code comprehension.

It's funny you'd mention this! I often format code that way, not out of any 
great love of it, but from muscle-memory of living under an old coding 
guideline somewhere in the distant past claiming that the ugliness of trailing 
unfinished-binops draws the eye to them and makes the user pay attention. Doug 
Crockford recommends this style; but of course Don Knuth agrees with you. I 
don't feel strongly about them as such, but I feel ... anti-strongly, I guess? 
Like changing that one thing isn't worth a cross-codebase rewrite / merge 
collision.

That said ...

> ** Option 2: Contributors each tweak clang-format according to their (in my 
> case strong) bias:

This option seems least-desirable to me. My preference would be that code stays 
formatted as uniformly and invariably as possible; I'm surprised to notice we 
have a .clang-format in the repo, given that it's not being enforced (and 
there's not even a build-system rule to use it, as far as I can see). How would 
people feel about automatic enforcement? i.e. testing a patch fails if 
clang-format has nonempty tree-reformatting to do.

-Graydon

(Who is happy to setq c-indentation-style to anything from Allman to Whitesmith)

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Michael Gottesman via swift-dev]

Can I make a more radical suggestion. Maybe the thing to do here is to enforce 
git-clang-format so that everyone uses it. It would be really simple to do. You 
would require people to put it in a pre commit hook. Then swift-ci would have a 
job that ran git-clang-format and would require it to come through clean.

Michael

> On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev  
> wrote:
> 
> Before I pull in a large PR that "accidentally" reformats a bunch of code, I 
> want to get feedback on how Swift compiler devs plan to use `clang-format`. 
> (BTW, here's the PR https://github.com/apple/swift/pull/6922).
> 
> During the code review, I ran `git clang-format` as part of being a good 
> citizen. There's only one problem with the tool. It rewraps long boolean 
> expressions, hiding those unsightly operators at the end of lines (after all 
> who wants to see those?).
> 
>if (some_expression->with_calls() ||
>another_expression->with(nested_calls()) &&
>an_even_longer_expression->making_your_eyes->glaze_over())
> 
> I don't get involved in style wars, but this is not a style change, it's an 
> objective code quality degradation. It's a demonstrably bug-prone thing to 
> do. It's hurt me too many times in the past, and I'm not happy using a 
> formatting tool that injects future bugs and harms code comprehension.
> 
> When the LLVM coding style was originally ratified, this aspect was left up 
> to individual preference and didn't get any discussion AFAIK. I think
> clang-format then ended up with a `BreakBeforeBinaryOperators: None` style 
> out of sheer inertia. 
> 
> So, how should I use clang-format on Swift compiler code? Vote please.
> 
> ** Option 1: Add a simple configuration option to swift/.clang-format:
> 
> 1a. BreakBeforeBinaryOperators: All
> 
> 1b. BreakBeforeBinaryOperators: NonAssignment
> 
> I have absolutely no preference between 1a and 1b. It's purely style.
> 
> 1a:
> SomeLongTypeName someLongVariableName =
>  someLongExpression();
> 
> 1b:
> SomeLongTypeName someLongVariableName
>  = someLongExpression();
> 
> ** Option 2: Contributors each tweak clang-format according to their (in my 
> case strong) bias:
> 
> My own command line:
> 2a. " --style "{BasedOnStyle: LLVM, BreakBeforeBinaryOperators: All}"
> 2b. " --style "{BasedOnStyle: LLVM, BreakBeforeBinaryOperators: 
> NonAssignment}"
> 
> ** Option 3: Don't bother using clang-format.
> 
> Let emacs do its indentation thing. Embrace idiosyncrasies in the code base.
> 
> -Andy
> ___
> swift-dev mailing list
> swift-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Andrew Trick via swift-dev]

> On Jan 26, 2017, at 10:34 AM, Andrew Trick via swift-dev 
>  wrote:
> 
> 
>> On Jan 26, 2017, at 10:25 AM, Jordan Rose > > wrote:
>> 
>>> 
>>> On Jan 26, 2017, at 09:35, Andrew Trick via swift-dev >> > wrote:
>>> 
 
 On Jan 26, 2017, at 9:29 AM, Ben Langmuir > wrote:
 
> 
> On Jan 26, 2017, at 9:14 AM, Andrew Trick  > wrote:
> 
> 
>> On Jan 26, 2017, at 9:11 AM, Ben Langmuir > > wrote:
>>> 
>>> ** Option 1: Add a simple configuration option to swift/.clang-format:
>>> 
>>> 1a. BreakBeforeBinaryOperators: All
>>> 
>>> 1b. BreakBeforeBinaryOperators: NonAssignment
>> 
>>> 
>>> I have absolutely no preference between 1a and 1b. It's purely style.
>>> 
>>> 1a:
>>> SomeLongTypeName someLongVariableName =
>>> someLongExpression();
>>> 
>>> 1b:
>>> SomeLongTypeName someLongVariableName
>>> = someLongExpression();
>> 
>> 1b sounds good to me.
> 
> I contradicted myself above. If you like the style shown in (1b), the 
> configuration option is actually BreakBeforeBinaryOperators: All.
 
 Glad you mentioned it, because I prefer  “NonAssignment”, but didn’t check 
 your example code against the above description :-)
>>> 
>>> Alright, I’l reformat my PR with that config, unless anyone else wants to 
>>> weigh in.
>>> 
>>> Incidentally, I despise what clang-format does with asserts now:
>>>   assert(condition
>>>  && “text”)
>>> 
>>> It’s a consequence of us not using a legit assert package, so I don’t know 
>>> if I want to push to get clang-format changed.
>> 
>> What do you want it to do with this style of assert?
> 
> assert((precondition1 || predcondition2) &&
>“informative message”)
> 
> The boolean operator is filling in for a comma. That’s how it’s meant to be 
> parsed by the reader. It’s presence in the code is purely distracting. From 
> the tool’s point of view, it’s tautological so shouldn’t be given 
> significance as a boolean operator.

clang-format bug filed to put this tangent to rest:
https://llvm.org/bugs/show_bug.cgi?id=31772 


-Andy

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Andrew Trick via swift-dev]

> On Jan 26, 2017, at 10:25 AM, Jordan Rose  wrote:
> 
>> 
>> On Jan 26, 2017, at 09:35, Andrew Trick via swift-dev > > wrote:
>> 
>>> 
>>> On Jan 26, 2017, at 9:29 AM, Ben Langmuir >> > wrote:
>>> 
 
 On Jan 26, 2017, at 9:14 AM, Andrew Trick > wrote:
 
 
> On Jan 26, 2017, at 9:11 AM, Ben Langmuir  > wrote:
>> 
>> ** Option 1: Add a simple configuration option to swift/.clang-format:
>> 
>> 1a. BreakBeforeBinaryOperators: All
>> 
>> 1b. BreakBeforeBinaryOperators: NonAssignment
> 
>> 
>> I have absolutely no preference between 1a and 1b. It's purely style.
>> 
>> 1a:
>> SomeLongTypeName someLongVariableName =
>> someLongExpression();
>> 
>> 1b:
>> SomeLongTypeName someLongVariableName
>> = someLongExpression();
> 
> 1b sounds good to me.
 
 I contradicted myself above. If you like the style shown in (1b), the 
 configuration option is actually BreakBeforeBinaryOperators: All.
>>> 
>>> Glad you mentioned it, because I prefer  “NonAssignment”, but didn’t check 
>>> your example code against the above description :-)
>> 
>> Alright, I’l reformat my PR with that config, unless anyone else wants to 
>> weigh in.
>> 
>> Incidentally, I despise what clang-format does with asserts now:
>>   assert(condition
>>  && “text”)
>> 
>> It’s a consequence of us not using a legit assert package, so I don’t know 
>> if I want to push to get clang-format changed.
> 
> What do you want it to do with this style of assert?

assert((precondition1 || predcondition2) &&
   “informative message”)

The boolean operator is filling in for a comma. That’s how it’s meant to be 
parsed by the reader. It’s presence in the code is purely distracting. From the 
tool’s point of view, it’s tautological so shouldn’t be given significance as a 
boolean operator.

-Andy

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Jordan Rose via swift-dev]

> On Jan 26, 2017, at 09:35, Andrew Trick via swift-dev  
> wrote:
> 
>> 
>> On Jan 26, 2017, at 9:29 AM, Ben Langmuir > > wrote:
>> 
>>> 
>>> On Jan 26, 2017, at 9:14 AM, Andrew Trick >> > wrote:
>>> 
>>> 
 On Jan 26, 2017, at 9:11 AM, Ben Langmuir > wrote:
> 
> ** Option 1: Add a simple configuration option to swift/.clang-format:
> 
> 1a. BreakBeforeBinaryOperators: All
> 
> 1b. BreakBeforeBinaryOperators: NonAssignment
 
> 
> I have absolutely no preference between 1a and 1b. It's purely style.
> 
> 1a:
> SomeLongTypeName someLongVariableName =
> someLongExpression();
> 
> 1b:
> SomeLongTypeName someLongVariableName
> = someLongExpression();
 
 1b sounds good to me.
>>> 
>>> I contradicted myself above. If you like the style shown in (1b), the 
>>> configuration option is actually BreakBeforeBinaryOperators: All.
>> 
>> Glad you mentioned it, because I prefer  “NonAssignment”, but didn’t check 
>> your example code against the above description :-)
> 
> Alright, I’l reformat my PR with that config, unless anyone else wants to 
> weigh in.
> 
> Incidentally, I despise what clang-format does with asserts now:
>   assert(condition
>  && “text”)
> 
> It’s a consequence of us not using a legit assert package, so I don’t know if 
> I want to push to get clang-format changed.

What do you want it to do with this style of assert?

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Andrew Trick via swift-dev]

> On Jan 26, 2017, at 9:29 AM, Ben Langmuir  wrote:
> 
>> 
>> On Jan 26, 2017, at 9:14 AM, Andrew Trick > > wrote:
>> 
>> 
>>> On Jan 26, 2017, at 9:11 AM, Ben Langmuir >> > wrote:
 
 ** Option 1: Add a simple configuration option to swift/.clang-format:
 
 1a. BreakBeforeBinaryOperators: All
 
 1b. BreakBeforeBinaryOperators: NonAssignment
>>> 
 
 I have absolutely no preference between 1a and 1b. It's purely style.
 
 1a:
 SomeLongTypeName someLongVariableName =
 someLongExpression();
 
 1b:
 SomeLongTypeName someLongVariableName
 = someLongExpression();
>>> 
>>> 1b sounds good to me.
>> 
>> I contradicted myself above. If you like the style shown in (1b), the 
>> configuration option is actually BreakBeforeBinaryOperators: All.
> 
> Glad you mentioned it, because I prefer  “NonAssignment”, but didn’t check 
> your example code against the above description :-)

Alright, I’l reformat my PR with that config, unless anyone else wants to weigh 
in.

Incidentally, I despise what clang-format does with asserts now:
  assert(condition
 && “text”)

It’s a consequence of us not using a legit assert package, so I don’t know if I 
want to push to get clang-format changed.

-Andy___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Ben Langmuir via swift-dev]

> On Jan 26, 2017, at 9:14 AM, Andrew Trick  wrote:
> 
> 
>> On Jan 26, 2017, at 9:11 AM, Ben Langmuir > > wrote:
>>> 
>>> ** Option 1: Add a simple configuration option to swift/.clang-format:
>>> 
>>> 1a. BreakBeforeBinaryOperators: All
>>> 
>>> 1b. BreakBeforeBinaryOperators: NonAssignment
>> 
>>> 
>>> I have absolutely no preference between 1a and 1b. It's purely style.
>>> 
>>> 1a:
>>> SomeLongTypeName someLongVariableName =
>>> someLongExpression();
>>> 
>>> 1b:
>>> SomeLongTypeName someLongVariableName
>>> = someLongExpression();
>> 
>> 1b sounds good to me.
> 
> I contradicted myself above. If you like the style shown in (1b), the 
> configuration option is actually BreakBeforeBinaryOperators: All.

Glad you mentioned it, because I prefer  “NonAssignment”, but didn’t check your 
example code against the above description :-)

> -Andy

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Andrew Trick via swift-dev]

> On Jan 26, 2017, at 9:11 AM, Ben Langmuir  wrote:
>> 
>> ** Option 1: Add a simple configuration option to swift/.clang-format:
>> 
>> 1a. BreakBeforeBinaryOperators: All
>> 
>> 1b. BreakBeforeBinaryOperators: NonAssignment
> 
>> 
>> I have absolutely no preference between 1a and 1b. It's purely style.
>> 
>> 1a:
>> SomeLongTypeName someLongVariableName =
>> someLongExpression();
>> 
>> 1b:
>> SomeLongTypeName someLongVariableName
>> = someLongExpression();
> 
> 1b sounds good to me.

I contradicted myself above. If you like the style shown in (1b), the 
configuration option is actually BreakBeforeBinaryOperators: All.

-Andy
___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[Ben Langmuir via swift-dev]

> On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev  
> wrote:
> 
> Before I pull in a large PR that "accidentally" reformats a bunch of code, I 
> want to get feedback on how Swift compiler devs plan to use `clang-format`. 
> (BTW, here's the PR https://github.com/apple/swift/pull/6922).
> 
> During the code review, I ran `git clang-format` as part of being a good 
> citizen. There's only one problem with the tool. It rewraps long boolean 
> expressions, hiding those unsightly operators at the end of lines (after all 
> who wants to see those?).
> 
>if (some_expression->with_calls() ||
>another_expression->with(nested_calls()) &&
>an_even_longer_expression->making_your_eyes->glaze_over())
> 
> I don't get involved in style wars, but this is not a style change, it's an 
> objective code quality degradation. It's a demonstrably bug-prone thing to 
> do. It's hurt me too many times in the past, and I'm not happy using a 
> formatting tool that injects future bugs and harms code comprehension.
> 
> When the LLVM coding style was originally ratified, this aspect was left up 
> to individual preference and didn't get any discussion AFAIK. I think
> clang-format then ended up with a `BreakBeforeBinaryOperators: None` style 
> out of sheer inertia. 
> 
> So, how should I use clang-format on Swift compiler code? Vote please.
> 
> ** Option 1: Add a simple configuration option to swift/.clang-format:
> 
> 1a. BreakBeforeBinaryOperators: All
> 
> 1b. BreakBeforeBinaryOperators: NonAssignment

> 
> I have absolutely no preference between 1a and 1b. It's purely style.
> 
> 1a:
> SomeLongTypeName someLongVariableName =
>  someLongExpression();
> 
> 1b:
> SomeLongTypeName someLongVariableName
>  = someLongExpression();

1b sounds good to me.

> 
> ** Option 2: Contributors each tweak clang-format according to their (in my 
> case strong) bias:
> 
> My own command line:
> 2a. " --style "{BasedOnStyle: LLVM, BreakBeforeBinaryOperators: All}"
> 2b. " --style "{BasedOnStyle: LLVM, BreakBeforeBinaryOperators: 
> NonAssignment}"
> 
> ** Option 3: Don't bother using clang-format.
> 
> Let emacs do its indentation thing. Embrace idiosyncrasies in the code base.
> 
> -Andy
> ___
> swift-dev mailing list
> swift-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Re: [swift-dev] Using git-clang-format in the Swift compiler code base.

[John McCall via swift-dev]

> On Jan 26, 2017, at 5:07 AM, Andrew Trick via swift-dev  
> wrote:
> Before I pull in a large PR that "accidentally" reformats a bunch of code, I 
> want to get feedback on how Swift compiler devs plan to use `clang-format`. 
> (BTW, here's the PR https://github.com/apple/swift/pull/6922).
> 
> During the code review, I ran `git clang-format` as part of being a good 
> citizen. There's only one problem with the tool. It rewraps long boolean 
> expressions, hiding those unsightly operators at the end of lines (after all 
> who wants to see those?).
> 
>if (some_expression->with_calls() ||
>another_expression->with(nested_calls()) &&
>an_even_longer_expression->making_your_eyes->glaze_over())

Well, I mean, I hope nobody mixes || and && like this. :)

Anyway, a vote:

> I don't get involved in style wars, but this is not a style change, it's an 
> objective code quality degradation. It's a demonstrably bug-prone thing to 
> do. It's hurt me too many times in the past, and I'm not happy using a 
> formatting tool that injects future bugs and harms code comprehension.
> 
> When the LLVM coding style was originally ratified, this aspect was left up 
> to individual preference and didn't get any discussion AFAIK. I think
> clang-format then ended up with a `BreakBeforeBinaryOperators: None` style 
> out of sheer inertia. 
> 
> So, how should I use clang-format on Swift compiler code? Vote please.
> 
> ** Option 1: Add a simple configuration option to swift/.clang-format:
> 
> 1a. BreakBeforeBinaryOperators: All
> 
> 1b. BreakBeforeBinaryOperators: NonAssignment
> 
> I have absolutely no preference between 1a and 1b. It's purely style.

> 
> 1a:
> SomeLongTypeName someLongVariableName =
>  someLongExpression();
> 
> 1b:
> SomeLongTypeName someLongVariableName
>  = someLongExpression();
> 
> ** Option 2: Contributors each tweak clang-format according to their (in my 
> case strong) bias:
> 
> My own command line:
> 2a. " --style "{BasedOnStyle: LLVM, BreakBeforeBinaryOperators: All}"
> 2b. " --style "{BasedOnStyle: LLVM, BreakBeforeBinaryOperators: 
> NonAssignment}"
> 
> ** Option 3: Don't bother using clang-format.
> 
> Let emacs do its indentation thing. Embrace idiosyncrasies in the code base.

I agree with your point that we should discourage breaking after the logical
operators, and maybe also + and so on.  Relational and assignment operators
I care less about, and I don't see a strong reason to force a choice, so I'm 
with
option #2.

John.

> 
> -Andy
> ___
> swift-dev mailing list
> swift-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

[swift-dev] Using git-clang-format in the Swift compiler code base.

[Andrew Trick via swift-dev]

Before I pull in a large PR that "accidentally" reformats a bunch of code, I 
want to get feedback on how Swift compiler devs plan to use `clang-format`. 
(BTW, here's the PR https://github.com/apple/swift/pull/6922).

During the code review, I ran `git clang-format` as part of being a good 
citizen. There's only one problem with the tool. It rewraps long boolean 
expressions, hiding those unsightly operators at the end of lines (after all 
who wants to see those?).

if (some_expression->with_calls() ||
another_expression->with(nested_calls()) &&
an_even_longer_expression->making_your_eyes->glaze_over())

I don't get involved in style wars, but this is not a style change, it's an 
objective code quality degradation. It's a demonstrably bug-prone thing to do. 
It's hurt me too many times in the past, and I'm not happy using a formatting 
tool that injects future bugs and harms code comprehension.

When the LLVM coding style was originally ratified, this aspect was left up to 
individual preference and didn't get any discussion AFAIK. I think
clang-format then ended up with a `BreakBeforeBinaryOperators: None` style out 
of sheer inertia. 

So, how should I use clang-format on Swift compiler code? Vote please.

** Option 1: Add a simple configuration option to swift/.clang-format:

1a. BreakBeforeBinaryOperators: All

1b. BreakBeforeBinaryOperators: NonAssignment

I have absolutely no preference between 1a and 1b. It's purely style.

1a:
SomeLongTypeName someLongVariableName =
  someLongExpression();

1b:
SomeLongTypeName someLongVariableName
  = someLongExpression();

** Option 2: Contributors each tweak clang-format according to their (in my 
case strong) bias:

My own command line:
2a. " --style "{BasedOnStyle: LLVM, BreakBeforeBinaryOperators: All}"
2b. " --style "{BasedOnStyle: LLVM, BreakBeforeBinaryOperators: NonAssignment}"

** Option 3: Don't bother using clang-format.

Let emacs do its indentation thing. Embrace idiosyncrasies in the code base.

-Andy
___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev