Re: [GRASS-dev] Proposal for using ClangFormat, replacing GNU indent, for C/C++ code formatting

[Nicklas Larsson via grass-dev]

> On 26 Jan 2023, at 15:07, Nicklas Larsson  wrote:
> 
> The question now is, are there any objections to do the same treatment to 
> grass-addons)?
> There are three PRs in-store [3-5] to make that happen.
> 



I understand there are no objections to apply clang-format and do the general 
trailing-whitespace/EOF cleanup in grass-addons.

I will in short proceed and merge the related PRs [3-5].


> 
> 
> [1] https://pre-commit.com 
> [2] https://github.com/OSGeo/grass/pull/2765 
> 
> [3] https://github.com/OSGeo/grass-addons/pull/852 
> 
> [4] https://github.com/OSGeo/grass-addons/pull/853 
> 
> [5] https://github.com/OSGeo/grass-addons/pull/854 
> 
> 
> 




Best,
Nicklas___
grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

Re: [GRASS-dev] Proposal for using ClangFormat, replacing GNU indent, for C/C++ code formatting

[Nicklas Larsson via grass-dev]

Even,
Thanks for the pointer to pre-commit!


All,

The complete GRASS source is now formatted with ClangFormat using the settings 
of the added ‘.clang-format’ file.

I have also added a '.pre-commit-config.yaml’, which facilitates the convenient 
use of ‘pre-commit’ [1].

The pre-commit hooks now supported are:
-  clang-format (for C, C++, JavaScript, JSON, Objective-C)
-  trailing-whitespace
-  end-of-file-fixer
-  markdownlint
-  black
-  flake8


For all developers, I **strongly recommend** installing and using pre-commit. 
It makes life so much easier and unloads considerable amount of unnecessary 
work on the CI runners (oh, Black failed, now ClangFormat too, push again…).

The submitting guides on the wiki has been converted to markdown and are 
awaiting the final destination in source repo [2]. After merge they need to be 
updated accordingly to latest developments.

The question now is, are there any objections to do the same treatment to 
grass-addons)?
There are three PRs in-store [3-5] to make that happen.


Cheers,
Nicklas



[1] https://pre-commit.com
[2] https://github.com/OSGeo/grass/pull/2765
[3] https://github.com/OSGeo/grass-addons/pull/852
[4] https://github.com/OSGeo/grass-addons/pull/853
[5] https://github.com/OSGeo/grass-addons/pull/854




> On 2 Jan 2023, at 19:44, Even Rouault  wrote:
> 
> I'd suggest you use pre-commit so that clang-format is automatically run on 
> git commit operations like we have done with GDAL. Then it is a no-brainer to 
> do changes.
> 
> You need to add a .pre-commit-config.yaml at the root of the repository (only 
> the part referencing clang-format 
> athttps://github.com/OSGeo/gdal/blob/master/.pre-commit-config.yaml#L30 
>  is 
> relevant for you):
> 
> https://github.com/OSGeo/gdal/blob/master/.pre-commit-config.yaml 
> 
> Once that file is in place:
> 
> -  "pip install pre-commit" : just once
> 
> - "pre-commit install": just once per repository
> 
> Cf https://gdal.org/development/dev_practices.html#commit-hooks 
> 
> Even
> 
> Le 02/01/2023 à 19:20, Nicklas Larsson via grass-dev a écrit :
>> Markus,
>> 
>> 
>>> On 2 Jan 2023, at 13:48, Markus Neteler >> > wrote:
>>> 
>>> Hi Nicklas,
>>> 
>>> On Wed, Dec 21, 2022 at 9:25 PM Nicklas Larsson via grass-dev
>>> mailto:grass-dev@lists.osgeo.org>> wrote:
 
 I understand there is agreement on using the .clang-format formatting 
 rules suggested with [1], which I just merged.
 
 I have formatted the whole source base with clang-format v.15.0.6, in 7 
 different PRs [2-8]. I will start merging them tomorrow if there are no 
 objections.
 
 I have also filed a PR [9] which adds a CI check for clang-format errors.
