nextsimhub / nextsimdg

neXtSIM_DG : next generation sea-ice model with DG
https://nextsim-dg.readthedocs.io/en/latest/?badge=latest
Apache License 2.0
10 stars 13 forks source link

clang format - is this expected behaviour #616

Closed TomMelt closed 4 months ago

TomMelt commented 4 months ago

@timspainNERSC, quick question. I am in the process of rebasing #556 and I realised some formatting discrepancies between two clang-formatted versions of dynamics/src/include/Tools.hpp. The following line feels like something that should be tidied up by the formatter but it isn't.

https://github.com/nextsimhub/nextsimdg/blob/0fb6d41c98695e0d6709719d64b6fa7578b3b592/dynamics/src/include/Tools.hpp#L101

Is this deliberate / expected behaviour. I thought it would at least split it over multiple lines.

When I run clang-format -i dynamics/src/include/Tools.hpp this line does not change. Equally, if I manually split it to something like the following, it will remain untouched by the formatter:

                SHEAR.row(i) = ParametricTools::massMatrix<S2A(DGs)>(smesh, i).inverse()
                    * (PSI<S2A(DGs), NGP>
                        * (((e11_gauss.array() - e22_gauss.array()).square()
                               + 4.0 * e12_gauss.array().square() + 1.e-20)
                                .sqrt()
                                .log10()
                            * ParametricTools::J<NGP>(smesh, i).array() * GAUSSWEIGHTS<NGP>.array())
                              .matrix()
                              .transpose());
timspainNERSC commented 4 months ago

This is Thomas' code. As far as I am concerned 'here be Dragons' and anything that passes CI, goes.

TomMelt commented 4 months ago

That's ok. I understand you didn't write it. Just curious if we want clang-formatter to be stricter. I would have thought that it would split really long lines into something more readable. But it's not a problem :ok_hand: