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

[Ahmad Samir]

On 13/9/23 01:46, Marc Mutz via Development wrote:
> [I didn't get André's Email...]
> 
> On 12.09.23 23:40, Ahmad Samir wrote:
>> On 13/9/23 00:11, apoenitz wrote:
>>> On Tue, Sep 12, 2023 at 11:33:17PM +0300, Ahmad Samir wrote:
>>>> A config file that is 80-90% correct is better than nothing.
>>>
>>> I disagree.
>>>
>>> The result of such a thing is that people submit patches matching the
>>> config
>>> 100%, deviating from the wanted style by 10-20%. Numbers are arbitrary
>>> here, but the effect is that in practice even long-term contributors
>>> start
>>> to submit "wrong" auto-formatted patches causing needless review
>>> roundtrips.
>>>
>>
>> The alternative is patches with arbitrary formatting; that's not better.
> 
> Sorry, no. If you submit a patch, you should just use whatever style you
> find in the file. If there's a mix, use whatever is adjacent to where
> you're coding. It's that simple. That shouldn't be too much to ask, and
> I, for one, am fine going into the Gerrit editor and fixing things for
> the submitter if that's the only thing holding me back from a +2. It's
> when people insist on rewriting lines because that's how QtCreator or
> git-clang-format format it, that the discussions start.
>  > And a clang-format file doesn't actually help here, because we have
> already introduced inconsistencies in qtbase and whatever we select in
> .clang-format will be wrong 50% of the time. Assuming, of course, that
> you can get it configured to be close enough to the prevailing style,
> which I have not seen proof of.
> 
> If anyone thinks it's possible, there's a simple litmus test: apply your
> config to all of qt(base) and look at the diff: Is it just fixing some
> outliers, or is it rewriting the world?
> 

Without testing, I am sure it's the latter. :)

>>> On top of that repeatedly discussion are started about adjusting the
>>> style to
>>> the tool, because "the tool can't be adjusted".
>>
>> True, but style arguments arise on their own too, tool or no tool.
> 
> It only arises because the tool cannot represent what we traditionally
> call the Qt style. If there are disagreements on what that means, it's
> easy to draw upon a majority vote of Qt 4 code to break the tie,
> disregarding clang-format-infected files.
> 
> I should also point out that the discussions a few years ago were
> nitpicking about Qt coding style violations, and the expected response
> from the submitter was to fix them. These days, everything is called
> into question because clang-format does it differently, ie. wrongly. And
> I'm kinda getting tired commenting on all these clang-format
> idiosyncrasies. If the tool was truly configurable, e.g. with an example
[...]

(Are most of those idiosyncrasies about 'template <>' in qtbase reviews? if so, 
then that's because qtbase doesn't have its own .clang-format with that style, 
because that's actually something the tool can do).

I _know_ there is no perfect solution, I've spent some time trying to tweak the 
clang-format config in Qt and other projects/repos, there is no way to get a dumb 
formatting tool to do exactly what you want in all cases, partly because it can't 
read minds, and partly because it can't know things like "breaking this line of 
code here is really stoopid".

One way to address these problems, especially for new devs or drive-by 
contributions, is stating clearly in the wiki page:
"use clang-format, it should get you at least half way there, but you still 
should/must also override it to match the style of surrounding code, as much as 
possible, in the file(s) you're editing, that's kinder to reviewers".

Technically, that's what I personally do, it's the same as having bash scripts, 
even if I can get it to do 50% of what I want, just not having to repeat myself to 
run some chain of commands 10 times a day is good enough.

> file from which it would pick formatting, we wouldn't have this
> discussion, either. But what I read between the lines is that Google is
> happy that it can reproduce their style, and contributions to extend it
> are not welcome.
> 
>> One way to stop the recurring discussions would be to reach a majority
>> consensus about a clang-format config file, format the whole code base
>> of each module in one go, then apply the formatting for each new commit,
>> less discussions. Is that ideal? no, because reaching a consensus about
>> coding style is not easy.
> 
> There _is_ consensus. It's in the wiki. And in older modules not
> infected by the _clang-format file. Discussions arise because
> of .clang-format, not despite it. Afaict, there never was a discussion
> about how faithful the _clang-format represents the Qt style before it
> was added. If there was _any_ attempt at the above-mentioned litmus
> test, the template<> issue and others would have been detected immediately.
> 
>> I could format it manually, probably two steps, or let the tool do it
> 
> That assumes the tool can be configured to reproduce the style. Until
> proven, you have to do the work 50% of the time, anyway. Tool won't help.
> 

It can/is configured to reproduce the style, but only as much as a tool can; it 
still needs the author to tweak/override/revert it as needed.

> Believe me, I wouldn't like anything better than a clang-format config
> that shuts these discussions up. But not at the cost of rewriting 50% of
> our code, a large percentage of which still dates back unchanged to the
> beginning of the public history.
> 

Yeah, the "Long live Qt 4.5!" commit, very annoying when you hit that one while 
digging in git log, e.g. while using 'git log -p -S"whatever"' and searching in 
the output.


> Thanks,
> Marc
> 

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/20230913/4141faee/attachment.sig>