>>> 
>>> Thanks for your efforts on the code reformatting!
>> 
>> :-)
>> 
>>> 
 Installing clang-format is perhaps most easily done with:
 python -m pip install 'clang-format==15.0.6'
 
 Formatting may be done with something like (following works on Mac):
 find -E . -regex '.*\.(cpp|hpp|c|h)' -exec clang-format -i {} \+
>>> 
>>> ... it fails on Linux, though
>>> 
>>> find: unknown predicate `-E')
>> 
>> 
>> I was pretty sure this would deviate from Mac/BSD on Linux systems:
>> 
>> 
>> Try something like (untested):
>> 
>> find . -regex '.*\.(cpp|hpp|c|h)' -exec clang-format -i {} \;
>> 
>> or manually with:
>> clang-format -I 
>> 
>> 
>>> 
 Contribution rules must be updated, I will start putting up a draft ASAP.
>>> 
>>> I am trying to fix the conflicts in
>>> https://github.com/OSGeo/grass/pull/2684 
>>> 
>>> 
>>> Conflicting files:
>>> 
>>> include/grass/iostream/mm.h
>>> lib/db/dbmi_base/dbmscap.c
>>> lib/external/ccmath/ccmath.h
>>> lib/gis/spawn.c
>>> lib/gis/user_config.c
>>> lib/iostream/rtimer.cpp
>>> lib/pngdriver/graph_set.c
>>> lib/rst/interp_float/point2d.c
>>> raster/r.terraflow/filldepr.cpp
>>> raster/r.terraflow/flow.cpp
>>> raster/r.terraflow/main.cpp
>>> raster/r.viewshed/statusstructure.cpp
>>> 
>>> The reason will be the missing clang-format update which I don't know
>>> how to apply on Linux.
>> 
>> Not quite sure what you mean. Did you try:
>> 
>> python -m pip install 'clang-format==15.0.6’
>> 
>> But I suspect any version 15 will do, perhaps even v. 14.
>> 
>> 
>> A tip to use the .clang-format file from main for branches without it:
>> 
>> 1. git checkout main
>> 2. cp .clang-forrmat ../.clang-format
>> 3. check out branch
>> 4. clang-format searches upwards in dir hierarchy  for next ‘.clang-format’ 
>> file
>> 5. run clang-format from grass source dir
>> 
>> 
>> 
>> 
>> ___
>> grass-dev mailing list
>> grass-dev@lists.osgeo.org 
>> 

Re: [GRASS-dev] Proposal for using ClangFormat, replacing GNU indent, for C/C++ code formatting

[Markus Neteler]

On Mon, Jan 2, 2023 at 7:20 PM Nicklas Larsson  wrote:
> On 2 Jan 2023, at 13:48, Markus Neteler  wrote:
>> On Wed, Dec 21, 2022 at 9:25 PM Nicklas Larsson via grass-dev
>>>
>>> Installing clang-format is perhaps most easily done with:
>>> python -m pip install 'clang-format==15.0.6'
>>>
>>> Formatting may be done with something like (following works on Mac):
>>> find -E . -regex '.*\.(cpp|hpp|c|h)' -exec clang-format -i {} \+
>>
>> ... it fails on Linux
>>
> Try something like (untested):
>
> find . -regex '.*\.(cpp|hpp|c|h)' -exec clang-format -i {} \;

That seems to work!

> or manually with:
> clang-format -I 
>
[...]
> I am trying to fix the conflicts in
> https://github.com/OSGeo/grass/pull/2684
>
> Conflicting files:
>
> include/grass/iostream/mm.h
> lib/db/dbmi_base/dbmscap.c
> lib/external/ccmath/ccmath.h
> lib/gis/spawn.c
> lib/gis/user_config.c
> lib/iostream/rtimer.cpp
> lib/pngdriver/graph_set.c
> lib/rst/interp_float/point2d.c
> raster/r.terraflow/filldepr.cpp
> raster/r.terraflow/flow.cpp
> raster/r.terraflow/main.cpp
> raster/r.viewshed/statusstructure.cpp
>
> The reason will be the missing clang-format update which I don't know
> how to apply on Linux.
>
>> Not quite sure what you mean.

Probably I was confused and it needs a rebase...

> Did you try:
>
> python -m pip install 'clang-format==15.0.6’
>
> But I suspect any version 15 will do, perhaps even v. 14.
>
> A tip to use the .clang-format file from main for branches without it:

... this I was looking for (sorry for my unclear wish)!

> 1. git checkout main
> 2. cp .clang-forrmat ../.clang-format
> 3. check out branch
> 4. clang-format searches upwards in dir hierarchy  for next ‘.clang-format’ 
> file
> 5. run clang-format from grass source dir

Will try.

Thanks,
Markus
___
grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

Re: [GRASS-dev] Proposal for using ClangFormat, replacing GNU indent, for C/C++ code formatting

[Even Rouault]

I'd suggest you use pre-commit so that clang-format is automatically run 
on git commit operations like we have done with GDAL. Then it is a 
no-brainer to do changes.


You need to add a .pre-commit-config.yaml at the root of the repository 
(only the part referencing clang-format at 
https://github.com/OSGeo/gdal/blob/master/.pre-commit-config.yaml#L30 is 
relevant for you):


https://github.com/OSGeo/gdal/blob/master/.pre-commit-config.yaml

Once that file is in place:

-  "pip install pre-commit" : just once

- "pre-commit install": just once per repository

Cf https://gdal.org/development/dev_practices.html#commit-hooks

Even

Le 02/01/2023 à 19:20, Nicklas Larsson via grass-dev a écrit :

Markus,



On 2 Jan 2023, at 13:48, Markus Neteler  wrote:

Hi Nicklas,

On Wed, Dec 21, 2022 at 9:25 PM Nicklas Larsson via grass-dev
 wrote:


I understand there is agreement on using the .clang-format 
formatting rules suggested with [1], which I just merged.


I have formatted the whole source base with clang-format v.15.0.6, 
in 7 different PRs [2-8]. I will start merging them tomorrow if 
there are no objections.


I have also filed a PR [9] which adds a CI check for clang-format 
errors.


Thanks for your efforts on the code reformatting!


:-)




Installing clang-format is perhaps most easily done with:
python -m pip install 'clang-format==15.0.6'

Formatting may be done with something like (following works on Mac):
find -E . -regex '.*\.(cpp|hpp|c|h)' -exec clang-format -i {} \+


... it fails on Linux, though

find: unknown predicate `-E')



