Review Request 126691: Add DBusMenuShortcut type overload for QDBusArgument

[David Kahles]

> On Jan. 10, 2016, 2:06 p.m., David Edmundson wrote:
> > That sounds like you're saying Qt has broken API. 
> > If that's true, they'll want to know about it.

I talked to them (see https://codereview.qt-project.org/#/c/144823/) and it seems like we shouldn't have done it like it was before, it worked by pure luck.
According to them, this change will do the right thing, but additional, we shouldn't inherit containers at all.
(I think the problem is, that QT containers don't have a virtual destructor)

Btw., libdbusmenu-qt suffers from the same problem, but I'm  not sure whether this is our responsibility (I'm confused because there's kind of a fork in plasma).


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126691/#review90839
-----------------------------------------------------------


On Jan. 10, 2016, 2:59 a.m., Dāvis Mosāns wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126691/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2016, 2:59 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> Add DBusMenuShortcut type overload for QDBusArgument.
> I don't know if there's a better way to fix this than this kinda code duplication.
> 
> 
> This is needed because otherwise it wouldn't compile with latest Qt dev branch. It's probably because QList<T> overload was removed in http://code.qt.io/cgit/qt/qtbase.git/commit/src/dbus/qdbusargument.h?h=dev&id=5f542f3cca13f2da58b82aee2efbaffefeee00a7
> 
> and Container<T> doesn't work...
> 
> /usr/include/QtDBus/qdbusargument.h:244:29: note: candidate: template<template<class> class Container, class T> const QDBusArgument& operator>>(const QDBusArgument&, Container<T>&)
>  inline const QDBusArgument &operator>>(const QDBusArgument &arg, Container<T> &list)
>                              ^
> /usr/include/QtDBus/qdbusargument.h:244:29: note:   template argument deduction/substitution failed:
> /mnt/KDE/kde/workspace/plasma-workspace/dataengines/statusnotifieritem/libdbusmenuqt/dbusmenuimporter.cpp:261:16: note:   can't deduce a template for ‘Container<T>’ from non-template type ‘DBusMenuShortcut’
>          arg >> dmShortcut;
>                 ^
> In file included from /usr/include/QtDBus/qdbuspendingreply.h:39:0,
>                  from /usr/include/QtDBus/qdbusreply.h:44,
>                  from /usr/include/QtDBus/QDBusReply:1,
>                  from /mnt/KDE/kde/workspace/plasma-workspace/dataengines/statusnotifieritem/libdbusmenuqt/dbusmenuimporter.cpp:27
> 
> 
> Diffs
> -----
> 
>   dataengines/statusnotifieritem/libdbusmenuqt/dbusmenutypes_p.h 4950a22279c09fb93c68fe3d38ff600279e856ca 
>   dataengines/statusnotifieritem/libdbusmenuqt/dbusmenutypes_p.cpp e98c4b93bc8532367ee96138ce72a54f44ac05ca 
> 
> Diff: https://git.reviewboard.kde.org/r/126691/diff/
> 
> 
> Testing
> -------
> 
> Compiles
> 
> 
> Thanks,
> 
> Dāvis Mosāns
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160207/8e1a98c1/attachment.html>