Re: [RFC] clang-format: outline the git project's coding style

On 08/08, Junio C Hamano wrote:

Brandon Williams [off-list ref] writes:

quoted

Add a '.clang-format' file which outlines the git project's coding
style.  This can be used with clang-format to auto-format .c and .h
files to conform with git's style.

Signed-off-by: Brandon Williams <redacted>
---

I'm sure this sort of thing comes up every so often on the list but back at
git-merge I mentioned how it would be nice to not have to worry about style
when reviewing patches as that is something mechanical and best left to a
machine (for the most part).  I saw that 'clang-format' was brought up on the
list once before a couple years ago
(https://public-inbox.org/git/20150121220903.GA10267@peff.net/) but nothing
really came of it.  I spent a little bit of time combing through the various
options and came up with this config based on the general style of our code
base.  The big issue though is that our code base isn't consistent so try as
you might you wont be able to come up with a config which matches everything we
do (mostly due to the inconsistencies in our code base).

Anyway, I thought I'd bring this topic back up and see how people feel about it.

I gave just one pass over all the rules you have here.  I didn't
think too deeply about implications of some of them, but most of
them looked sensible.  Thanks for compiling this set of rules.

Having said that, it is easier to refine individual rules you have
below than to make sure that among the develoepers there is a shared
notion of how these rules are to be used.  If we get that part wrong,
we'd see unpleasant consequences, e.g.

 - We may see unwanted code churn on existing codebase, only for the
   sake of satisfying the formatting rules specified here.

This is an issue when you have an inconsistent code base such as ours as
the tool, even when used to format a diff, will format the surrounding
context.

 - We may see far more style-only critique to patches posted on the
   list simply because there are more readers than writers, and it
   is likely that running the tool to nitpick other people's patches
   is far easier than writing these patches in the first place (and
   forgetting to ask the formatter to nitpick before sending them
   out).

I would hope that use of such a tool would eventually completely
eliminate style-only critiques.  

 - Human aesthetics judgement often is necessary to overrule
   mechanical rules (e.g. A human may have two pairs of <ptr, len>
   parameters on separate lines in a function declaration;
   BinPackParameters may try to break after ptrA, lenA, ptrB before
   lenB in such a case).

I know that when you introduce a formatter there are bound to be some
issues where a human would choose one way over the other.  If we start
using a formatter I would expect some of the penalties would need to be
tweaked overtime before we rely too heavily on it.

We need to set our expectation and a guideline to apply these rules
straight, before introducing something like this.

quoted

 .clang-format | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 166 insertions(+)
 create mode 100644 .clang-format

diff --git a/.clang-format b/.clang-format
new file mode 100644
index 000000000..7f28dc259
--- /dev/null
+++ b/.clang-format

@@ -0,0 +1,166 @@
+# Defaults
+
+# Use tabs whenever we need to fill whitespace that spans at least from one tab
+# stop to the next one.
+UseTab: Always
+TabWidth: 8
+IndentWidth: 8
+ContinuationIndentWidth: 8
+ColumnLimit: 80

I often deliberately chomp my lines much shorter than this limit,
and also I deliberately write a line that is a tad longer than this
limit some other times, if doing so makes the result easier to read.
And I know other develoepers with good taste do the same.  It is
pointless to have a discussion that begins with "who uses a physical
terminal these days that can only show 80-columns?" to tweak this
value, I think.  It is more important to give a guideline on what to
do when lines in your code goes over this limit.

A mechanical "formatter" would just find an appropriate point in a
line with least "penalty" and chomp it into two.  But an appropriate
way to make such a code that is way too deeply indented readable may
instead be judicious use of goto's and one-time helper functions,
for example, which mechanical tools would not do.

That is an example of what I meant above, i.e. a guideline to apply
these rules.  We would not want to say "clang-format suggests this
rewrite, so we should accept the resulting code that is still too
deeply indented as-is"---using the tool to point out an issue is
good, though.

quoted

+
+# C Language specifics
+Language: Cpp

Hmph ;-)

Well there's no 'C' Lang option so Cpp is the closest there is ;)

quoted

+# Add a line break after the return type of top-level functions
+# int
+# foo();
+AlwaysBreakAfterReturnType: TopLevel

We do that?

Haha So generally no we don't do this.  Though there are definitely many
places in our code base where we do.  Personally this makes it a bit
easier to read when you end up having long function names.  I also
worked on a code base which did this and it made it incredible easy to
grep for the definition of a function.  If you grep for 'foo()' then
you'd get all the uses of the function including the definition but if
you grep for '^foo()' you'd get only the definition.

But that's my preference, if we end up using this tool it would probably
make sense to change this.

quoted

+# Pack as many parameters or arguments onto the same line as possible
+# int myFunction(int aaaaaaaaaaaa, int bbbbbbbb,
+#                int cccc);
+BinPackArguments: true
+BinPackParameters: true

I am OK with this but with the caveats I already mentioned.

The alternative (with this config) is to have arguments and parameters
each on their own separate line.

quoted

+# Insert a space after a cast
+# x = (int32) y;    not    x = (int32)y;
+SpaceAfterCStyleCast: true

Hmph, I thought we did the latter, i.e. cast sticks to the casted
expression without SP.

I've seen both and I wasn't sure which was the correct form to use.

Thanks.

-- 
Brandon Williams