update clang-format specs

6 -> 8, see https://github.com/dune-community/Dockerfiles/pull/71

mentioned in merge request !22 (closed)

So I just ran make format in my dune-xt checkout and I got a lot of changes. Very uncool since I only did this prior to committing. The reason being my .clang-format link pointed to the 6er spec still. Not sure how this happened. dunecontrol all had run already. We really need a way to prevent this type of issue going forward.

And even updating the link into .vcsetup and re-running make format I still have lots of changes. What am I doing wrong?

Also, the find script for clang-format still needs an update.

Apparently I was missing !22 (14327f1b) Although I rebased on master. If one looks to https://github.com/dune-community/dune-xt the link is into the 6er spec file. Same here https://zivgitlab.uni-muenster.de/ag-ohlberger/dune-community/dune-xt/-/tree/master

Did you forget to include commits from !22 (closed) into !20 (merged) @falbr_01 ?

René Fritze commented on a discussion: #11 (comment 95084)

Apparently I was missing !22 (14327f1b)

Although I rebased on master. If one looks to https://github.com/dune-community/dune-xt the link is into the 6er spec file. Same here https://zivgitlab.uni-muenster.de/ag-ohlberger/dune-community/dune-xt/-/tree/master

Did you forget to include commits from !22 (closed) into !20 (merged) @falbr_01 ?

I did not include the commits but rather ran make format. Was a bit aurprised bout the little number of changes, so perhaps I suffered the same misconfiguration...

make format would still have formatted python and cmake, so it's really not obvious when clang format isn't run.

I'm making sure this doesn't happen again:

  if(NOT TARGET format)
    if(NOT ClangFormat_FOUND)
      message(WARNING "clang-format not found, not adding format target")
    else()
      add_custom_target(format)
    endif()
  else()
    if(NOT ClangFormat_FOUND)
      message(FATAL "clang-format not found but format target already exists")
    endif()
  endif(NOT TARGET format)

This allows the unlikely use case of not finding clang format, but still producing the other sub targets (py/cmake) if respective tools are found.

Also Also, make format does not error out if cmake did not find the right clang format.

Seems like a great situation in total...

Do you think we should take care to not format the pybind11 files, to make updating pybind11 easier, @r_milk01?

Not really. I very recently updated pybind and I see no benefit int that.

k

I just ran clang-format-10 under arch and the changes I got look sensible to me, e.g.

+Subproject commit 75e46272125c983b7f02af5b3e7b60749a6fac3b
diff --git a/dune/gdt/operators/lincomb.hh b/dune/gdt/operators/lincomb.hh
index c5d93f1b..75c1eca8 100644
--- a/dune/gdt/operators/lincomb.hh
+++ b/dune/gdt/operators/lincomb.hh
@@ -514,7 +514,7 @@ public:

using BaseType::operator*;

-  LincombOperatorType operator*(const FieldType& alpha)override final
+  LincombOperatorType operator*(const FieldType& alpha) override final
{
LincombOperatorType ret(*this); // logging is inherited by copy ctor
ret *= alpha;

Did any of you notice crappy formatting under clang-format-8?

Yeah, there seemed to be a few errors in the formatting. But we probably have to live with it, or can we use clang-format-10?

Clang-format-10 will only be available from the next debian release. It'd be fine with a simple to install backport package or sth though, but a compile from source is def not worth this imo.

Agreed. Though this it what I am doing with clang8 for arch now. Having the wrong clang-format is cumbersome ...

With the incoming pre-commit setup this becomes much easier. clang-format is sourced from binary wheels. Here's what happens (and how-to) switch to spec version 13: 47b2984b

Very nice! This will also make future updates much easier since we only have to change a single line in the config and do not have to wait till the respective version is available in all our CI environments.

mentioned in merge request !92 (merged)

Fixed in !92 (merged).

closed