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

[John McCall]

> On Jan 26, 2017, at 5:07 AM, Andrew Trick via swift-dev <swift-dev at swift.org> 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 at swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev