qmenumodel

Merge lp:~aacid/qmenumodel/aboutToShow into lp:qmenumodel

  1. aboutToShow
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Merged
Approved by: Charles Kerr
Approved revision: 132
Merged at revision: 130
Proposed branch: lp:~aacid/qmenumodel/aboutToShow
Merge into: lp:qmenumodel
Diff against target: 96 lines (+51/-2)
3 files modified
debian/changelog (+7/-0)
libqmenumodel/src/unitymenumodel.cpp (+42/-1)
libqmenumodel/src/unitymenumodel.h (+2/-1)
To merge this branch: bzr merge lp:~aacid/qmenumodel/aboutToShow
Reviewer Review Type Date Requested Status
Charles Kerr (community) Approve
Review via email: mp+319078@code.launchpad.net

Commit message

Call about to show for those items that have a qtubuntu-tag property

To post a comment you must log in.
lp:~aacid/qmenumodel/aboutToShow updated
130. By Albert Astals Cid

Increase version because of new api

Revision history for this message
dobey (dobey) :
Revision history for this message
Charles Kerr (charlesk) wrote :

comments inline

review: Needs Fixing
Revision history for this message
Albert Astals Cid (aacid) wrote :

I will add all those auto, but those are *totally* not on the style of the rest of the file, basically the file has 0 auto variables.

> You probably meant to use -1 here to use the default timeout? I'm not sure what specifying a timeout of 0msec would do but it's probably not good
No, I don't want the default timeout, i simply want no timeout, it either works or doesn't, i don't see why it's "probably not good" to use a 0 timeout

> Not a blocker, but want to confirm that you meant to not provide a callback here so that you can log any errors reported by g_dbus_connection_call_finish(), yes?
Yes, if you want I can add error reporting, but don't think it's going to give us much tbh.

lp:~aacid/qmenumodel/aboutToShow updated
131. By Albert Astals Cid

Review improvements

Revision history for this message
Charles Kerr (charlesk) wrote :

> I will add all those auto, but those are *totally* not on the style of the
> rest of the file, basically the file has 0 auto variables.

No worries, as I said those were trivial comments. Redundant type info
is bad, but inconsistency is also bad, pick your minor poison :)

> > You probably meant to use -1 here to use the default timeout? I'm not sure
> what specifying a timeout of 0msec would do but it's probably not good
> No, I don't want the default timeout, i simply want no timeout, it either
> works or doesn't, i don't see why it's "probably not good" to use a 0 timeout

Because the docs say use G_MAXINT for no timeout.

g_dbus_connection_call() is in gdbusconnection.c, which the call down a few steps.
The timeout is applied in g_dbus_connection_send_message_with_reply_unlocked(),
which reads:

  if (timeout_msec != G_MAXINT)
    {
      data->timeout_source = g_timeout_source_new (timeout_msec);
      g_task_attach_source (task, data->timeout_source,
                            (GSourceFunc) send_message_with_reply_timeout_cb);
      g_source_unref (data->timeout_source);
    }

I /think/ you are effectively asking the call to timeout asap.

> > Not a blocker, but want to confirm that you meant to not provide a callback
> here so that you can log any errors reported by
> g_dbus_connection_call_finish(), yes?
> Yes, if you want I can add error reporting, but don't think it's going to give
> us much tbh.

Agreed that it's minor.

Keeping as NF for the G_MAXINT issue

review: Needs Fixing
lp:~aacid/qmenumodel/aboutToShow updated
132. By Albert Astals Cid

0 -> G_MAXINT as found by Charles

