formatting depends on clang-format version (and the one I used created ugly things)
KrisThielemans opened this issue · 4 comments
I ran clang-format on Ubuntu 22.04. This turns out to be version 14.0.0-1ubuntu1.1. This is also the version run by the pre-commit-check action,
so it's happy on
master.
Problem
However, I now ran it with latest clang-format from conda-forge, which is version 17.0.6. This gives changes in probably 40 files. This is probably a well-known problem, see e.g. https://stackoverflow.com/questions/59395507/how-to-maintain-a-clang-format-file-for-different-clang-format-versions and the post it refers to. It degrades my opinion on clang-format dramatically.
Probably the only solution to this is to enforce a clang-format version, which can be done via https://pre-commit.ci. I rather dislike this solution: people set-up their editor carefully, they commit some code and push, pre-commit.ci reformats it. They have to pull (presumably). Now their editor reformats it back again. etc. etc. (I guess this would lead the our number of commits increasing with a factor 2, and git fame reporting wrong results).
Another solution is to use a more stable tool than clang-format, and therefore likely change 1000+ files again.
@casperdcl @dvolgyes @markus-jehl any suggestions?
More detail
I'll create a PR with the 17.0.0 version such that the changes can be expected, but I've picked 3 illustrating 3 different categories:- trivial, presumably could be configured to stick to the 14 version.
- weird: the 14 version made more sense. This category is worrying as presumably this is due to bugs in 17.0.0, which will be
- beneficial: clean-up a total mess introduced by 14.0.0
trivial
diff --git a/src/utilities/construct_randoms_from_singles.cxx b/src/utilities/construct_randoms_from_singles.cxx
index 86c234429..d76ae5ddb 100644
--- a/src/utilities/construct_randoms_from_singles.cxx
+++ b/src/utilities/construct_randoms_from_singles.cxx
@@ -27,7 +27,7 @@
#include <iostream>
#include <fstream>
#include <string>
-//#include <algorithm>
+// #include <algorithm>
weird
diff --git a/src/recon_buildblock/distributable.cxx b/src/recon_buildblock/distributable.cxx
index d422a0723..00db9c610 100644
--- a/src/recon_buildblock/distributable.cxx
+++ b/src/recon_buildblock/distributable.cxx
@@ -627,12 +627,12 @@ LM_distributable_computation(const shared_ptr<ProjMatrixByBin> PM_sptr,
std::vector<float> measured_div_fwd;
#ifdef STIR_OPENMP
# pragma omp parallel shared(local_output_image_sptrs, \
- local_row, \
- local_log_likelihoods, \
- local_counts, \
- local_count2s, \
- local_measured_bin, \
- local_fwd_bin)
+ local_row, \
+ local_log_likelihoods, \
+ local_counts, \
+ local_count2s, \
+ local_measured_bin, \
+ local_fwd_bin)
#endif
beneficial
the following code snippet and the lines below it
STIR/src/test/numerics/test_matrices.cxx
Lines 160 to 166 in ee307c7
some other posts
- https://egli.dev/posts/update-alternatives-for-clang-format/ says how to select a version in ubuntu
- https://youtrack.jetbrains.com/issue/CPP-15236/Support-custom-clang-format-binary says that CLion bundles its own version, so it cannot be forced to a fixed version.
For now, all PRs should be run with clang-format 14.0.0.
https://youtrack.jetbrains.com/issue/CPP-15236/Support-custom-clang-format-binary says that CLion bundles its own version, so it cannot be forced to a fixed version.
This might not be true anymore,
and from CLI
$ which clang-tidy
/usr/bin/clang-tidy
$ clang-tidy --version
Ubuntu LLVM version 14.0.0
Optimized build.
Default target: x86_64-pc-linux-gnu
Host CPU: alderlake
that could help, except of course we're after clang-format (or does the former include the latter?)