Preview of astyle-kdelibs changes up in a branch (was: Re: [Port blocker] Re: [Kexi-devel] RFC: plan for starting the Qt5/KF5 port)

[Boudewijn Rempt]

On Fri, 6 Mar 2015, Elvis Stansvik wrote:

> 2015-03-06 8:25 GMT+01:00 Dmitry Kazakov <dimula73 at gmail.com>:
>       Hi!
> 
> I looked through the patch and updated the google docs:
> https://docs.google.com/document/d/1jhq6oXuXKvTilJhcoS6FVKO7yYRu2yCgBS9ojhc2QRU/edit#
> 
> The changes which I believe we must fix before applying it to master:
> 
> 1) Inlined functions. They are not covered by the KDE style policy, so I believe we should follow Qt- and KDE- style of handling inlined functions with the opening brace at the signature line. It
> applies only to the functions declared *and* defined inside the class itself.
> 
> 
> I can't see Qt following this rigorously. Here's an example of both styles mixed in the same file:
> 
>    https://github.com/qtproject/qtbase/blob/dev/src/corelib/io/qfileinfo_p.h
> 
> Why not try to be consistent in Calligra and use the same style everywhere (inline or not inline)? Less rules to remember.


Yes, I wouldn't call this a blocker.

>  
>
>       2) "Wrong alignment of function parameters". See the corresponding topic in the google docs. Astyle seems to be a bit broken here, because in some places it applies correctly, and in some not.
>       The units that are always wrong are constructors.

This is worse -- do you know whether this is tunable, Friedrich? 
I wish that for this and the next problem, it were possible to use Qt 
Creator's whitespacer, that does the correct thing...

>
>       3) "Indentation of multiline conditionals is wrong". See the corresponding topic in the google docs. Should be 8 spaces instead of 12.
> 
> I think we should commit the patch only after we fix at least these three issues.

Are they fixable? If not, it's a matter of deciding what is worth more: 
finally getting rid of all the misplaced stars, brackets and so on, or 
fixing up these issues manually. I guess I could try to load all files in 
Kate and apply its clean indentation manually...

But we need a resolution for this, because it's blocking me porting 
calligra/libs so the rest of the porting effort can start!

Boudewijn

> 
> (optional)
> 4) I really feel we must *not* follow one-line-if-always-with-braces rule. 'if (some_stuff) return 0' is perfectly ok in my opinion. But this is my opinion only and I cannot block the patch with that.
> I only search for support from other developers in this topic.
> 
> 
> I have no strong opinion on this. But isn't the point that we pick a rule and stick to it? The one-line-if-always-with-braces rule has been in effect in Calligra for quite a while. It also has the benefit of
> 1) less rules to remember, 2) consistent with kdelibs.
> 
> Regards,
> Elvis
>  
> 
> 
> On Thu, Mar 5, 2015 at 11:05 AM, Boudewijn Rempt <boud at valdyas.org> wrote:
>       On Thu, 5 Mar 2015, C. Boemann wrote:
>
>             As i already like the previous results this is just an improvement afaict.
>
>             However, as a pure enhancement, do you know how difficult it would be to reformat all ctor initialize statements to be like:
>
>             class::class()
>                : super()
>                , member(val)
>             {
>
>             I know it would require a patch, just wondering how much work it would be. If you don't want to spend time on it we can keep it as it is - but we have like 3 different styles in
>             use so it would be nice to clear it up too,
> 
>
>       I would love that as well. This style of initializer lists is the easiest to maintain.
>
>       Boudewijn
>       _______________________________________________
>       calligra-devel mailing list
>       calligra-devel at kde.org
>       https://mail.kde.org/mailman/listinfo/calligra-devel
> 
> 
> 
> 
> --
> Dmitry Kazakov
> 
> _______________________________________________
> calligra-devel mailing list
> calligra-devel at kde.org
> https://mail.kde.org/mailman/listinfo/calligra-devel
> 
> 
> 
>