GNOME Bugzilla – Bug 680302
Move model logic out of EmpathyRosterView
Last modified: 2012-08-07 14:48:21 UTC
.
Created attachment 219421 [details] [review] empathy-roster-model-manager: Pass an EmpathyIndividualManager to EmpathyRosterModelManager
Created attachment 219423 [details] [review] empathy-roster-model-manager.c: implement empathy_roster_model_manager_get_individuals () https://bugzilla.gnome.org/show_bug.cgi?id=679868
Created attachment 219424 [details] [review] empathy-roster-view: start using empathy-roster-model
Created attachment 219425 [details] [review] empathy-roster-model-manager: deal with members-changed signals empathy-roster-view: use empathy-roster-model-manager signals
Created attachment 219426 [details] [review] empathy-roster-model, empathy-roster-model-manager: Now deals with groups-changed signal empathy-roster-view.c: now uses roster-model's groups-changed signal
Created attachment 219427 [details] [review] empathy-roster-model: New virtual method _get_groups_for_individual empathy-roster-model-manager: implemented _get_groups_for_individual empathy-roster-view.c: using _get_groups_for_individual
Created attachment 219428 [details] [review] empathy-roster-model-manager: added xmpp-local contacts support empathy-roster-view: removed xmpp-local contacts support
Created attachment 219429 [details] [review] empathy-roster-model-manager: checks if individual is in top_group empathy-roster-view: removed this check
Created attachment 219430 [details] [review] empathy-roster-model-manager: now catches and treats notify::top-individuals empathy-roster-model: new function _notify_top_individuals empathy-roster-view: now catches the model's notify::top-individuals instead of the manager's
Created attachment 219431 [details] [review] empathy-roster-model and empathy-roster-model-manager: now deals with notify::favourites_changed signal empathy-roster-view: now catches favourites_changed from model instead of manager
Created attachment 219432 [details] [review] empathy-roster-model and empathy-roster-model-manager: new function _get_top_individuals empathy-roster-view: now uses empathy_roster_model_get_top_individuals instead of empathy_individual_manager_get_top_individuals
Created attachment 219433 [details] [review] empathy-roster-view: removed the manager, relying only on the model empathy-roster-window, empathy-nautilus-sendto and test-empathy-roster-view: adapted the roster_view_new call
Created attachment 219434 [details] [review] Moved contact_is_favourite from roster-view to the model
Created attachment 219435 [details] [review] Moved contact_in_top from roster-view to roster-model
Review of attachment 219421 [details] [review]: ++
Review of attachment 219423 [details] [review]: ++
Review of attachment 219424 [details] [review]: ++
Review of attachment 219425 [details] [review]: ++
Review of attachment 219426 [details] [review]: ::: libempathy-gtk/empathy-roster-model-manager.c @@ +87,3 @@ + empathy_roster_model_fire_groups_changed (EMPATHY_ROSTER_MODEL (self), + individual, + gboolean is_member, No need to split on multi lines. ::: libempathy-gtk/empathy-roster-view.c @@ +1459,3 @@ g_return_val_if_fail (EMPATHY_IS_INDIVIDUAL_MANAGER (manager), NULL); g_return_val_if_fail (EMPATHY_IS_ROSTER_MODEL (model), NULL); + What has been changed here?
Attachment 219421 [details] pushed as 2a6cc7b - empathy-roster-model-manager: Pass an EmpathyIndividualManager to EmpathyRosterModelManager Attachment 219423 [details] pushed as 3ba0b11 - empathy-roster-model-manager.c: implement empathy_roster_model_manager_get_individuals () Attachment 219424 [details] pushed as 8751131 - empathy-roster-view: start using empathy-roster-model Attachment 219425 [details] pushed as 18ded7a - empathy-roster-model-manager: deal with members-changed signals empathy-roster-view: use empathy-roster-model-manager signals
Review of attachment 219427 [details] [review]: ::: libempathy-gtk/empathy-roster-model.c @@ +105,3 @@ + +GList * +empathy_roster_model_get_groups_for_individual (EmpathyRosterModel *self, It would be nice to document (in a new commit) the transfer type of those functions. Looking at your implementation this one seems to be transfer container, so add a something like that before the function definition: /* Returns: (transfer container) */ Ditto for the other public API.
Review of attachment 219428 [details] [review]: ::: libempathy-gtk/empathy-roster-model-manager.c @@ +54,3 @@ */ +#define PEOPLE_NEARBY _("People Nearby") You can now use EMPATHY_ROSTER_VIEW_GROUP_PEOPLE_NEARBY
Review of attachment 219429 [details] [review]: commit message should be something like "start moving top individuals logic from roster-view to roster-model". ::: libempathy-gtk/empathy-roster-model-manager.c @@ +54,3 @@ */ +#define TOP_GROUP _("Top Contacts") Same, use the EMPATHY_ROSTER_VIEW_GROUP_UNGROUPED. Actually, this should probably be renamed EMPATHY_ROSTER_MODEL_GROUP_* and moved to roster-model.h
Created attachment 219466 [details] [review] Renamed #define'd groups names to EMPATHY_ROSTER_MODEL_GROUP_*
Created attachment 219467 [details] [review] Moved roster_contacts from roster-view to the model empathy-roster-model and empathy-roster-model-manager: Added roster_contacts and methods to insert, remove, remove_all and lookup for an individual empathy-roster-view: removed roster_contact and modifie
Created attachment 219468 [details] [review] Moved roster_groups from view to model empathy-roster-model and empathy-roster-model-manager: added roster_groups and methods to insert, remove_all and lookup for a group empathy-roster-view: removed roster_groups and modified roster_groups usages to use
Review of attachment 219466 [details] [review]: You should remove the #define in roster-view.h
Created attachment 219480 [details] [review] Renamed #define'd groups names to EMPATHY_ROSTER_MODEL_GROUP_*
Attachment 219426 [details] pushed as 5e102f0 - empathy-roster-model, empathy-roster-model-manager: Now deals with groups-changed signal empathy-roster-view.c: now uses roster-model's groups-changed signal Attachment 219427 [details] pushed as f7200ee - empathy-roster-model: New virtual method _get_groups_for_individual empathy-roster-model-manager: implemented _get_groups_for_individual empathy-roster-view.c: using _get_groups_for_individual Attachment 219428 [details] pushed as f253f50 - empathy-roster-model-manager: added xmpp-local contacts support empathy-roster-view: removed xmpp-local contacts support
Created attachment 219570 [details] [review] Documented public functions in empathy-roster-model
Created attachment 219593 [details] [review] Documented public functions in empathy-roster-model
Review of attachment 219433 [details] [review]: ++
Review of attachment 219434 [details] [review]: I don't understand this. Isn't 'favorite' just an implementation details of being in the 'top group'? Do we really need to expose this?
(In reply to comment #33) > Review of attachment 219434 [details] [review]: > > I don't understand this. Isn't 'favorite' just an implementation details of > being in the 'top group'? Do we really need to expose this? I considered that it was the model's role to decide if a contact is favourite or not, but I may have been wrong.
I get this when running the test app with your branch: (lt-test-empathy-roster-view:31497): GLib-GObject-WARNING **: invalid cast from `GParamPointer' to `EmpathyRosterModel' Program received signal SIGTRAP, Trace/breakpoint trap. 0x00007ffff3e441e9 in g_logv (log_domain=0x7ffff4178350 "GLib-GObject", log_level=G_LOG_LEVEL_WARNING, format=0x7ffff4179ac0 "invalid cast from `%s' to `%s'", args1=0x7fffffffbac8) at gmessages.c:758 758 G_BREAKPOINT (); (gdb) bt full
+ Trace 230620
I just pushed Laurent's latest patches; closing. This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.