Modernize clang-format file

[HomesGH]

Description

As discussed in issue #276 , we want to add a clang-format file to enforce an unified style in ls1.
For this, we have to:

• Agree on the content of .clang-format file
• Agree on clang-format version (and add it to CI file)
• Add CI to check format (DISARMED FOR NOW)
• Add target to CMake to just run command make clang-format for formatting (this is not working for now, help wanted!)
• Add target to Makefile to just run command make clang-format for formatting

If we want to be very fancy, we could do something like in MegaMol.

Resolved Issues

• cpplint / clang-format / clang-tidy #276 (in part)
• fixes enforce formatting through jenkins #29

I would recommend that this PR does not apply the formatting to the whole code base in ls1. This should be done in a separate PR for the sake of clarity.

[amartyads]

I like that the clangformat file only has the essentials for now, I think it makes it easier for discussion purposes. In the final version, however, we should change it back into a full file.

I made the BreakBeforeBraces style Attach, now we will have } else { again instead of else on a newline.

One (potentially) controversial change I've made is PointerAlignment: Left. We prefer Right (I do too), but the PDF style guide says Left, and I think that if we're following the PDF on spaces vs tabs, we should follow it here as well.

• Change .clang-format file back to a full config file, from a cropped one

The CI seems to be using clangformat 14. I would vastly prefer 18, because that's what MaMiCo uses, but we can discuss that further.

We could also use git hooks to format all new files according to the clangformat, however AFAIK git hooks are not server side.

[github-actions]

⚠️ The CI detected formatting issues in this pull request. Please run clang-format locally to resolve them.
Details are shown in the job log.

[HomesGH]

We agreed on the style and content of the .clang-format file during our meeting.

[HomesGH]

@cniethammer @FG-TUM @amartyads
Please have a look on how to integrate clang-format into CMake. This was first mentioned here. I am not very experienced with CMake and think that my attempt is not working properly...

[cniethammer]

I just encountered a non-trivial problem with clang-format, which was not mentioned so far:

We have the Doxygen documentation including \code blocks for the various readxml functions that show
the XML structure of the expected input and include also some XML comments. We should prevent these
comments from getting modified as this will end up in the auto-generated API documentation in a very messed up
way otherwise. I expect also other documentation parts for which have to take care of.
We will need to do a lot of manual intervention ...!

The doxygen style is using /** ... */ blocks. To exclude code from clang-format one has to put in clang-format markers. These however expect to be of the form // clang-format ... or /* clang-format ... */. So they do not match the Doxygen comment format and we cannot throw them directly into the Doxygen blocks. // comments inside the /** */ blocks are not possible.
So the only solution is to close and reopen the doxygen blocks. Luckily Doxygen allows this and, e.g., the class descriptions will stay attached to their class. The following two styles work as far as my experiments go:

Option 1:

/** @brief ...
*
*  The following xml object structure is handled by this method:
*/
// clang-format off
/**
*  \code{.xml}
* 	<outputplugin ...>
...
* 	</outputplugin>
*  \endcode
*/
// clang-format on
virtual void readXML(XMLfileUnits &xmlconfig);

Option 2:

/// @brief Read in XML configuration for CsvWriter and all its included objects.
///
/// The following xml object structure is handled by this method:
// clang-format off
/// \code{.xml}
///	<outputplugin ...>
...
///	</outputplugin>
/// \endcode
// clang-format on
virtual void readXML(XMLfileUnits &xmlconfig);

Both options require us to go over all the comments. Option 1 is somehow consistent with the current Doxygen block style - but looks IMHO disturbing. Option 2 looks nice but will require us to change the Doxygen block style.

I am in favour of Option 2.

Note: While I am not fully opposing the idea of a consistent coding style - I am not convinced, that reformating and making this mandatory are worth the effort ... especially since I am starting to look more into this.

Sidenote: GitHub code review suggestions will by the way also be affected ... nothing a cmake formater tool will help you with, especially when you use the smartphone app.

[cniethammer]

Found another special case: Copyright comments at the beginning of files - we should not mess up institutions and copyright years.

[FG-TUM]

I'm almost sure we can resolve both of the special cases you mention with CommentPragmas.

[HomesGH]

It could be even uglier. Sometimes, the doxygen codes have no leading *, see here.

We could:

1. use clang-format on/off (maybe use script to include before \code{.xml} and after \endcode
2. exclude all doxygen comments, e.g. with CommentPragmas: '^\\s*\\*|\\/\\*\\*|\\/\\/\\!'
3. exclude all comments with CommentPragmas: '^.*$'

Explanation of point 2:
^\\s*\\* matches lines starting with an asterisk *
\\/\\*\\* matches block comments starting with /**
\\/\\/\\! matches Doxygen inline comments starting with //!

[FG-TUM]

What are we actually trying to solve here? Afaik clang-format just enforces a line limit on comments and leaves indent as is. Yes introducing line breaks in xml code can be confusing in documentation but that's what a PR review is for.

In my opinion examples like the one liked by @HomesGH are bad and should be formatted anyway instead of preserving.

@cniethammer you mentioned code that ends up "in a very messed up way". Please list them (some?) which are unchangeable so we can discuss the actually problematic cases.

[cniethammer]

I did not say it is impossible (brought also up a solution I found). I am just trying to look for potential issues and what they will require us to invest in terms of effort for our upcoming one-time update and future maintenance of the code after enforcing this - as well as the setup that will be built around, i.e., the clang-format style as well as related CI, will require work in the future and some maintainer, too. I would not consider myself enough of a specialist in clang-format to do this.

I just had a quick look into the io plugins - RDF.h and ADIOSWriter.h are for example such cases. But it's a fair point, that refactoring the comments could prevent some of the trouble. I guess this needs some closer look and experimenting. But nothing to be done in a day - at least for me...

[amartyads]

So I took the two files MaxCheck.h and RDF.h and it is undeniably true that long xml tags in the docs are getting badly mangled. I'm attaching the result of applying the current clangformat file on both here, as *-342.h since this is PR 342.

RDF-342.txt
MaxCheck-342.txt

The code looks a lot better though.

My proposal is, instead of scary regex magic spells, we simply turn ReflowComments to false (Never in clangformat-20, false in older versions). Then clangformat never touches any comments.

RDF-noRefl.txt
MaxCheck-noRefl.txt

My reasoning is, since we're adding formatting to make code readable, it shouldn't be necessary for comments, since comments are already readable (ideally).

I couldn't test for any files where clangformat messes up the copyright info. I can also try to work on a more focused regex that excludes doxygen code blocks and copyright info, if someone could give me an example for the latter.

One last thing I tried was to replace \code \endcode with @code @endcode but that didn't help.

So in conclusion, we can add ReflowComments: false to circumvent the problem.

[FG-TUM]

Two observations from the files you show:

1. Wide comments: IMHO we should address the formatting of those comments. E.g. MaxCheck.h:78 is 201 characters wide. That is very inconvenient to read but also rare so I think we can catch those in a review.
2. Code in Doc: The example you show in RDF.h:50-55 is actually an argument why we NEED formatting in comments: These lines are formatted with a mix of tabs and spaces that only looks correct with a tabsize of 4. clang-format can rectify this.

I'm not really invested in this anymore, so I don't care to how you handle this in the end. My advice would be to bite the bullet and refactor/reformat the whole code base properly including comments, documentation, and licenses. Sure this is some work but in the long term this brings consistency and thus readability. But if none of you want to do that I guess ReflowComments: false and accepting some chaos is the next best thing.