> On April 12, 2011, 10:55 p.m., David Edmundson wrote: > > account-filter-model.cpp, line 106 > > <http://git.reviewboard.kde.org/r/101108/diff/3/?file=14240#file14240line106> > > > > I'm really not convinced this works, if you're going to treat them like > > ints then there's no point overriding this method at all. > > > > I'm pretty confident (though I may be wrong) this will put offline > > users at the top, then online, away, busy. > > > > I think that we want to treat them as the enum values they are and > > need something more like: > > > > if (leftPresence == Tp::PresenceOnline) { > > return false; > > } > > if (rightPresence == Tp::PresenceOnline { > > return true; > > } > > > > if (leftPresence == Tp::PresenceAway) { > > return false; > > > > etc etc. > > > > Martin Klapetek wrote: > The purpose of lessThan(..) is to compare values to do the sort and it > seems like a good way to compare the uints (if they are ordered like > 0-online, 1-away...7-offline, otherwise it's no use). In your proposed way it > would have to compare all combinations of presences, no? Like if(leftPresence > == Online && rightPresence == Away) return true; if(leftPresence == Online && > rightPresence == Busy) return true; > > or am I getting this wrong? :) > > Also, Rémy, try to test with offline users shown. > > David Edmundson wrote: > Your statement contains a massive "if" there. > I'm pretty sure the statuses are in this order: > > http://telepathy.freedesktop.org/spec/Connection_Interface_Simple_Presence.html#Enum:Connection_Presence_Type > Which isn't a sensible order for listing. > > Also I don't think we need to do every possibly combo if it's written > like suggested, if either of them is "online" then that side 'wins' and you > return either true or false. Otherwise you continue onto the next presence, > if either of them are 'away' that way you only need each status once - though > it's still pretty lengthy. > > You may be right that it is easiest to turn the presence type into a rank > score, which does have 0 for online, 7 for offline, then compare those. I > would be happy with that. I just don't think it works as-is.
Ahh, now I'm getting your logic in the code above. Yes, this might be actually a way better solution...as always :P - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101108/#review2599 ----------------------------------------------------------- On April 12, 2011, 9:05 p.m., Rémy Greinhofer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101108/ > ----------------------------------------------------------- > > (Updated April 12, 2011, 9:05 p.m.) > > > Review request for Telepathy. > > > Summary > ------- > > Sorts the contacts by name offers the possibility to sort it also by status. > > > This addresses bug 246223. > http://bugs.kde.org/show_bug.cgi?id=246223 > > > Diffs > ----- > > account-filter-model.h d33231d > account-filter-model.cpp 7182362 > main-widget.h 18f97ee > main-widget.cpp a24ccf0 > main-widget.ui d000b9b > > Diff: http://git.reviewboard.kde.org/r/101108/diff > > > Testing > ------- > > 1. Open the contact list, the contacts are sorted by name. > 2. Click on the "Sort by onlineness button", the contacts are sorted by > onlineness and by name. > > > Thanks, > > Rémy > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
