Merge lp:~mterry/lightdm/qt-get into lp:lightdm

[Michael Terry]

Add get() methods to the Qt models, to emulate what ListModel does in Qml

Add get() methods to the two Qt models. This makes it possible to get data from arbitrary rows of the model (otherwise, Qml only exposes the currently selected row).

[David Edmundson (david.edmundson)]

That's not true, use itemAt() in your repeater in QML, then you have access to all the properties in your delegate (and thus model)

Show me your QML and I can take a look if you like.

[Michael Terry (mterry)]

I thought itemAt took x,y coordinates, so was only good if (A) you had x,y and (B) the item was on the screen. So not quite the same as access-by-row, right?

[David Edmundson (david.edmundson)]

No, that's Item::ChildAt
Repeater::itemAt takes an index.

http://doc.qt.digia.com/stable/qml-repeater.html#itemAt-method

[Michael Terry (mterry)]

OK. But Repeater seems uncommon? I'm used to having my models be part of a PathView or ListView or something. And those use the x,y version of itemAt.

So when using liblightdm-qt's models with such widgets, get() would still be useful, no?

[David Edmundson (david.edmundson)]

In "normal" C++ code one would do model->index(rowNumber,0).data(Qt::DisplayRole) that's quite common. Quite often one passes round QPersistantModelIndexes anyway so you can just call data() on that.

From a C++ QWidget perspective this would be really wrong and pointless, however I do see why you want to do it to work round QML's limitations. I hate making bad Qt libraries with duplication for the sake of QML, which is what this comes down to.

If you don't have a itemAt() method you can still call pathView.children[index].property.
What I'll do is check with some other Qt devs and get their opinion and report back.

[David Edmundson (david.edmundson)]

So I don't have any outstanding reviews I'm going to close this on the grounds that this is a Qt library and not a QML one.

You need to write helper code to call qmlRegisterType anyway so any other QML or greeter specific code should be done in your code and not in here which is shared in many projects.

For this particular case it's probably worth having a helper method in Ubuntu globally, rather than trying to update every existing model. KDE, for example, uses this: http://api.kde.org/4.x-api/kde-runtime-apidocs/plasma/declarativeimports/core/html/classPlasma_1_1SortFilterModel.html

(BTW, If you do go with this approach, call itemData() once on the model then loop through that rather than calling data() for each role which is a much slower operation performing a list lookup each time)

review: Disapprove

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1597
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~mterry/lightdm/qt-get/+merge/149119/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/lightdm-ci/3/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/lightdm-quantal-amd64-ci/3/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/lightdm-ci/3/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1597
http://jenkins.qa.ubuntu.com/job/lightdm-ci/9/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/lightdm-raring-amd64-ci/3

Click here to trigger a rebuild:
http://s-jenkins:8080/job/lightdm-ci/9/rebuild

review: Approve (continuous-integration)

[Robert Ancell (robert-ancell)]

Outside my Qt knowledge.

review: Abstain

[Michael Terry (mterry)]

I'm fine without this. I'll close it.

Unmerged revisions

1597. By Michael Terry on 2013-02-18

Add get() methods to the Qt models, to emulate what ListModel does in Qml

=== modified file 'liblightdm-qt/QLightDM/sessionsmodel.h'
--- liblightdm-qt/QLightDM/sessionsmodel.h	2013-01-31 20:56:09 +0000
+++ liblightdm-qt/QLightDM/sessionsmodel.h	2013-02-18 18:19:24 +0000
@@ -38,6 +38,7 @@
 
         int rowCount(const QModelIndex &parent) const;
         QVariant data(const QModelIndex &index, int role=Qt::DisplayRole) const;
+        Q_INVOKABLE QVariantMap get(int row) const;
 
     protected:
         SessionsModelPrivate *d_ptr;

=== modified file 'liblightdm-qt/QLightDM/usersmodel.h'
--- liblightdm-qt/QLightDM/usersmodel.h	2013-01-31 20:56:09 +0000
+++ liblightdm-qt/QLightDM/usersmodel.h	2013-02-18 18:19:24 +0000
@@ -38,6 +38,7 @@
 
     int rowCount(const QModelIndex &parent) const;
     QVariant data(const QModelIndex &index, int role) const;
+    Q_INVOKABLE QVariantMap get(int row) const;
 
 protected:
 

=== modified file 'liblightdm-qt/sessionsmodel.cpp'
--- liblightdm-qt/sessionsmodel.cpp	2013-02-06 14:08:25 +0000
+++ liblightdm-qt/sessionsmodel.cpp	2013-02-18 18:19:24 +0000
@@ -146,6 +146,19 @@
     return QVariant();
 }
 
+QVariantMap SessionsModel::get(int row) const
+{
+    QVariantMap result;
+    QHashIterator<int, QByteArray> i(roleNames());
+    while (i.hasNext()) {
+        i.next();
+        QModelIndex modelIndex = index(row, 0);
+        QVariant data = modelIndex.data(i.key());
+        result[i.value()] = data;
+     }
+     return result;
+}
+
 #if QT_VERSION >= QT_VERSION_CHECK(5, 0, 0)
 #include "sessionsmodel_moc5.cpp"
 #else

=== modified file 'liblightdm-qt/usersmodel.cpp'
--- liblightdm-qt/usersmodel.cpp	2013-02-07 14:26:19 +0000
+++ liblightdm-qt/usersmodel.cpp	2013-02-18 18:19:24 +0000
@@ -242,6 +242,18 @@
     return QVariant();
 }
 
+QVariantMap UsersModel::get(int row) const
+{
+    QVariantMap result;
+    QHashIterator<int, QByteArray> i(roleNames());
+    while (i.hasNext()) {
+        i.next();
+        QModelIndex modelIndex = index(row, 0);
+        QVariant data = modelIndex.data(i.key());
+        result[i.value()] = data;
+     }
+     return result;
+}
 
 #if QT_VERSION >= QT_VERSION_CHECK(5, 0, 0)
 #include "usersmodel_moc5.cpp"