I was pretty sure this would deviate from Mac/BSD on Linux systems:


Try something like (untested):

find . -regex '.*\.(cpp|hpp|c|h)' -exec clang-format -i {} \;

or manually with:
clang-format -I 




Contribution rules must be updated, I will start putting up a draft 
ASAP.


I am trying to fix the conflicts in
https://github.com/OSGeo/grass/pull/2684

Conflicting files:

include/grass/iostream/mm.h
lib/db/dbmi_base/dbmscap.c
lib/external/ccmath/ccmath.h
lib/gis/spawn.c
lib/gis/user_config.c
lib/iostream/rtimer.cpp
lib/pngdriver/graph_set.c
lib/rst/interp_float/point2d.c
raster/r.terraflow/filldepr.cpp
raster/r.terraflow/flow.cpp
raster/r.terraflow/main.cpp
raster/r.viewshed/statusstructure.cpp

The reason will be the missing clang-format update which I don't know
how to apply on Linux.


Not quite sure what you mean. Did you try:

python -m pip install 'clang-format==15.0.6’

But I suspect any version 15 will do, perhaps even v. 14.


A tip to use the .clang-format file from main for branches without it:

1. git checkout main
2. cp .clang-forrmat ../.clang-format
3. check out branch
4. clang-format searches upwards in dir hierarchy  for next 
‘.clang-format’ file

5. run clang-format from grass source dir



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


--
http://www.spatialys.com
My software is free, but my time generally not.
___
grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

Re: [GRASS-dev] Proposal for using ClangFormat, replacing GNU indent, for C/C++ code formatting

[Nicklas Larsson via grass-dev]

Markus,


