Unnecessary changes in code base because of assumption that clang-format defines project coding style

[jfischer-no]

There is increased unnecessary changes in codebase because of assumption that clang-format is defines project coding style.

The formatting of clang-format is not completely compatible with our established coding style and requires some manual post-processing. For some reason new contributors start changing existing code that follows Zephyr's coding style, like for example in https://github.com/zephyrproject-rtos/zephyr/pull/48586/commits/6d2716d9405a048f55d365b32fb6c3f61e0f474c The worst are the changes caused by clang-format to function/macro call and function definition, which are caused by the change of the ColumnLimit from 80 to 100. There seems to be no option to turn it off other than by changing back to ColumnLimit: 80, where necessary anyone can re-edit during post-processing, that in sum will cause much less noise in the codebase.

There are other minor issues like changing the alignment of blocks of #defines and initializers. Kernel documentation is more detailed, this could also be improved in the project's clang-format documentation.

[gmarull]

There's the option to remove/unset ColumnLimit, then clang-format will not try to fit content up to any column width. However, this means we need to enforce the maximum somehow (we already do with checkpatch).

[henrikbrixandersen]

I reintroduced a ColumnLimit of 100 after it was reduced to 80 since clang-format otherwise reformatted code lines with length > 80 that otherwise conformed to our established coding style.

[gmarull]

I think that we should settle on this one. Either set a maximum to 80, 100 or something in between. We inherited the 100 cols exception from Linux Kernel, but IMO it's a bad rule because there's no way a tool can decide when 80 or 100 is the right choice. We should define a coding style that can be enforced/automated for basic things like this.

[henrikbrixandersen]

I think that we should settle on this one. Either set a maximum to 80, 100 or something in between. We inherited the 100 cols exception from Linux Kernel, but IMO it's a bad rule because there's no way a tool can decide when 80 or 100 is the right choice. We should define a coding style that can be enforced/automated for basic things like this.

The original decision to move from 80 to 100 happened in https://github.com/zephyrproject-rtos/zephyr/issues/30426

[jfischer-no]

I reintroduced a ColumnLimit of 100 after it was reduced to 80 since clang-format otherwise reformatted code lines with length > 80 that otherwise conformed to our established coding style.

As said one can not use this tool without post-processing anyway, ColumnLimit: 80 generates a code that better matches our coding style, where necessary one can/need post-edit.

I think that we should settle on this one. Either set a maximum to 80, 100 or something in between. We inherited the 100 cols exception from Linux Kernel, but IMO it's a bad rule because there's no way a tool can decide when 80 or 100 is the right choice. We should define a coding style that can be enforced/automated for basic things like this.

.clang-format and clang-format tool does not and IMO should not define project's coding style, but since it seems that new users see it first, we should choose there rather conservative setting, as for example Linux Kernel does.

[gmarull]

The problem with the 100 line rule is that the decision is left to the developer. checkpatch will just complain if you exceed 100 cols, but it's up to you where to break (subjective). As I said earlier, you can disable column limit in clang-format, then, every developer can break when they think it is necessary. checkpatch will then complain when 100 cols are exceeded.

I'd personally fix a column limit and stay with it, there'll be always a tiny portion of the code that won't look as I'd like. I've been using black for Python code (sets column limit to 88 cols, an interesting value) and I've never questioned anymore if 90 cols are better for one particular line.

[Laczen]

My personal preference would also be to fix it to a lower value of about 80. To allow easy tabbing to add a continuation marker at the end of a line it would be good to fix it to 81.

[broglep-work]

I think the first and foremost fix should be to make it clear in the documentation that clang-format does not produce code that adheres to the code style and there is no intention to change that for the time being. And that it is the responsibly of the developer to make sure the code style matches.

The documentation currently states (emphasis mine):

The clang-format tool can be helpful to quickly reformat large amounts of new source code to our Coding Style standards

which understandably lets developer assume that clang-format defines the project style.

[jfischer-no]

See https://github.com/zephyrproject-rtos/zephyr/issues/52712#issuecomment-1516541794

[broglep-work]

So no clarification in the documentation?

I think the history of this issue and all the cross-references to other issues is proof enough the current stance on this matter is far from clear to every developer working with the code base. A note about https://github.com/zephyrproject-rtos/zephyr/issues/52712#issuecomment-1516541794 would not hurt.

I would get sick of having the same discussion reappearing from the to time, but that is not my battle ;)

[jfischer-no]

https://docs.zephyrproject.org/latest/contribute/guidelines.html#coding-style

[broglep-work]

https://github.com/zephyrproject-rtos/zephyr/issues/48785#issuecomment-1342880392