Bug 28469

Hey, thanks for the patch! I had an outstanding branch I forgot to merge that refactored how the parameter mappings worked, so I've merged that branch and am rebasing your patch onto it now. I'll attach it here for you to take a look when I'm done.

So I've got a git branch at <http://git.collabora.co.uk/?p=user/wjt/telepathy-haze;a=shortlog;h=refs/heads/usersplit> in which I've rebased your branch onto master-as-of-an-hour-ago. ☺

I made a couple, changes to your patches in the process — removed tabs, used g_list_length (), misc. coding style stuff (I'm aware Haze is not exactly the epitome of style consistency; I'm vaguely hoping it'll move towards Telepathy style), fixed leaking 'username' in haze_connection_create_account() if the account already exists — but (I hope) nothing functional. It seems to still work, but I don't actually have a SIPE server to connect to. Any chance you could test it against your SIPE server? If it flies, I'll merge the branch.

Thanks again!

Thanks, I cloned the git and your rebase seems to work in my quick test.

BTW: this is not a SIPE only issue. A quick scan through Pidgin protocol plugin source code reveals:

bonjour/bonjour.c:      PurpleAccountUserSplit *split;
irc/irc.c:      PurpleAccountUserSplit *split;
jabber/libxmpp.c:       PurpleAccountUserSplit *split;
null/nullprpl.c:  PurpleAccountUserSplit *split = purple_account_user_split_new(
silc10/silc.c:  PurpleAccountUserSplit *split;
silc/silc.c:    PurpleAccountUserSplit *split;
simple/simple.c:        PurpleAccountUserSplit *split;

So these changes might help those protocols too...


I realized that making them mandatory parameters is not correct, i.e. it will not work for users that need to leave one of the parameters empty. I'll submit another patch based on your re-base once I'm done.


As for the style: have you considered adding at least some hints for Emacs to end of each file? Then the Emacs C mode will make sure that the code is entered using the correct style. Here is the settings we use for pidgin-sipe code:

/*
  Local Variables:
  mode: c
  c-file-style: "bsd"
  indent-tabs-mode: t
  tab-width: 8
  End:
*/

(In reply to comment #3)
> Thanks, I cloned the git and your rebase seems to work in my quick test.

Great!

> BTW: this is not a SIPE only issue. A quick scan through Pidgin protocol plugin
> source code reveals:
> 
> bonjour/bonjour.c:      PurpleAccountUserSplit *split;
> irc/irc.c:      PurpleAccountUserSplit *split;
> jabber/libxmpp.c:       PurpleAccountUserSplit *split;
> null/nullprpl.c:  PurpleAccountUserSplit *split =
> purple_account_user_split_new(
> silc10/silc.c:  PurpleAccountUserSplit *split;
> silc/silc.c:    PurpleAccountUserSplit *split;
> simple/simple.c:        PurpleAccountUserSplit *split;
> 
> So these changes might help those protocols too...

Yeah, that's true. (That said... I don't really expect anyone to use the first three through Haze (we have native CMs for Bonjour, IRC and Jabber, which work better), although IRC is bug 14213 and Jabber is bug 14212, and I had a quick stab at making them work with Jabber yesterday. Simple: well, we have a SIP CM. And does anyone use SILC? ☺)

> I realized that making them mandatory parameters is not correct, i.e. it will
> not work for users that need to leave one of the parameters empty. I'll submit
> another patch based on your re-base once I'm done.

Great, thanks.

> As for the style: have you considered adding at least some hints for Emacs to
> end of each file? Then the Emacs C mode will make sure that the code is entered
> using the correct style.

Yep, that's a good idea. I think the modelines for emacs and Vim should follow <http://telepathy.freedesktop.org/wiki/Style>, since that's the direction I'd like Haze to head.

Created attachment 36204 [details] [review]
connection(-manager): PurpleAccountUserSplits should be optional parameters

Works OK with Empathy. I'm trying to test the whole shebang on my N900 ASAP.

I've backported the patch series ontop of telepathy-haze_0.3.4-1maemo2 and installed the resulting .deb file on my N900. Works like a charm, i.e. I can now set up my Office Communicator account completely from the Account Setup UI.

Great! Merged; will be in haze 0.3.5. Thanks!