Merge lp:~aacid/dee-qt/behave_better into lp:dee-qt

Proposed by Albert Astals Cid
Status: Merged
Approved by: Michal Hruby
Approved revision: 83
Merged at revision: 81
Proposed branch: lp:~aacid/dee-qt/behave_better
Merge into: lp:dee-qt
Diff against target: 65 lines (+12/-4)
1 file modified
deelistmodel.cpp (+12/-4)
To merge this branch: bzr merge lp:~aacid/dee-qt/behave_better
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Michal Hruby (community) Approve
Review via email: mp+181258@code.launchpad.net

Commit message

Be a better behaved QAbstractListModel

That means:
  * decrement/increment the count between the begin/end calls
  * Add a workaround so that data() ignores the not really removed row after endRemoveRows has been called

We still are not good behaved in the fact that we call beginInsertRows
after the model has been internall modified, but luckily it seems
not much people listens to that signal's data

We also rely on the fact that nothing will call data() between us
resetting m_rowBeingRemoved to -1 at the end of DeeListModelPrivate::onRowRemoved
and the internal dee model actually removing the row

To post a comment you must log in.
lp:~aacid/dee-qt/behave_better updated
82. By Albert Astals Cid

move the if inside the if

83. By Albert Astals Cid

Actually don't need the if

Revision history for this message
Michal Hruby (mhr3) wrote :

Looks reasonable, it certainly isn't ideal, but that's as far as we can go since changes to dee models emit just a single signal, which isn't easy to map to Qt's begin and end change model.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'deelistmodel.cpp'
2--- deelistmodel.cpp 2013-07-25 14:23:11 +0000
3+++ deelistmodel.cpp 2013-08-21 11:03:21 +0000
4@@ -49,9 +49,10 @@
5 int m_count;
6 bool m_listeningSynchronized;
7 QHash<int, QByteArray> m_roleNames;
8+ int m_rowBeingRemoved;
9 };
10
11-DeeListModelPrivate::DeeListModelPrivate(DeeListModel* parent) : m_parent(parent), m_deeModel(NULL), m_count(0), m_listeningSynchronized(false)
12+DeeListModelPrivate::DeeListModelPrivate(DeeListModel* parent) : m_parent(parent), m_deeModel(NULL), m_count(0), m_listeningSynchronized(false), m_rowBeingRemoved(-1)
13 {
14 }
15
16@@ -180,7 +181,6 @@
17 return;
18 }
19
20- model->d->m_count++;
21 gint position = dee_model_get_position(model->d->m_deeModel, iter);
22 /* Force emission of QAbstractItemModel::rowsInserted by calling
23 beginInsertRows and endInsertRows. Necessary because according to the
24@@ -189,6 +189,7 @@
25 cannot be explicitly emitted in subclass code."
26 */
27 model->beginInsertRows(QModelIndex(), position, position);
28+ model->d->m_count++;
29 model->endInsertRows();
30 Q_EMIT model->countChanged();
31 }
32@@ -207,7 +208,6 @@
33 what one would expect.
34 See Dee's dee_sequence_model_remove() method.
35 */
36- model->d->m_count--;
37 gint position = dee_model_get_position(model->d->m_deeModel, iter);
38 /* Force emission of QAbstractItemModel::rowsRemoved by calling
39 beginRemoveRows and endRemoveRows. Necessary because according to the
40@@ -216,8 +216,11 @@
41 cannot be explicitly emitted in subclass code."
42 */
43 model->beginRemoveRows(QModelIndex(), position, position);
44+ model->d->m_rowBeingRemoved = position;
45+ model->d->m_count--;
46 model->endRemoveRows();
47 Q_EMIT model->countChanged();
48+ model->d->m_rowBeingRemoved = -1;
49 }
50
51 void
52@@ -316,7 +319,12 @@
53 GVariant* gvariant;
54 DeeModelIter* iter;
55
56- iter = dee_model_get_iter_at_row(d->m_deeModel, index.row());
57+ int row = index.row();
58+ if (d->m_rowBeingRemoved >= 0 && row >= d->m_rowBeingRemoved) {
59+ row++;
60+ }
61+
62+ iter = dee_model_get_iter_at_row(d->m_deeModel, row);
63 gvariant = dee_model_get_value(d->m_deeModel, iter, role);
64 QVariant qvariant = QVariantFromGVariant(gvariant);
65 g_variant_unref(gvariant);

Subscribers

People subscribed via source and target branches

to all changes: