[Pidgin] #5447: Buddy list sort by recent activity

[Pidgin]

#5447: Buddy list sort by recent activity
-------------------------------------+--------------------------------------
 Reporter:  cconnett                 |        Owner:  rlaager     
     Type:  patch                    |       Status:  new         
Milestone:                           |    Component:  pidgin (gtk)
  Version:  2.5.2                    |   Resolution:              
 Keywords:  buddylist sort activity  |  
-------------------------------------+--------------------------------------

Comment(by rlaager):

 I think this patch is acceptable to be committed after the following
 issues are resolved:

 1. cconnett, is there a real name and email address you'd like used to
 credit you? Bonus points if you add your own real name to the COPYRIGHT
 file (which is sorted by last name) in the patch and just let me know the
 email address to use. ;)

 2. The code in log.c uses the logsize_users hash table. This means that
 the activity counts will overwrite the "total size" counts, which are
 cached in that table. This is BAD.

 It should use a separate GHashTable. All that needs to be done there is we
 need a new global variable in log.c (right after logsize_users) and it
 needs to be initialized in purple_log_init(). You can pretty much copy-
 and-paste the existing initialization. The _hash, _equal, and _free_key
 functions can be left the same... there's no need to duplicate those. Then
 you can just use that new variable instead of logsize_users in the
 get_activity_score function you added.

 Bonus points if you add code (i.e. a call to g_hash_table_destroy) to
 uninitialize *both* hash tables in purple_log_unit(). We're currently
 leaking that memory, it seems. It's not a big deal, since we only uninit()
 on shutdown, but we like to free our memory on shutdown to make debugging
 with valgrind easier.

 3. The code in Pidgin calls purple_log_get_activity_score() once and then
 purple_log_get_total_size() later in the function. I'd imagine both should
 be changed to _get_activity_score(). I'd imagine the only reason this
 works is because of the problem I just addressed in #2.

 Bonus points for fixing code like this:
   PurpleBlistNode *n;
   ...
   log_size += purple_log_get_activity_score(PURPLE_LOG_IM,
 ((PurpleBuddy*)(n))->name, ((PurpleBuddy*)(n))->account);

   I'd like to see it look like this so it's easier to read:
   PurpleBlistNode *n;
   PurpleBuddy *buddy;
   ...
   buddy = (PurpleBuddy *)n;
   log_size += purple_log_get_activity_score(PURPLE_LOG_IM, buddy->name,
 buddy->account);

 cconnett, the more of these that you fix, the less there is for us to do
 to get this committed. Plus, I'd like to know that it still works well for
 you after these changes.

 Bonus points for adding @since 2.6.0 to the documentation for
 purple_log_get_activity_score in log.h.

 4. I'm saving the biggest issue for last. Is it really necessary to have a
 sort method for total log size *and* one for recent activity? For now, the
 patch as written is fine, but it's something I'd love to hear people's
 input on.

-- 
Ticket URL: <http://developer.pidgin.im/ticket/5447#comment:8>
Pidgin <http://pidgin.im>
Pidgin