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 itThat 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
OpenPGP_signature
Description: OpenPGP digital signature
-- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