Revision history for this message
Charles Kerr (charlesk) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2017-01-10 06:41:53 +0000
3+++ debian/changelog 2017-03-16 15:50:45 +0000
4@@ -1,3 +1,10 @@
5+qmenumodel (0.2.12) UNRELEASED; urgency=medium
6+
7+ [ Albert Astals Cid ]
8+ * Add UnityMenuModel::aboutToShow ((LP: 1664578)
9+
10+ -- Albert Astals Cid <albert.astals@canonical.com> Tue, 07 Mar 2017 13:08:46 +0100
11+
12 qmenumodel (0.2.11+17.04.20170110.1-0ubuntu1) zesty; urgency=medium
13
14 [ Nick Dedekind ]
15
16=== modified file 'libqmenumodel/src/unitymenumodel.cpp'
17--- libqmenumodel/src/unitymenumodel.cpp 2016-12-09 17:06:25 +0000
18+++ libqmenumodel/src/unitymenumodel.cpp 2017-03-16 15:50:45 +0000
19@@ -289,7 +289,7 @@
20 priv = new UnityMenuModelPrivate(this);
21 }
22
23-UnityMenuModel::UnityMenuModel(const UnityMenuModelPrivate& other, QObject *parent):
24+UnityMenuModel::UnityMenuModel(const UnityMenuModelPrivate& other, UnityMenuModel *parent):
25 QAbstractListModel(parent)
26 {
27 priv = new UnityMenuModelPrivate(other, this);
28@@ -729,6 +729,47 @@
29 }
30 }
31
32+void UnityMenuModel::aboutToShow(int index)
33+{
34+ GSequenceIter *it = g_sequence_get_iter_at_pos (priv->items, index);
35+ if (g_sequence_iter_is_end (it)) {
36+ return;
37+ }
38+
39+ auto item = static_cast<GtkMenuTrackerItem*>(g_sequence_get(it));
40+ if (!item) {
41+ return;
42+ }
43+
44+ quint64 actionTag;
45+ if (gtk_menu_tracker_item_get_attribute (item, "qtubuntu-tag", "t", &actionTag)) {
46+ // Child UnityMenuModel have priv->connection null, so climb to the parent until we find a non null one
47+ UnityMenuModelPrivate *privToUse = priv;
48+ while (privToUse && !privToUse->connection) {
49+ auto pModel = dynamic_cast<UnityMenuModel*>(privToUse->model->QObject::parent());
50+ if (pModel) {
51+ privToUse = pModel->priv;
52+ } else {
53+ privToUse = nullptr;
54+ }
55+ }
56+ if (privToUse) {
57+ g_dbus_connection_call (privToUse->connection,
58+ privToUse->busName,
59+ privToUse->menuObjectPath,
60+ "qtubuntu.actions.extra",
61+ "aboutToShow",
62+ g_variant_new("(t)", actionTag),
63+ nullptr,
64+ G_DBUS_CALL_FLAGS_NO_AUTO_START,
65+ G_MAXINT,
66+ nullptr,
67+ nullptr,
68+ nullptr);
69+ }
70+ }
71+}
72+
73 void UnityMenuModel::activateByVariantString(int index, const QString& parameter)
74 {
75 activate(index, Converter::toQVariantFromVariantString(parameter));
76
77=== modified file 'libqmenumodel/src/unitymenumodel.h'
78--- libqmenumodel/src/unitymenumodel.h 2016-10-18 09:55:14 +0000
79+++ libqmenumodel/src/unitymenumodel.h 2017-03-16 15:50:45 +0000
80@@ -63,6 +63,7 @@
81 Q_INVOKABLE QVariant get(int row, const QByteArray &role);
82
83 Q_INVOKABLE void activate(int index, const QVariant& parameter = QVariant());
84+ Q_INVOKABLE void aboutToShow(int index);
85 Q_INVOKABLE void activateByVariantString(int index, const QString& parameter = QString());
86 Q_INVOKABLE void changeState(int index, const QVariant& parameter);
87 Q_INVOKABLE void changeStateByVariantString(int index, const QString& parameter);
88@@ -90,7 +91,7 @@
89 class UnityMenuModelPrivate *priv;
90 friend class UnityMenuModelPrivate;
91
92- UnityMenuModel(const UnityMenuModelPrivate& other, QObject *parent);
93+ UnityMenuModel(const UnityMenuModelPrivate& other, UnityMenuModel *parent);
94 };
95
96 #endif

Subscribers

People subscribed via source and target branches