<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0cm;
mso-margin-bottom-alt:auto;
margin-left:0cm;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;
mso-fareast-language:EN-US;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:3.0cm 2.0cm 3.0cm 2.0cm;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="DA" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">I am okay with using the standard setup. It seems quite readable to me. I do have one wish though.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">The default seem to use two spaces per indentation level. In the existing code we mostly use 4 spaces.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">I would prefer to keep it that way. I think it looks better, but the real selling point for me is that it<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">lowers the amount of lines that will be changed should we ever want to reformat the C-code with<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">clang-format (of course using the same rules as for C++).<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">I’ve been playing around with it a bit to see the effect on the code in the repository today. Adding<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">“IndentWidth: 4” to the clang-format setup touches ~20k lines less than the default setup. That<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">seems worthwhile to me. I’ve uploaded branches with the changes to my fork.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">Default LLVM formatting:
<a href="https://github.com/OSGeo/proj.4/compare/master...kbevers:clang-format-test">
https://github.com/OSGeo/proj.4/compare/master...kbevers:clang-format-test</a><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">LLVM + IndenWidth: 4:
<a href="https://github.com/OSGeo/proj.4/compare/master...kbevers:clang-format-test2">
https://github.com/OSGeo/proj.4/compare/master...kbevers:clang-format-test2</a><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">I tried adding a few other options to see if that would bring down the number of affected lines<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">by a significant amount but without any luck. I guess that is a testament to how varied the code<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">formatting is across the many files in the repository.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">It is interesting to browse through the diffs of the reformatted code. Generally clang-format does<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">a good job of reformatting the code but there are of course some situations where the author<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">of the code has formatted the code very carefully which clang-format disrupts. Arguably that is<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">a step backwards. At least locally – across the whole codebase the improvements are for the<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">better!<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">So to sum up, if we can add an indentation level of 4 spaces on top of the default LLVM style<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">I would be happy.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US">/Kristian<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif">Fra:</span></b><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Kurt Schwehr [mailto:schwehr@gmail.com]
<br>
<b>Sendt:</b> 27. maj 2018 17:08<br>
<b>Til:</b> Even Rouault <even.rouault@spatialys.com><br>
<b>Cc:</b> Kris</span><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">tian Evers <kreve@sdfe.dk>; PROJ.4 and general Projections Discussions <proj@lists.maptools.org><br>
<b>Emne:</b> Re: C++ formatting rules [was Re: [Proj] Use of C++]<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">+1 to Even's suggestion.<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">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<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Sun, May 27, 2018, 6:45 AM Even Rouault <<a href="mailto:even.rouault@spatialys.com">even.rouault@spatialys.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm">
<p class="MsoNormal">> For this reason I would ask that some of the<br>
> C++ experts in the community come up with a set of coding guidelines for<br>
> the C++ parts of the code base. Lately the lack of code formatting<br>
> conventions in PROJ has been frustrating to several contributors. With the<br>
> addition of the new C++ parts of the code we have the chance to at least<br>
> include conventions for the C++ code. I know this has been a topic for GDAL<br>
> recently and since we have a big overlap with the GDAL community perhaps we<br>
> can benefit from their experiences. Kurt and Even, I believe you’ve been<br>
> involved in improving the GDAL code formatting rules, would one of you be<br>
> willing to suggest something that we can use in PROJ? A good starting<br>
> point, I guess, would be<br>
> <a href="https://trac.osgeo.org/gdal/wiki/rfc69_cplusplus_formatting" target="_blank">
https://trac.osgeo.org/gdal/wiki/rfc69_cplusplus_formatting</a>.<br>
> <br>
<br>
One possibility would be to claim "this project has no particular C++ code <br>
formating rules. Do reasonable things [1]". But I'm afraid that won't be <br>
enough.<br>
<br>
I've experimented a bit with the suggested clang-format. Basically why not <br>
just using it in its default setup (LLVM style), without a particular .clang-<br>
format ?<br>
<br>
And have a scripts/autoformat.sh [2], to autoreformat things. Travis-CI could <br>
run it, and bail out if a file has been reformatted.<br>
<br>
Equivalently to my above adhoc script, I also see there is a 'git clang-<br>
format' tool that automatically runs clang-format on files that have been git <br>
added. Actually when testing it it seems to run it only on the parts you <br>
changed, probably to avoid causing code churn in non-modified parts: cf <br>
<a href="https://electronjs.org/docs/development/clang-format" target="_blank">https://electronjs.org/docs/development/clang-format</a><br>
<br>
That said there might be a slight risk of output instability depending on the <br>
clang-format version. I've tried with the one of LLVM 3.7, 3.8 and 7dev<br>
The good news is that the output of 3.8 and 7dev is identical on the .cpp <br>
files I've sketched.<br>
<br>
There was a difference with 3.7 and later versions regarding include sorting <br>
header (with 3.7, in foo.cpp, include "foo.h" must come first, whereas later <br>
versions insist on includes being sorted, accepting as an exception that <br>
"foo.h" is first, probably for compat with 3.7...). But 3.7 can probably be <br>
considered ancient, so let's aim for >= 3.8<br>
<br>
Even<br>
<br>
[1] Reminds me of road signs in Montana "Drive at reasonable speed"...<br>
<br>
[2]<br>
<br>
#!/bin/sh<br>
set -eu<br>
clang-format $1 > $1.reformatted<br>
if diff -u $1.reformatted $1; then<br>
# No reformatting: remove temporary file<br>
rm $1.reformatted<br>
else<br>
# Differences. Backup original file, and use reformatted file<br>
cp $1 $1.before_reformat<br>
mv $1.reformatted $1<br>
fi<br>
<br>
<br>
<br>
-- <br>
Spatialys - Geospatial professional services<br>
<a href="http://www.spatialys.com" target="_blank">http://www.spatialys.com</a><o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
</body>
</html>