#9630: Abstract Blist Saving

[Pidgin]

#9630: Abstract Blist Saving
------------------+---------------------------------------------------------
Reporter:  hanzz  |        Type:  patch    
  Status:  new    |   Component:  libpurple
 Version:  2.5.8  |    Keywords:          
------------------+---------------------------------------------------------
 Hi,
 some days ago, I've written patch (attached to this email) which allows
 applications, which use libpurple, to choose how they want to save/load
 blist data (currently stored in blist.xml). The goal is to make it easy to
 write for example mysql backend for blist storing without any change in
 libpurple code.

--
Ticket URL: <http://developer.pidgin.im/ticket/9630>
Pidgin <http://pidgin.im>
Pidgin
_______________________________________________
Tracker mailing list
Tracker@...
http://pidgin.im/cgi-bin/mailman/listinfo/tracker

[Pidgin]

#9630: Abstract Blist Saving
------------------------------------+---------------------------------------
 Reporter:  hanzz                   |        Owner:          
     Type:  patch                   |       Status:  new      
Milestone:  Patches Needing Review  |    Component:  libpurple
  Version:  2.5.8                   |   Resolution:          
 Keywords:                          |  
------------------------------------+---------------------------------------
Changes (by darkrain42):

  * milestone:  => Patches Needing Review


--
Ticket URL: <http://developer.pidgin.im/ticket/9630#comment:1>
Pidgin <http://pidgin.im>
Pidgin
_______________________________________________
Tracker mailing list
Tracker@...
http://pidgin.im/cgi-bin/mailman/listinfo/tracker

[Pidgin]

#9630: Abstract Blist Saving
-----------------------------------------+----------------------------------
 Reporter:  hanzz                        |        Owner:          
     Type:  patch                        |       Status:  new      
Milestone:  Patches Needing Improvement  |    Component:  libpurple
  Version:  2.5.8                        |   Resolution:          
 Keywords:                               |  
-----------------------------------------+----------------------------------
Changes (by darkrain42):

  * milestone:  Patches Needing Review => Patches Needing Improvement


Comment:

 This patch breaks compatibility with UIs, as they need to work with
 absolutely no changes; they can't be relied upon to call
 `purple_blist_schedule_save`.  The test for this is that both Pidgin and
 Finch need to work with no changes in `pidgin/` and `finch/`.

 A possible solution to this is to keep `purple_blist_schedule_save` in
 every place where you commented the UI needs to call it and add a new UI
 op (which `purple_blist_schedule_save` calls) that saves everything about
 an account (passed as a parameter), including the privacy data, and if
 `account == NULL`, it saves the entire file.

 I envision the function in blistsaving.c looking something like this:
 {{{
 static void
 foo_bar(PurpleAccount *account)
 {
 #if 0
         /* TODO: Optimization for other UIs, save data only for this
 account */
         if (account != NULL) {
                 /* Save it somewhere, yay! */
         } else
 #endif
                 purple_blist_sync();
 }
 }}}

 You need not actually write that optimization for the blist.xml format.

   * You added pidgin_blist_load() and pidgin_blist_schedule_save(), but
 they don't seem to be defined or used anywhere.
   * Rename 'delete_node' to 'remove_node' (since it's called when removing
 a node from the buddylist, not deleting/destorying the !PurpleBlistNode)

--
Ticket URL: <http://developer.pidgin.im/ticket/9630#comment:2>
Pidgin <http://pidgin.im>
Pidgin
_______________________________________________
Tracker mailing list
Tracker@...
http://pidgin.im/cgi-bin/mailman/listinfo/tracker

[Pidgin]

#9630: Abstract Blist Saving
-----------------------------------------+----------------------------------
 Reporter:  hanzz                        |        Owner:          
     Type:  patch                        |       Status:  new      
Milestone:  Patches Needing Improvement  |    Component:  libpurple
  Version:  2.5.8                        |   Resolution:          
 Keywords:                               |  
-----------------------------------------+----------------------------------

Comment(by hanzz):

 Thanks for your opinion. I think I've fixed problems you mentioned. I've
 added save_account ui_op to PurpleAccountUiOps which calls
 purple_blist_schedule_save. It's called from account.c and also from
 privacy.c. This version of patch (abstract_blist_save_2.diff) is
 transparent for current UIs.

 Thanks for your work and your time.

--
Ticket URL: <http://developer.pidgin.im/ticket/9630#comment:3>
Pidgin <http://pidgin.im>
Pidgin
_______________________________________________
Tracker mailing list
Tracker@...
http://pidgin.im/cgi-bin/mailman/listinfo/tracker

[Pidgin]

