> On March 25, 2011, 1:30 a.m., David Edmundson wrote:
> > account-filter-model.cpp, line 95
> > <http://git.reviewboard.kde.org/r/100838/diff/4/?file=12728#file12728line95>
> >
> >     I don't understand what this method is for.

This is for triggering the invalidateFilter() from "outside", because 
invalidateFilter() is protected. Since the proxy now filters also the empty 
(disconnected) accounts, it needs to trigger the invalidateFilter() once the 
account is disconnected to filter the empty account group out. That's what this 
method does.


> On March 25, 2011, 1:30 a.m., David Edmundson wrote:
> > main-widget.cpp, line 377
> > <http://git.reviewboard.kde.org/r/100838/diff/4/?file=12746#file12746line377>
> >
> >     If you need to manually invalidate your filter you have something wrong 
> > with either your model or the filter code.

This is the case I described above - this is a slot called when the account 
connection state changes. When it gets disconnected, the filtering needs to be 
invalidated to 'refilter' the items, therefore this is called here. See the 
docs for QSortFilterProxyModel::invalidateFilter() - "This function should be 
called if you are implementing custom filtering (e.g. filterAcceptsRow()), and 
your filter parameters have changed." - This is exactly the case.


> On March 25, 2011, 1:30 a.m., David Edmundson wrote:
> > main-widget.cpp, line 379
> > <http://git.reviewboard.kde.org/r/100838/diff/4/?file=12746#file12746line379>
> >
> >     I think the comment "fall Through" is in the wrong place.
> >     
> >     I initially read this as expecting ConnectionStatusDisconnected to fall 
> > through, which clearly it doesn't.

So it should be one line down? I'm used to write comments above the lines, so 
that's why it might be off.


> On March 25, 2011, 1:30 a.m., David Edmundson wrote:
> > main-widget.cpp, line 528
> > <http://git.reviewboard.kde.org/r/100838/diff/4/?file=12746#file12746line528>
> >
> >     This is a bit confusing.
> >     
> >     1) do you really need two casts? why not 
> > qVariantValue<ContactModelItem*>(..)
> >     
> >     2) you don't need to model->data(index, role) you can just:
> >     
> >     index.data(role);
> >

Actually I'm not sure if it is really needed (1), but since the model returns 
QVariant<QObject*>, I tried to directly cast it to ContactModelItem*, but this 
wouldn't compile for me. I'll try to play a little with it nevertheless.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100838/#review2132
-----------------------------------------------------------


On March 23, 2011, 9:32 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100838/
> -----------------------------------------------------------
> 
> (Updated March 23, 2011, 9:32 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> This is the work that begun in my clone repo and has matured enough now to be 
> merged back to 'upstream', where the work should continue now.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 2c62ea10556cdba38d1bca0fe63603df97414336 
>   account-button.h PRE-CREATION 
>   account-button.cpp PRE-CREATION 
>   account-filter-model.h PRE-CREATION 
>   account-filter-model.cpp PRE-CREATION 
>   accounts-model-item.h PRE-CREATION 
>   accounts-model-item.cpp PRE-CREATION 
>   accounts-model.h PRE-CREATION 
>   accounts-model.cpp PRE-CREATION 
>   contact-delegate-overlay.h PRE-CREATION 
>   contact-delegate-overlay.cpp PRE-CREATION 
>   contact-delegate.h PRE-CREATION 
>   contact-delegate.cpp PRE-CREATION 
>   contact-model-item.h PRE-CREATION 
>   contact-model-item.cpp PRE-CREATION 
>   contact-overlays.h PRE-CREATION 
>   contact-overlays.cpp PRE-CREATION 
>   contact-view-hover-button.h PRE-CREATION 
>   contact-view-hover-button.cpp PRE-CREATION 
>   filter-bar.h PRE-CREATION 
>   filter-bar.cpp PRE-CREATION 
>   main-widget.h aed6f7c8336321bc8a6ffb3b9af6b1d493f85790 
>   main-widget.cpp 6146b62eec17b54be63b594200613931673ac5fe 
>   main-widget.ui 5b0d5aaaf3a4e2f49eb030b98b15fcae3a86170c 
>   main.cpp bba0e4175e8853afe603f26ea4707f4974192d0f 
>   ontologies/CMakeLists.txt 3e0bbe8c634908f689dcd360409aeddcc6fc0d23 
>   tree-node.h PRE-CREATION 
>   tree-node.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/100838/diff
> 
> 
> Testing
> -------
> 
> We did a lot of thourough testing on #kde-telepathy, also some people emailed 
> their reports which all has been fixed.
> 
> 
> Thanks,
> 
> Martin
> 
>

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to