Add uncrustify script
This is an alternative to !3 (closed)
Merge request reports
Activity
added 1 commit
- 6bf01569 - Fix a type constraint definition in intersection assembler archetype
added 1 commit
- 3b72a47f - Add a cmake command to run uncrustify on the c++ files
mentioned in merge request !3 (closed)
added 1 commit
- 66b3fdf0 - Add --check option to uncrustify in gitlabci
added 1 commit
- bc442857 - Modify the style of the spaces before function braces
added 1 commit
- 1469f744 - Modify the style of the spaces before function braces
mentioned in commit 94e059b6
113 117 Concepts::BindableToElement<LA, typename TestLocalView::Element> && 114 118 Concepts::BindableToLocalViews<LA, TestLocalView, TrialLocalView> && 115 119 Concepts::BindableToOutsideLocalViews<LA, TestLocalView, TrialLocalView> && 116 requires(LA la, typename TestLocalView::GridView::Intersection is, Archetypes::Matrix (&localMatrix)[2][2], Archetypes::LocalPattern (&localPattern)[2][2]) 120 requires(LA la, typename TestLocalView::GridView::Intersection is, Archetypes::Matrix localMatrix[2][2], Archetypes::LocalPattern localPattern[2][2]) See !11 (comment 137215) for an example where different
uncrustifyversions lead to a problem. The incompatibility revealed there:Foo (...) : Foo (...) // Mind the space before (...) introduced by new uncrustify {}which is removed by newer versions
Foo (...) : Foo(...) // No extra space here in old uncrustify {}Edited by Carsten Gräser
Oh, this is problematic. Even if we would remove the CI check, still if someone touches the file with a different uncrustify version as it was saved before, it would change the formatting. If uncrustify is included in the editor, it is hard to enforce a violation of the rules.
What we could do is to enforce a specific uncrustify version. That must be the same as in the gitlab ci. Then there would be no change.
Or we remove the CI check and just document the formatting rules. But this will probably lead to inconsistent formatting soon.
-
As written in the other MR (or in Matrix), in each uncrustify version the formatting (and the options database) changes. In the example above, I would say we want the "newer version", because function calls should not have a space between function name and brackets, but function declaration should. (By the current configuration)
Edited by Simon Praetorius -
I will try to adapt the options file. I have just removed/renamed any unknown option in the version-specific cfg files. Maybe there is a new option added that works similar to the old version. Also, there is an
--update-configflat in uncrustify. Maybe the brings an existing cfg file to the new version? I have to try this out We may update the container used by the CI, but then people will have the same problem when they use an older version locally. The simplest solution is to use a version compatible to what most people use. Currently the CI uses Debian 11 (oldstable). I just tried Debian 12 (stable). This gives the same result.
It would be nice if all of you try
make fix-format. If it does not modify anything (in particular if it does not add the space before(in: Assembler(...)) for all of you, we can stick with the current version. Personally I now know how to easily switch the used version and am happy to use whatever works for the majority.
> uncrustify --version Uncrustify-0.78.1_fdiff --git a/dune/assembler/defaultglobalassembler.hh b/dune/assembler/defaultglobalassembler.hh index 8ba28f1..47cf739 100644 --- a/dune/assembler/defaultglobalassembler.hh +++ b/dune/assembler/defaultglobalassembler.hh @@ -48,7 +48,7 @@ public: {} Assembler (const RowBasis& rowBasis) requires std::same_as<RowBasis, ColBasis> - : Assembler(rowBasis, rowBasis) + : Assembler (rowBasis, rowBasis) {} // ....I don't think this incompatibility issue will be as big a problem in practice as it may sound here. Possibly a problem, yes, but not a big one.
IMO the uncrustify version used by CI is the official one that defines the "correct" formatting. If people use a different version locally then they may have to fix formatting differences manually. The
git diffrun by CI tells them exactly what needs fixing. In a given MR, formatting differences can only occur in files touched by that MR (i.e., few) [0][0] unless, of course, if you run
make fix-formattingon the entire repo. That you cannot do, but in my experience that's an inconvenience but not a serious problem.IMO the uncrustify version used by CI is the official one that defines the "correct" formatting. If people use a different version locally then they may have to fix formatting differences manually.
I agree, that we should declare this the official version. However, adjusting the code manually seems to be tedious compared to
make fix-format, but for me using an older version ofuncrustifyis fine.@christi With this version you will also run into trouble. Would either manual reformatting or using
uncrustifyvia a container be an option for you?
-
In order to make sure that everyone uses the correct uncrustify version, we could
- make a cmake version check.
- only use the
uncrustify-0.72.cfgconfig file and not the one without a version number. - Add a comment in the top of the file that this cfg files is for uncrustify version 0.72 only.
