Add a CI job that runs uncrustify and checks for modifications

This is the setup I use for the dune-gfe module. So far I am quite happy with it.

If we ever put more things into the Dune::Fufem that will lead to further large reindentation patches. To avoid this one may decide to add the namespaces now, i.e., before the initial uncrustify reformatting. Or one may decide to not care about this.

There's some more cleanup that we should do before. It looks like this adds one level of indent per scope. Hence any later change of

namespace Dune {
namespace Fufem {
}
}

to

namespace Dune::Fufem {
}

(which I strongly prefer) will imply plenty of white space changes.

I don't like the indentation of lambda functions. It seems that this currently uses a base indentation corresponding to the offset of the capture block [...]. This has a few nasty implications:

IMO indentation should reflect the structure of the code. Here it depends on names. If we rename a lambda object, the whole block needs to be reindented.
If we use a long, meaning full name for the lambda, this increases the length of lines significantly.
This also leads to odd indentations which does not align well with how editors handle indentation.

I generally agree with this. One possibility is to start playing with the uncrustify options file. uncrustify has lots of options, and chance are high that there are a few for lambda formatting. The dilemme is that (at least IMO) there is also value in sticking to the "official" options file, i.e., the one from dune-project.org.

But these are not just visual problems and a matter of taste. Name dependence of indentation may have a severe impact on the readability of diffs. And odd indent will lead to more work because you will never get this correct in the first try.

Then we maybe need to change the "official" style.

Using

indent_cpp_lambda_only_once=true

seems to be the option to disable the strange additional indent.

Another potential problem: To avoid playing trial-and-error against the CI one must be able to generate the correct format locally. But for me uncrustify complains about the unrecognized option align_number_left. Thus I suspect that calling uncrustify locally and in the CI may give different results (similar to the code-spell-checking job).

Changing the official style would be the best course of action, IMO.

I do get the warnings about align_number_left (locally) as well, but I have not seen a situation yet where the local output of uncrustify differs from the CI one. Therefore I have not looked at the problem in any detail.

If it has no effect, removing it should not harm.

Apparently the option has been renamed A Long Time Ago: https://github.com/uncrustify/uncrustify/pull/1393

I have suggested to add indent_cpp_lambda_only_once in infrastructure/dune-website!534.

added 1 commit

4690a2f1 - Improve indentation of lambda definitions

Compare with previous version

Add a CI job that runs uncrustify and checks for modifications

Merge request reports

Activity