[Proj] C++ formatting rules [was Re: Use of C++]

[Kristian Evers]

I am okay with using the standard setup. It seems quite readable to me. I do have one wish though.
The default seem to use two spaces per indentation level. In the existing code we mostly use 4 spaces.
I would prefer to keep it that way. I think it looks better, but the real selling point for me is that it
lowers the amount of lines that will be changed should we ever want to reformat the C-code with
clang-format (of course using the same rules as for C++).

I’ve been playing around with it a bit to see the effect on the code in the repository today. Adding
“IndentWidth: 4” to the clang-format setup touches ~20k lines less than the default setup. That
seems worthwhile to me. I’ve uploaded branches with the changes to my fork.

Default LLVM formatting: https://github.com/OSGeo/proj.4/compare/master...kbevers:clang-format-test
LLVM + IndenWidth: 4: https://github.com/OSGeo/proj.4/compare/master...kbevers:clang-format-test2

I tried adding a few other options to see if that would bring down the number of affected lines
by a significant amount but without any luck. I guess that is a testament to how varied the code
formatting is across the many files in the repository.

It is interesting to browse through the diffs of the reformatted code. Generally clang-format does
a good job of reformatting the code but there are of course some situations where the author
of the code has formatted the code very carefully which clang-format disrupts. Arguably that is
a step backwards. At least locally – across the whole codebase the improvements are for the
better!

So to sum up, if we can add an indentation level of 4 spaces on top of the default LLVM style
I would be happy.

/Kristian

Fra: Kurt Schwehr [mailto:schwehr at gmail.com]
Sendt: 27. maj 2018 17:08
Til: Even Rouault <even.rouault at spatialys.com>
Cc: Kristian Evers <kreve at sdfe.dk>; PROJ.4 and general Projections Discussions <proj at lists.maptools.org>
Emne: Re: C++ formatting rules [was Re: [Proj] Use of C++]

+1 to Even's suggestion.

The default is the least complicated setup.  And you can always turn off formatting for small blocks that really need special formatting.  E.g.  a matrix


On Sun, May 27, 2018, 6:45 AM Even Rouault <even.rouault at spatialys.com<mailto:even.rouault at spatialys.com>> wrote:
> For this reason I would ask that some of the
> C++ experts in the community come up with a set of coding guidelines for
> the C++ parts of the code base. Lately the lack of code formatting
> conventions in PROJ has been frustrating to several contributors. With the
> addition of the new C++ parts of the code we have the chance to at least
> include conventions for the C++ code. I know this has been a topic for GDAL
> recently and since we have a big overlap with the GDAL community perhaps we
> can benefit from their experiences. Kurt and Even, I believe you’ve been
> involved in improving the GDAL code formatting rules, would one of you be
> willing to suggest something that we can use in PROJ? A good starting
> point, I guess, would be
> https://trac.osgeo.org/gdal/wiki/rfc69_cplusplus_formatting.
>

One possibility would be to claim "this project has no particular C++ code
formating rules. Do reasonable things [1]". But I'm afraid that won't be
enough.

I've experimented a bit with the suggested clang-format. Basically why not
just using it in its default setup (LLVM style), without a particular .clang-
format ?

And have a scripts/autoformat.sh [2], to autoreformat things. Travis-CI could
run it, and bail out if a file has been reformatted.

Equivalently to my above adhoc script, I also see there is a 'git clang-
format' tool that automatically runs clang-format on files that have been git
added. Actually when testing it it seems to run it only on the parts you
changed, probably to avoid causing code churn in non-modified parts: cf
https://electronjs.org/docs/development/clang-format

That said there might be a slight risk of output instability depending on the
clang-format version. I've tried with the one of LLVM 3.7, 3.8 and 7dev
The good news is that the output of 3.8 and 7dev is identical on the .cpp
files I've sketched.

There was a difference with 3.7 and later versions regarding include sorting
header (with 3.7, in foo.cpp, include "foo.h" must come first, whereas later
versions insist on includes being sorted, accepting as an exception that
"foo.h" is first, probably for compat with 3.7...). But 3.7 can probably be
considered ancient, so let's aim for >= 3.8

Even

[1] Reminds me of road signs in Montana "Drive at reasonable speed"...

[2]

#!/bin/sh
set -eu
clang-format $1 > $1.reformatted
if diff -u $1.reformatted $1; then
    # No reformatting: remove temporary file
    rm $1.reformatted
else
    # Differences. Backup original file, and use reformatted file
    cp $1 $1.before_reformat
    mv $1.reformatted $1
fi



--
Spatialys - Geospatial professional services
http://www.spatialys.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/proj/attachments/20180528/873890df/attachment.html>