[Development] Proposal: (re)move qt5.git/_clang-format

[Ahmad Samir]

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 would therefore propose to remove the file from qt5.git:
> https://codereview.qt-project.org/c/qt/qt5/+/503430
> 
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):
"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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <http://lists.qt-project.org/pipermail/development/attachments/20230912/742ba087/attachment.sig>