Merge lp:~nick-dedekind/qmenumodel/lp1385331.led into lp:qmenumodel

[Nick Dedekind]

Add support for overriding QDBusActionGroup state parser

Add support for overriding QDBusActionGroup state parser

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:115
http://jenkins.qa.ubuntu.com/job/qmenumodel-ci/54/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qmenumodel-vivid-amd64-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qmenumodel-vivid-armhf-ci/1
        deb: http://jenkins.qa.ubuntu.com/job/qmenumodel-vivid-armhf-ci/1/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qmenumodel-ci/54/rebuild

review: Approve (continuous-integration)

[lp:~nick-dedekind/qmenumodel/lp1385331.led]

116. By Nick Dedekind on 2014-11-11

bump version

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:116
http://jenkins.qa.ubuntu.com/job/qmenumodel-ci/55/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qmenumodel-vivid-amd64-ci/2/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qmenumodel-vivid-armhf-ci/2
        deb: http://jenkins.qa.ubuntu.com/job/qmenumodel-vivid-armhf-ci/2/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qmenumodel-ci/55/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:116
http://jenkins.qa.ubuntu.com/job/qmenumodel-ci/56/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qmenumodel-vivid-amd64-ci/3
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qmenumodel-vivid-armhf-ci/3
        deb: http://jenkins.qa.ubuntu.com/job/qmenumodel-vivid-armhf-ci/3/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qmenumodel-ci/56/rebuild

review: Approve (continuous-integration)

[Albert Astals Cid (aacid)]

Should we delete m_actionStateParser in m_actionStateParser ?

review: Needs Fixing

[Albert Astals Cid (aacid)]

I meant:

Should we delete m_actionStateParser in setActionStateParser ?

[Albert Astals Cid (aacid)]

But...

Having a look at how setActionStateParser is used in https://code.launchpad.net/~nick-dedekind/unity8/lp1385331.led/+merge/241417 it seems it should not be deleted, but the code still makes me a bit unhappy since it's the typical structure in which you need to delete m_actionStateParser otherwise you leak the one you created in the constructor.

What about instead this change adding a
  Q_INVOKABLE QVariant actionState(const QString &name, ActionStateParser* actionStateParser);
?

So if it is passed you use it and otherwise you use Converter::toQVariant ?

review: Needs Information

[Albert Astals Cid (aacid)]

After reading the code more i see my suggestion is not implementable without a big-ish rework (and maybe not at all) so i'll be happy enough if you add a comment to setActionStateParser saying that the class does not take ownership of actionStateParser

review: Needs Fixing

[lp:~nick-dedekind/qmenumodel/lp1385331.led]

117. By Nick Dedekind on 2014-12-10

added comment

[Nick Dedekind (nick-dedekind)]

> After reading the code more i see my suggestion is not implementable without a
> big-ish rework (and maybe not at all) so i'll be happy enough if you add a
> comment to setActionStateParser saying that the class does not take ownership
> of actionStateParser

done

[Albert Astals Cid (aacid)]

Looks good.

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2014-10-13 09:36:50 +0000
+++ debian/changelog	2014-12-10 16:23:52 +0000
@@ -1,3 +1,9 @@
+qmenumodel (0.2.9-0ubuntu1) UNRELEASED; urgency=medium
+
+  * Added support for overriding QDBusActionGroup state parser
+
+ -- Nick Dedekind <[e-mail]>  Tue, 11 Nov 2014 15:25:11 +0000
+
 qmenumodel (0.2.8+14.10.20141013-0ubuntu1) utopic; urgency=low
 
   [ Nick Dedekind ]

=== modified file 'libqmenumodel/src/qdbusactiongroup.cpp'
--- libqmenumodel/src/qdbusactiongroup.cpp	2014-10-10 17:28:11 +0000
+++ libqmenumodel/src/qdbusactiongroup.cpp	2014-12-10 16:23:52 +0000
@@ -17,6 +17,7 @@
  *      Renato Araujo Oliveira Filho <[e-mail]>
  */
 
+#include "actionstateparser.h"
 #include "qdbusactiongroup.h"
 #include "qstateaction.h"
 #include "converter.h"
@@ -58,7 +59,8 @@
 QDBusActionGroup::QDBusActionGroup(QObject *parent)
     :QObject(parent),
      QDBusObject(this),
-     m_actionGroup(NULL)
+     m_actionGroup(NULL),
+     m_actionStateParser(new ActionStateParser(this))
 {
 }
 
@@ -89,7 +91,13 @@
 {
     QVariant result;
     GVariant *state = g_action_group_get_action_state(m_actionGroup, name.toUtf8().data());
-    result = Converter::toQVariant(state);
+
+    if (m_actionStateParser != NULL) {
+        result = m_actionStateParser->toQVariant(state);
+    } else {
+        result = Converter::toQVariant(state);
+    }
+
     if (state) {
         g_variant_unref(state);
     }
@@ -190,6 +198,19 @@
     }
 }
 
+ActionStateParser* QDBusActionGroup::actionStateParser() const
+{
+    return m_actionStateParser;
+}
+
+void QDBusActionGroup::setActionStateParser(ActionStateParser* actionStateParser)
+{
+    if (m_actionStateParser != actionStateParser) {
+        m_actionStateParser = actionStateParser;
+        Q_EMIT actionStateParserChanged(actionStateParser);
+    }
+}
+
 /*! \internal */
 void QDBusActionGroup::clear()
 {

=== modified file 'libqmenumodel/src/qdbusactiongroup.h'
--- libqmenumodel/src/qdbusactiongroup.h	2013-08-09 08:01:43 +0000
+++ libqmenumodel/src/qdbusactiongroup.h	2014-12-10 16:23:52 +0000
@@ -26,6 +26,7 @@
 #include <QVariant>
 
 class QStateAction;
+class ActionStateParser;
 
 typedef char gchar;
 typedef void* gpointer;
@@ -40,6 +41,7 @@
     Q_PROPERTY(QString busName READ busName WRITE setBusName NOTIFY busNameChanged)
     Q_PROPERTY(QString objectPath READ objectPath WRITE setObjectPath NOTIFY objectPathChanged)
     Q_PROPERTY(int status READ status NOTIFY statusChanged)
+    Q_PROPERTY(ActionStateParser* actionStateParser READ actionStateParser WRITE setActionStateParser NOTIFY actionStateParserChanged)
 
 public:
     QDBusActionGroup(QObject *parent=0);
@@ -52,6 +54,10 @@
     Q_INVOKABLE QStateAction *action(const QString &name);
     Q_INVOKABLE QVariant actionState(const QString &name);
 
+    ActionStateParser* actionStateParser() const;
+    // Ownership of the actionStateParser is not passed to this object.
+    void setActionStateParser(ActionStateParser* actionStateParser);
+
 Q_SIGNALS:
     void busTypeChanged(DBusEnums::BusType type);
     void busNameChanged(const QString &busNameChanged);
@@ -60,6 +66,7 @@
     void actionAppear(const QString &name);
     void actionVanish(const QString &name);
     void actionStateChanged(const QString &name, QVariant state);
+    void actionStateParserChanged(ActionStateParser* parser);
 
 public Q_SLOTS:
     void start();
@@ -77,6 +84,8 @@
     int m_signalActionRemovedId;
     int m_signalStateChangedId;
 
+    ActionStateParser* m_actionStateParser;
+
     // workaround to support int as busType
     void setIntBusType(int busType);