#9630: Abstract Blist Saving
------------------------------------+---------------------------------------
 Reporter:  hanzz                   |        Owner:          
     Type:  patch                   |       Status:  new      
Milestone:  Patches Needing Review  |    Component:  libpurple
  Version:  2.5.8                   |   Resolution:          
 Keywords:                          |  
------------------------------------+---------------------------------------
Changes (by darkrain42):

  * milestone:  Patches Needing Improvement => Patches Needing Review


--
Ticket URL: <http://developer.pidgin.im/ticket/9630#comment:4>
Pidgin <http://pidgin.im>
Pidgin
_______________________________________________
Tracker mailing list
Tracker@...
http://pidgin.im/cgi-bin/mailman/listinfo/tracker

[Pidgin]

#9630: Abstract Blist Saving
------------------------------------+---------------------------------------
 Reporter:  hanzz                   |        Owner:          
     Type:  patch                   |       Status:  new      
Milestone:  Patches Needing Review  |    Component:  libpurple
  Version:  2.5.8                   |   Resolution:          
 Keywords:                          |  
------------------------------------+---------------------------------------

Comment(by darkrain42):

 Could you please review the attached patch and make sure it looks good
 (and will work for you)?

 I know we initially recommended you split the functionality out into a
 separate file, which I think turned out to be a poor suggestion (sorry!).
 I've consolidated everything into minimal changes as much as I can because
 there are two other SoC projects (Sulabh's privacy workings and Eric's
 GObjectification) that are modifying a bunch of the same code, and I'm
 going to be yelled at if this patch makes merging with their work too
 difficult.

 Anyway, I've only lightly tested this, so please also make sure I didn't
 muck up the saving callbacks.

--
Ticket URL: <http://developer.pidgin.im/ticket/9630#comment:5>
Pidgin <http://pidgin.im>
Pidgin
_______________________________________________
Tracker mailing list
Tracker@...
http://pidgin.im/cgi-bin/mailman/listinfo/tracker

[Pidgin]

#9630: Abstract Blist Saving
------------------------------------+---------------------------------------
 Reporter:  hanzz                   |        Owner:          
     Type:  patch                   |       Status:  new      
Milestone:  Patches Needing Review  |    Component:  libpurple
  Version:  2.5.8                   |   Resolution:          
 Keywords:                          |  
------------------------------------+---------------------------------------

Comment(by hanzz):

 Your patch looks much more better than my :). I've tested it in Pidgin and
 it works for me. I think I've tested all changes we made (including
 privacy and setting local alias for account).

--
Ticket URL: <http://developer.pidgin.im/ticket/9630#comment:6>
Pidgin <http://pidgin.im>
Pidgin
_______________________________________________
Tracker mailing list
Tracker@...
http://pidgin.im/cgi-bin/mailman/listinfo/tracker

[Pidgin]

#9630: Abstract Blist Saving
-------------------+--------------------------------------------------------
 Reporter:  hanzz  |        Owner:          
     Type:  patch  |       Status:  closed  
Milestone:  2.6.0  |    Component:  libpurple
  Version:  2.5.8  |   Resolution:  fixed    
 Keywords:         |  
-------------------+--------------------------------------------------------
Changes (by hanzz@...):

  * status:  new => closed
  * resolution:  => fixed
  * milestone:  Patches Needing Review => 2.6.0


Comment:

 (In [9de96edcea00fc0b41b3668bdd9072738cbf149d]):[[BR]]
 Add blist ui-ops to overload the saving of data to blist.xml. Closes
 #9630.

 Patch from Jan "HanzZ" Kaluza with some changes by me so that it's
 easier to merge with Sulabh and Eric's SoC projects (mostly so grim
 wouldn't yell at me).  Anyway, any bugs introduced by me (darkrain42).

--
Ticket URL: <http://developer.pidgin.im/ticket/9630#comment:7>
Pidgin <http://pidgin.im>
Pidgin
_______________________________________________
Tracker mailing list
Tracker@...
http://pidgin.im/cgi-bin/mailman/listinfo/tracker

[Pidgin]

#9630: Abstract Blist Saving
-------------------+--------------------------------------------------------
 Reporter:  hanzz  |        Owner:          
     Type:  patch  |       Status:  closed  
Milestone:  2.6.0  |    Component:  libpurple
  Version:  2.5.8  |   Resolution:  fixed    
 Keywords:         |  
-------------------+--------------------------------------------------------

Comment(by hanzz):

 thanks for commit :)

--
Ticket URL: <http://developer.pidgin.im/ticket/9630#comment:8>
Pidgin <http://pidgin.im>
Pidgin
_______________________________________________
Tracker mailing list
Tracker@...
http://pidgin.im/cgi-bin/mailman/listinfo/tracker