[Differential] [Commented On] D1924: KXMLGui: Fix merge indices when removing xmlgui clients with actions in groups.

[svuorela (Sune Vuorela)]

svuorela added inline comments.

INLINE COMMENTS

> kxmlgui_unittest.cpp:306
> +        if (i >= actions.count())
> +            break;
> +        QAction *action = actions.at(i);

Isn't it better to move the last QCOMPARE(count,count); up first?  Or is it to  be able to easier debug if something went wrong?

> kxmlguibuilder.h:83
>  
> +    // KF6 TODO: REMOVE
>      virtual void removeCustomElement(QWidget *parent, QAction *action);

And use what instead? maybe mark deprecated?

> kxmlguifactory_p.cpp:391
> +{
> +    Q_FOREACH (QAction *action, actions) {
> +        const int pos = container->actions().indexOf(action);

actions is const, so range based for is free to use

> dhaumann wrote in kxmlguifactory_p.cpp:717
> I'm not sure I get this in detail: but as you already mentioned yesterday, it seems this saves iterators. It seems to me this code was written at a time (?) when QList iterators were stable, but since a QList behaves similar to a QVector for ints (iirc), this is rather dangerous, right? I guess that is what you imply with your comment here. In that case, it's a wonder all this is still working ;)

this is the only place the QList is actualyl modified so it is actually safe. but only until the next person modifies the code...

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D1924

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dfaure, svuorela, dhaumann
Cc: kde-frameworks-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160617/42409776/attachment-0001.html>