On Fri, 6 Mar 2015, Elvis Stansvik wrote:
2015-03-06 8:25 GMT+01:00 Dmitry Kazakov <dimul...@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 <b...@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@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel
--
Dmitry Kazakov
_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel
_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel
