On 12/9/23 20:29, Marc Mutz via Development wrote:
Hi,TL;DR: - remove _clang-format in qt5.git - add it instead to submodules which conform to it The clang-format philosophy is that you pick a config and stick to it. If your personal preferences are different, you use a different configuration locally and re-format on check-in and check-out. This means that the source code in the VCS must always conform to the clang-format configuration in effect in that branch. It also means that when and if you make changes to the clang-format configuration, you need to re-run the tool over the whole code base and apply the changed configuration, otherwise the clang-format workflow doesn't work (produces unrelated white-space changes). We have for quite some time had a _clang-format config file in qt5.git. After many attempts by many people, it has become clear that a) it has completely wrong settings (e.g. template<> vs. template <>),
This particular issue with template<> has been "fixed" recently thanks to Mårten https://codereview.qt-project.org/c/qt/qt5/+/433720.
leading to a complete formatting mess in modules that predate the addition of the file and more discussions around formatting than before, because there appear to be two true styles, and b) we can't seem to hammer clang-format into reproducing what we traditionally understand as the Qt style. In addition, the position in qt5.git means that if you edit files in submodules of qt5.git, the config is in effect, if you just checkout qtbase, it is not.
_clang-format isn't picked up by clang-format by default, you'd have to rename it to .clang-format.
I propose leaving that _clang-format file as the default clang-format config file, but each repo should have a .clang-format committed in git where the module maintainers/powers-to-be can put the code style that they want in their module. Some repos in Qt already do that. One of them has the max line length set to way more than 100, with a comment (paraphrased):I would therefore propose to remove the file from qt5.git: https://codereview.qt-project.org/c/qt/qt5/+/503430
"most lines in this repo are long, we have to live with with, move on"which is perfectly fine, the tool will pick up the config file and act accordingly. You can even have a .clang-format per dir in each module.
So, remove it if you want, but a precursor to that is copying it to each Qt module, new _and_ old. It's OK for each module to change its own copy to fit its own style, but there should be a .clang-format, because it's much easier to let it format a diff, e.g. `git clang-format`, and override the few places that the tool got "wrong". This is more important for new devs who aren't familiar with the current coding style (yes, there is a good wiki page about the Qt coding style, but no it's not enough, there so much code and too many quirks to cover in one wiki page).
Modules that don't have a .clang-format in git are implying "use the default config file". With a .clang-format committed to git, you get it any which way you checkout a Qt repo, `init-reporistory` or `git clone`.
A config file that is 80-90% correct is better than nothing. For example I have copied that file to qtbase/.clang-format and changed that `template <>` part, it's still useful for me to have the file there as I could select some text and format only that part in e.g. KDE's Kate. I can override the changes to conform to qtbase's coding style, but I don't have to do it all manually.
Drive-by suggestion: each module should also have a .git-blame-ignore-revs file (name should be unified in all Qt repos), to put commit hashes that git-blame should ignore. (See 'git blame --ignore-revs-file').
Regards, Ahmad Samir
OpenPGP_signature
Description: OpenPGP digital signature
-- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