> On 2 Jan 2023, at 13:48, Markus Neteler  wrote:
> 
> Hi Nicklas,
> 
> On Wed, Dec 21, 2022 at 9:25 PM Nicklas Larsson via grass-dev
>  wrote:
>> 
>> I understand there is agreement on using the .clang-format formatting rules 
>> suggested with [1], which I just merged.
>> 
>> I have formatted the whole source base with clang-format v.15.0.6, in 7 
>> different PRs [2-8]. I will start merging them tomorrow if there are no 
>> objections.
>> 
>> I have also filed a PR [9] which adds a CI check for clang-format errors.
> 
> Thanks for your efforts on the code reformatting!

:-)

> 
>> Installing clang-format is perhaps most easily done with:
>> python -m pip install 'clang-format==15.0.6'
>> 
>> Formatting may be done with something like (following works on Mac):
>> find -E . -regex '.*\.(cpp|hpp|c|h)' -exec clang-format -i {} \+
> 
> ... it fails on Linux, though
> 
> find: unknown predicate `-E')


I was pretty sure this would deviate from Mac/BSD on Linux systems:


Try something like (untested):

find . -regex '.*\.(cpp|hpp|c|h)' -exec clang-format -i {} \;

or manually with:
clang-format -I 


> 
>> Contribution rules must be updated, I will start putting up a draft ASAP.
> 
> I am trying to fix the conflicts in
> https://github.com/OSGeo/grass/pull/2684
> 
> Conflicting files:
> 
> include/grass/iostream/mm.h
> lib/db/dbmi_base/dbmscap.c
> lib/external/ccmath/ccmath.h
> lib/gis/spawn.c
> lib/gis/user_config.c
> lib/iostream/rtimer.cpp
> lib/pngdriver/graph_set.c
> lib/rst/interp_float/point2d.c
> raster/r.terraflow/filldepr.cpp
> raster/r.terraflow/flow.cpp
> raster/r.terraflow/main.cpp
> raster/r.viewshed/statusstructure.cpp
> 
> The reason will be the missing clang-format update which I don't know
> how to apply on Linux.

Not quite sure what you mean. Did you try:

python -m pip install 'clang-format==15.0.6’

But I suspect any version 15 will do, perhaps even v. 14.


A tip to use the .clang-format file from main for branches without it:

1. git checkout main
2. cp .clang-forrmat ../.clang-format
3. check out branch
4. clang-format searches upwards in dir hierarchy  for next ‘.clang-format’ file
5. run clang-format from grass source dir


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

Re: [GRASS-dev] Proposal for using ClangFormat, replacing GNU indent, for C/C++ code formatting

[Markus Neteler]

Hi Nicklas,

On Wed, Dec 21, 2022 at 9:25 PM Nicklas Larsson via grass-dev
 wrote:
>
> I understand there is agreement on using the .clang-format formatting rules 
> suggested with [1], which I just merged.
>
> I have formatted the whole source base with clang-format v.15.0.6, in 7 
> different PRs [2-8]. I will start merging them tomorrow if there are no 
> objections.
>
> I have also filed a PR [9] which adds a CI check for clang-format errors.

Thanks for your efforts on the code reformatting!

> Installing clang-format is perhaps most easily done with:
> python -m pip install 'clang-format==15.0.6'
>
> Formatting may be done with something like (following works on Mac):
> find -E . -regex '.*\.(cpp|hpp|c|h)' -exec clang-format -i {} \+

... it fails on Linux, though

find: unknown predicate `-E')

> Contribution rules must be updated, I will start putting up a draft ASAP.

I am trying to fix the conflicts in
https://github.com/OSGeo/grass/pull/2684

Conflicting files:

include/grass/iostream/mm.h
lib/db/dbmi_base/dbmscap.c
lib/external/ccmath/ccmath.h
lib/gis/spawn.c
lib/gis/user_config.c
lib/iostream/rtimer.cpp
lib/pngdriver/graph_set.c
lib/rst/interp_float/point2d.c
raster/r.terraflow/filldepr.cpp
raster/r.terraflow/flow.cpp
raster/r.terraflow/main.cpp
raster/r.viewshed/statusstructure.cpp

The reason will be the missing clang-format update which I don't know
how to apply on Linux.

Markus

PS: If easy on Mac, feel free to directly push an update to the PR :-)
___
grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

Re: [GRASS-dev] Proposal for using ClangFormat, replacing GNU indent, for C/C++ code formatting

[Nicklas Larsson via grass-dev]

