[Development] clang-format

[Eirik Aavitsland]

On 20. juni 2018 13:23, Morten Sørvig wrote:
> 
> 
>> On 19 Jun 2018, at 17:33, Allan Sandfeld Jensen <kde at carewolf.com> wrote:
>>
>> Btw. Just for your information.
>>
>> I have attached a few random examples of what we can look forward too after
>> running an auto-"beautifying" tool over our hand-formated Qt code. And these
>> changes are NOT something we can configure out ouf of in clang-format, it is
>> just cases where it can't do any better.<clang-beautified.txt>_______________________________________________
>> Development mailing list
>> Development at qt-project.org
>> http://lists.qt-project.org/mailman/listinfo/development
> 
> Thanks, this should really be the first thing we do: run a pre-project
> so that we can have an informed discussion.
> 
> The point of machine formatting is perhaps not to make code beautiful, rather
> it is to have automated formatting in order to offload work from the humans.
> 
> 
> Looking at qConvertRgb16To32:
> 
> Manual formatting:
>      return 0xff000000
>          | ((((c) << 3) & 0xf8) | (((c) >> 2) & 0x7))
>          | ((((c) << 5) & 0xfc00) | (((c) >> 1) & 0x300))
>          | ((((c) << 8) & 0xf80000) | (((c) << 3) & 0x70000));
> 
> Clang-format:
>      return 0xff000000 | ((((c) << 3) & 0xf8) | (((c) >> 2) & 0x7)) | ((((c) << 5) & 0xfc00) | (((c) >> 1) & 0x300))
>             | ((((c) << 8) & 0xf80000) | (((c) << 3) & 0x70000));
> 
> The manually formatted version clearly has something going for it.
> 

Yup, I also did some manual testing on existing Qt code, and I generally 
found that

- clang-format's intra-line changes were fine (correcting white space in 
expressions etc.)

- but whenever clang-format would add or remove linebreaks, which it 
would quite often do, the changes were typically detrimental to 
readability, and often significantly so (as in the example above).

In my view, the readability improvements of the former in no way 
outweighs the readability damage of the latter. So to me this tool (or 
its config) is not ready to be trusted with the power of doing automatic 
(not developer reviewed & approved) changes.

- Eirik Aa.