Code formatting ignores line length in documentation, comments and code

[BenjaminRodenberg]

Currently our code formatter seems to ignore the line length in documentation / comments. This makes comments hard to read in github, when reviewing (see https://github.com/precice/precice/pull/1029#discussion_r772205012). Our pipeline that checks for correct formatting, for example, does not complain about this line:

https://github.com/precice/precice/blob/9b3f6b7fa7e78122931db98cf34abb76ac656206/src/time/Waveform.hpp#L46-L49

[davidscn]

This is actually not specific to commented code, but to all our code (line-breaks in error messages were mostly manually added). The reason is the following line in our formatting settings

https://github.com/precice/precice/blob/6da260d3385d1bb81a9ffbf37f4e04c86510da2f/.clang-format#L47

I would (again) strongly support that we add a limit, since the current setting also allows some ambiguity (e.g. adding line-breaks in function declarations or else-where).

[fsimonis]

I am in favour of automating as much as possible in regards to styling.

We need to agree on a version of clang-format to keep formatting consistent. Then we can agree on a configuration and apply it to the code base.

[davidscn]

AFAIK we agreed on clang-format-8 in our code-base? We could increase the version, of course.

[fsimonis]

Ubuntu 20.04 LTS provides clang-format 10.0 12 this is also available for Alpine 3.12 3.15 (CI image)

So, we could increase the version to clang-format 10 12 without issues.

[davidscn]

For the sake of completeness: on ubuntu 20.04 you can get all clang-format version between 6 and 12. Upgrading is a good idea, though.

The more interesting question is probably the actual ColumnLimit. I would suggest a small limit of around 100, because it would allow a window splitting and reading complete code side-by-side.

[MakisH]

FYI, on GitHub on the desktop, the limit seems to be 160 characters. So, anything below that should be fine. I agree with the tighter limit for the same reasons.

[fsimonis]

Decision: upgrade to version 12 and try out various line lengths (100, 120, 160) When: directly before the release to prevent conflicts

[davidscn]

Here a summary (note that there are already quite a lot of changes removing the current ambiguity, i.e., the code does not exceed the column limit)

• limit: 160: 329 files changed, 4127 insertions(+), 8930 deletions(-)
• limit 120: 348 files changed, 5757 insertions(+), 9192 deletions(-)
• limit 100: 394 files changed, 8814 insertions(+), 10140 deletions(-)

[BenjaminRodenberg]

In the python-bindings we use a limit of 120 columns (see https://github.com/precice/python-bindings/blob/8eee60710b786f9ae0bd6aeb5abc02b90c9f58e9/.github/workflows/check-pep8.yml#L18). According to PEP8 (python formatting rules) 80 columns are suggested, but since this comes from the time of small 4:3 screens, it is broadly accepted to not follow this rule.

So far, we did not make bad experienced in the python bindings with 120 columns. 100 might be a bit too few, if you have long function names at unfortunate spots before linebreaks.

Btw: Do we have a limit on actual code? Because the title of this issue only says "documentation / comments". I think it would make sense to make both limits consistent. I imagine it looks odd to cut code at, say, 80 columns and allow 160 columns for documentation.

[davidscn]

IIRC, @fsimonis also preferred 120. Then I will go for this option.

Btw: Do we have a limit on actual code? Because the title of this issue only says "documentation / comments". I think it would make sense to make both limits consistent. I imagine it looks odd to cut code at, say, 80 columns and allow 160 columns for documentation.

Here, the column limit refers to both, code and comments (code being even my higher priority).

[BenjaminRodenberg]

Btw: Do we have a limit on actual code? Because the title of this issue only says "documentation / comments". I think it would make sense to make both limits consistent. I imagine it looks odd to cut code at, say, 80 columns and allow 160 columns for documentation.

Here, the column limit refers to both, code and comments (code being even my higher priority).

Sure. Just wanted to make sure we don't forget it. I will quickly update the title of the issue accordingly.

[fsimonis]

The PR for the CI image is ready. Once it is merged, the CI will break https://github.com/precice/ci-images/pull/3

[fsimonis]

IIRC, @fsimonis also preferred 120. Then I will go for this option.

I merely read that 120 chars is a common choice for a limit. Exception are big projects like the linux kernel (80), llvm (80) and the google style guide (100).

I honestly don't mind as long as we pick something and stick to it.