I understand there is agreement on using the .clang-format formatting rules 
suggested with [1], which I just merged.

I have formatted the whole source base with clang-format v.15.0.6, in 7 
different PRs [2-8]. I will start merging them tomorrow if there are no 
objections.

I have also filed a PR [9] which adds a CI check for clang-format errors.

Installing clang-format is perhaps most easily done with:
python -m pip install 'clang-format==15.0.6'

Formatting may be done with something like (following works on Mac):
find -E . -regex '.*\.(cpp|hpp|c|h)' -exec clang-format -i {} \+

Contribution rules must be updated, I will start putting up a draft ASAP.
Valuable inspiration may be taken from e.g. GDAL [10] and libtiff [11].

Cheers,
   Nicklas











Add .clang-format:
[1] https://github.com/OSGeo/grass/pull/2272

Format files:
[2] https://github.com/OSGeo/grass/pull/2709
[3] https://github.com/OSGeo/grass/pull/2710
[4] https://github.com/OSGeo/grass/pull/2711
[5] https://github.com/OSGeo/grass/pull/2712
[6] https://github.com/OSGeo/grass/pull/2713
[7] https://github.com/OSGeo/grass/pull/2714
[8] https://github.com/OSGeo/grass/pull/2715

CI check:
[9] https://github.com/OSGeo/grass/pull/2716

[10] https://lists.osgeo.org/pipermail/gdal-dev/2022-December/056658.html
[11] 
https://gitlab.com/libtiff/libtiff/-/merge_requests/431/diffs?commit_id=01cc265a7c7f24de14cda6dc40c960eb8c3c68bb

> On 5 Dec 2022, at 14:06, Nicklas Larsson via grass-dev 
>  wrote:
> 
> Dear All,
> 
> I have put up a PR [1] with the intent to add a “.clang-format” file and 
> ultimately replace the use of “GNU indent” [2] with ClangFormat [3] for 
> formatting GRASS’ source code. ClangFormat, using the Clang compiler parser, 
> is able to format any valid code in both C and C++. (In contrast to numerous 
> problems and limitations with GNU indent).
> I’ve had the impression that this suggestion in general is a welcome one and 
> not particularly controversial.
> 
> I have initially made the '.clang-format' to mirror as close as possible the 
> settings in 'utils/grass_indent.sh' [4].
> 
> However, I'd like to propose changes to the (ClangFormat’s) 
> “BreakBeforeBraces” rules [5]. The "GRASS"-style is a modified version of the 
> GNU style. I think it would be preferable to simplify these rules with the 
> so-called “Stroustrup” style, which is as close as it gets to K (in this 
> regard) and also gives a slightly more compact code vertically. It can be 
> described in short: braces start on new line *only* after functions, and 
> 'else' and 'catch' start on new line after previous closing brace.
> 
> For example:
> 
> int f(void)
> {
>if (...) {
>...
>}
>else {
>...
>}
> }
> 
> All other cases -- enums, structs etc. -- attaches the starting brace, e.g.:
> 
> struct s {
>...
> }
> 
> 
> Please, check out the formatted example files in the PR [6] to see what the 
> proposed changes would look like with the "Stroustrup" 
> BreakBeforeBraces-rules (note: the PR aims to only add the '.clang-format' 
> file).
> 
> I have intentionally left the setting of "ReflowComments" to the default 
> 'true', which cleverly reformats comments extending the 80 columns. This 
> causes some (aesthetic) issues with trailing Doxygen comments in mainly the 
> in the 'include/grass/ and 'lib/' directories, which probably necessitates 
> some initial manual work. On the other hand, a batch format of all module 
> code will likely go pretty smoothly.
> 
> 
> 
> TO THIS INTENT, to finally solve the long extended problem with -- or lack of 
> -- uniformly formatted code and not kicking this stone further down the road, 
> I suggest the following:
> 
> 
> 1. We adapt the formatting policy using ClangFormat.
> 
> 2. We implement "BreakBeforeBraces" rules according the "Stroustrup" style.
> 
> 3. If there are no objections raised within a two weeks period, say until 
> December 18, either to points 1 and/or 2 or even to this proposed deadline, 
> the PR [1] will be merged and work can start on source code formatting.
> 
> 4. Any changes decided upon ought to be added to 
> https://trac.osgeo.org/grass/wiki/Submitting/C
> 
> 
> 
> Cheers,
> Nicklas
> 
> 
> 
> 
> 
> [1] https://github.com/OSGeo/grass/pull/2272
> [2] https://www.gnu.org/software/indent/
> [3] https://clang.llvm.org/docs/ClangFormat.html
> [4] https://github.com/OSGeo/grass/blob/main/utils/grass_indent.sh
> [5] https://clang.llvm.org/docs/ClangFormatStyleOptions.html
> [6] https://github.com/OSGeo/grass/pull/2272/files
> 
> ___
> grass-dev mailing list
> grass-dev@lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/grass-dev

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

Re: [GRASS-dev] Proposal for using ClangFormat, replacing GNU indent, for C/C++ code formatting

[Nicklas Larsson via grass-dev]

> On 7 Dec 2022, at 02:51, Vaclav Petras  wrote:
> 
> 
> 
> On Mon, 5 Dec 2022 at 08:07, Nicklas Larsson via grass-dev 
> mailto:grass-dev@lists.osgeo.org>> wrote:
> 
> 1. We adapt the formatting policy using ClangFormat.
> 
> +1
> 
> I would prefer if formatting changes from GNU indent which can be done 
> separately are done separately.

I don’t quite follow you.


> For Python & Black, I did that together but 1) in multiple PRs and 2) we 
> lacked any formatting, so it was a little different. Things like 
> ReflowComments seem like a good second round. (Sorry, I'm a little confused 
> about your intention with unused .clang-format and what will be in there.) 


My intention is simple. First agree on style and add the .clang-format file to 
source and only after that start the actual formatting work.

I strongly advice to make visual supervision on formatting changes before 
merging. For that reason alone the whole source base should be formatted in 
batches of about 500 files each to be manageable. In total there are ca. 3360 
files. The directories ‘include’ and ‘lib’ may need special attention because 
of the Doxygen comments, all the other will be very easily processed. In all 
this will need some 5-8 PRs.


Directory #files

lib 1199
include  103
db   120
display  109
general   67
imagery  379
misc  13
ps84
raster   774
raster3d 100
temporal   1
utils  2
vector   408
visualization  2

3361

I’d say we should strive to limit the formatting changes of a single file to 
minimum, i.e. to 1 (as in one) time. Therefore any changes like ReflowComments 
should be done in one go. (Also, I’m happy to go through all those file once, 
but not twice.)


> 2. We implement "BreakBeforeBraces" rules according the "Stroustrup" style.
> 
> 
> Related to that, the in-line comments in the PR are modified to have one 
> space after the code while Black, i.e., our Python code, has two.

Well, the two are distinct different languages with different history. Black 
also imposes 88 col line width, not 80. I’d say we keep the ClangFormat 
defaults as long as there is no strong argument against. In the case with 
SpacesBeforeTrailingComments set to 1: do not forget that C-style comments take 
at least 6 characters!  E.g. `/* comment */`. And that is not even a Doxygen 
doc comment.


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

Re: [GRASS-dev] Proposal for using ClangFormat, replacing GNU indent, for C/C++ code formatting

[Vaclav Petras]

On Mon, 5 Dec 2022 at 08:07, Nicklas Larsson via grass-dev <
grass-dev@lists.osgeo.org> wrote:

>
> 1. We adapt the formatting policy using ClangFormat.
>

+1

I would prefer if formatting changes from GNU indent which can be done
separately are done separately. For Python & Black, I did that together but
1) in multiple PRs and 2) we lacked any formatting, so it was a little
different. Things like ReflowComments seem like a good second round.
(Sorry, I'm a little confused about your intention with unused
.clang-format and what will be in there.)


> 2. We implement "BreakBeforeBraces" rules according the "Stroustrup" style.
>

Given the lack of unified standard in C and C++, I'm for making choices
which bring the formatting closer to Python and Black which Stroustrup
seems to do.

Related to that, the in-line comments in the PR are modified to have one
space after the code while Black, i.e., our Python code, has two.


