[datamodel] Rework items insert/remove
AbandonedPublic

Authored by anthonyfieroni on Jan 14 2019, 6:31 PM.

Details

Reviewers
davidedmundson
broulik
ngraham
mart
Group Reviewers
Plasma
Summary

I give a try to rework insertion / deletion of items in datamodel which is used in systemtray. This continues approach from D16890, what the problem is

  1. Right click systemtray -> configure
  2. Click on Entries
  3. Click Ok

Even only touching model dismissing dialog made index after invalid, now clicking on any entry in systray result in invalid index

Old approach tries to make optimization but that's not a right place since QML is time-to-time rewritten itself, also it sets role names incorrect to me.

Test Plan

Still not complete tested

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Lint Skipped
Unit
Unit Tests Skipped
anthonyfieroni created this revision.Jan 14 2019, 6:31 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 14 2019, 6:31 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
anthonyfieroni requested review of this revision.Jan 14 2019, 6:31 PM
davidedmundson added inline comments.Jan 14 2019, 7:53 PM
src/declarativeimports/core/datamodel.cpp
55–61

This part, ship it! but I don't think it'll make a huge difference to anything

77

This shouldn't be needed

setSourceModel will will do a reset internally

379

I don't understand what you're changing here, can you provide a bit more detail on the exact problem.

444

this isn't equivalent.

I could have an entry with an empty count

Now this gets left behind

anthonyfieroni added inline comments.Jan 16 2019, 9:34 AM
src/declarativeimports/core/datamodel.cpp
379

Optimization is you have [5, 6, 8] then [5, 6, 8, 9] so it make only
beginInsertRows({}, 3, 3);
full replace (m_items[sourceName] = list.toVector();)
endInsertRows()
Same if you [5, 6]
beginRemoveRows
full replace (m_items[sourceName] = list.toVector();)
endRemoveRows

It does not touch either m_roleNames size, which are changed.

I try to remove all first, then re-add with m_roleNames changes.

444

Got'cha

src/declarativeimports/core/datamodel.cpp
379

The worst case is when you have different values [5, 6, 8] and [5, 8, 9, 10] it will add only 10 and internally replace other, is QML model expect that? So why i first make full remove, then full insert.

I investigate on testing this patch, it does not fix systray issue, to discard it?

anthonyfieroni abandoned this revision.Feb 22 2019, 5:29 AM