> 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.
>
> Martin Klapetek wrote:
> So it should be one line down? I'm used to write comments above the
> lines, so that's why it might be off.
It could just be a personal style thing.
I'm used to writing
switch(foo) {
case A:
break;
case B: /*Fall Through*/
case C:
break;
}
so it's clear that B is meant to fall through and doesn't accidentally having a
missing break.
I'm not too fussed
> 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.
>
> Martin Klapetek wrote:
> 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.
I'm not convinced. You haven't changed the filter - you're still filtering out
offline accounts, therefore the filter parameters haven't changed.
When the model emits dataChanged() it should automatically call
filterAcceptsRow();
If you need something outside the model to monitor for changes and poke the
filter, it seems to defeat the point of it.
I'll try taking a look. If I can make it work without it, then we remove it.
Otherwise it clearly needs to stay.
- David
-----------------------------------------------------------
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