> 3. If there are no objections raised within a two weeks period, say until
> December 18, either to points 1 and/or 2 or even to this proposed deadline,
> the PR [1] will be merged and work can start on source code formatting.
>

My approach with Black was to do config+formatting+CI at once which has the
disadvantage of adding the commit with config+CI into
.git-blame-ignore-revs, but a comment in that file warns about that and it
is exactly what will happen when we change the formatting later on with
active CI (the formatting will have to go in together with config and CI
changes).


> 4. Any changes decided upon ought to be added to
> https://trac.osgeo.org/grass/wiki/Submitting/C
>

Yes. FYI, although details are unclear, the long-term plan for these
documents is to move them to code (based on a discussion at a PSC meeting).

Best,
Vaclav


>
>
> Cheers,
> Nicklas
>
>
>
>
>
> [1] https://github.com/OSGeo/grass/pull/2272
> [2] https://www.gnu.org/software/indent/
> [3] https://clang.llvm.org/docs/ClangFormat.html
> [4] https://github.com/OSGeo/grass/blob/main/utils/grass_indent.sh
> [5] https://clang.llvm.org/docs/ClangFormatStyleOptions.html
> [6] https://github.com/OSGeo/grass/pull/2272/files
>
> ___
> grass-dev mailing list
> grass-dev@lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/grass-dev
>
___
grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

[GRASS-dev] Proposal for using ClangFormat, replacing GNU indent, for C/C++ code formatting

[Nicklas Larsson via grass-dev]

Dear All,

I have put up a PR [1] with the intent to add a “.clang-format” file and 
ultimately replace the use of “GNU indent” [2] with ClangFormat [3] for 
formatting GRASS’ source code. ClangFormat, using the Clang compiler parser, is 
able to format any valid code in both C and C++. (In contrast to numerous 
problems and limitations with GNU indent).
I’ve had the impression that this suggestion in general is a welcome one and 
not particularly controversial.

I have initially made the '.clang-format' to mirror as close as possible the 
settings in 'utils/grass_indent.sh' [4].

However, I'd like to propose changes to the (ClangFormat’s) “BreakBeforeBraces” 
rules [5]. The "GRASS"-style is a modified version of the GNU style. I think it 
would be preferable to simplify these rules with the so-called “Stroustrup” 
style, which is as close as it gets to K (in this regard) and also gives a 
slightly more compact code vertically. It can be described in short: braces 
start on new line *only* after functions, and 'else' and 'catch' start on new 
line after previous closing brace.

For example:

int f(void)
{
if (...) {
...
}
else {
...
}
}

All other cases -- enums, structs etc. -- attaches the starting brace, e.g.:

struct s {
...
}


Please, check out the formatted example files in the PR [6] to see what the 
proposed changes would look like with the "Stroustrup" BreakBeforeBraces-rules 
(note: the PR aims to only add the '.clang-format' file).

I have intentionally left the setting of "ReflowComments" to the default 
'true', which cleverly reformats comments extending the 80 columns. This causes 
some (aesthetic) issues with trailing Doxygen comments in mainly the in the 
'include/grass/ and 'lib/' directories, which probably necessitates some 
initial manual work. On the other hand, a batch format of all module code will 
likely go pretty smoothly.



TO THIS INTENT, to finally solve the long extended problem with -- or lack of 
-- uniformly formatted code and not kicking this stone further down the road, I 
suggest the following:


1. We adapt the formatting policy using ClangFormat.

2. We implement "BreakBeforeBraces" rules according the "Stroustrup" style.

3. If there are no objections raised within a two weeks period, say until 
December 18, either to points 1 and/or 2 or even to this proposed deadline, the 
PR [1] will be merged and work can start on source code formatting.

4. Any changes decided upon ought to be added to 
https://trac.osgeo.org/grass/wiki/Submitting/C



Cheers,
Nicklas





[1] https://github.com/OSGeo/grass/pull/2272
[2] https://www.gnu.org/software/indent/
[3] https://clang.llvm.org/docs/ClangFormat.html
[4] https://github.com/OSGeo/grass/blob/main/utils/grass_indent.sh
[5] https://clang.llvm.org/docs/ClangFormatStyleOptions.html
[6] https://github.com/OSGeo/grass/pull/2272/files

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