----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112437/#review39329 -----------------------------------------------------------
logviewer/log-viewer.cpp <http://git.reviewboard.kde.org/r/112437/#comment28996> Why are you doing this? This will make parsing the original model a lot lot harder than it needs to be. You have your own custom grouping of items anyway (see below comment), we're grouping items - then merging contacts with entity model - then grouping it again. logviewer/person-entity-merge-model.cpp <http://git.reviewboard.kde.org/r/112437/#comment28995> I should have spotted this on the first review. Why are you doing your own custom group grouping? We have a working proxy model for doing this without reinventing the wheel. That one supports multiple groups properly too. This one doesn't. It's very hard to do properly which is why I only want to do it once. logviewer/person-entity-merge-model.cpp <http://git.reviewboard.kde.org/r/112437/#comment28994> Why has this been added in the latest revision. Is this due to a lack of a sourceModelInitialised signal? We could have just added that in. I think I even hinted at that. A 5 line change vs a 300 line one. I know you're seriously pro at programming, but you don't need to go deliberately out of your way to make life difficult for yourself. The only change I was expecting was PeronsModel model to ContactsModel (without grouping) and a few role enums. What I want changed: - PersonsEntityMergeModel uses the ungrouped ContactsModel - Then use AbstractGroupingModel on the PersonsEntityMergeModel I reckon this will halve the size of the code.. and the grouping will work better. - David Edmundson On Sept. 4, 2013, 7:01 a.m., Dan Vrátil wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112437/ > ----------------------------------------------------------- > > (Updated Sept. 4, 2013, 7:01 a.m.) > > > Review request for Telepathy. > > > Description > ------- > > I have finished porting our beloved Log Viewer to KPeople and would like to > merge it for 0.7. > > The idea is that we take EntityModel (populated by entities from logger) and > KPeople model and merge them into one model. This is done by an ugly beast > called PersonEntityMergeModel. There is still some space for improvements > (mainly performance), but generally it works and it is reasonably fast with > cca 100 entities/contacts. I have also stolen code for tree view delegates > from contact list, so that we are more consistent across KTp components. > > Contacts are grouped by their group membership (retrieved from KPeople). When > you select a Persona, dates from all subcontacts are loaded into the dates > view and the Persona is expanded so that you can selected individual > subcontacts. > > The date picker has been replaced by a tree view with list of dates grouped > by months. If there are logs from multiple subcontacts for one date, the date > can be further expanded and you can pick specific subcontact. > > When KPeople is disabled at build time or Nepomuk is not running, we fallback > to current logviewer behavior, i.e. cntacts are grouped by accounts and > display names are fetched from Telepathy (so it works only for accounts that > are online). > > As a new feature, the logger now has settings page which contains two tabs: > one for theme configuration and one specifically for logviewer. So far it > only has one option - you can pick whether you want to sort messages in logs > from newest to oldest or vice versa. It's not related to KPeople, but I > started working on it in the kpeople branch (don't ask me why), so there it > is. > > > Diffs > ----- > > CMakeLists.txt 8083bc2 > logviewer/CMakeLists.txt 300ba8f > logviewer/config/CMakeLists.txt PRE-CREATION > logviewer/config/behavior-config.h PRE-CREATION > logviewer/config/behavior-config.cpp PRE-CREATION > logviewer/config/behavior-config.ui PRE-CREATION > logviewer/config/kcm_ktp_logviewer_behavior.desktop PRE-CREATION > logviewer/conversation-date-picker.h 6e1bd60 > logviewer/conversation-date-picker.cpp b8db972 > logviewer/dates-model.h PRE-CREATION > logviewer/dates-model.cpp PRE-CREATION > logviewer/dates-view-delegate.h PRE-CREATION > logviewer/dates-view-delegate.cpp PRE-CREATION > logviewer/entity-filter-model.h PRE-CREATION > logviewer/entity-filter-model.cpp PRE-CREATION > logviewer/entity-model-item.h d7cb8ed > logviewer/entity-model-item.cpp 4e2189d > logviewer/entity-model.h f9bf293 > logviewer/entity-model.cpp 164f263 > logviewer/entity-proxy-model.h 837f4af > logviewer/entity-proxy-model.cpp b57296b > logviewer/entity-view-delegate.h PRE-CREATION > logviewer/entity-view-delegate.cpp PRE-CREATION > logviewer/entity-view.cpp 77fefc0 > logviewer/log-viewer.h 5820a88 > logviewer/log-viewer.cpp 909777d > logviewer/log-viewer.rc 82df425 > logviewer/log-viewer.ui a76b34a > logviewer/message-view.h 4e2e2bb > logviewer/message-view.cpp b037900 > logviewer/person-entity-merge-model.h PRE-CREATION > logviewer/person-entity-merge-model.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/112437/diff/ > > > Testing > ------- > > Browsed some logs, seems to work :) > > > Thanks, > > Dan Vrátil > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
