Work in Progress: http://cgit.collabora.com/git/user/danni/empathy.git/log/?h=goa-mc-plugin

+  GHashTable *accounts; /* string -> GoaAccount */
Please document the semantic of the string.


GTalk account should have fallback-servers and extra-certificate-identities
set, see empathy_protocol_chooser_create_account_settings().
We should probably set require-encryption as well.

See also bug 653269 about exporting more information for the Chat accounts. I'm thinking that there shouldn't be any special-casing in the plugin, and the "Chat" account info would be exported.

(In reply to comment #2)
> GTalk account should have fallback-servers and extra-certificate-identities
> set, see empathy_protocol_chooser_create_account_settings().
> We should probably set require-encryption as well.

We just added more parameters to GTalk accounts:
http://git.gnome.org/browse/empathy/commit/?id=53a110fcf6a8b697f8d27a8c34b0cccf4db04b24
http://git.gnome.org/browse/empathy/commit/?id=60d6f85f560b4c958085c8f759f519363a05e779

(In reply to comment #3)
> See also bug 653269 about exporting more information for the Chat accounts. I'm
> thinking that there shouldn't be any special-casing in the plugin, and the
> "Chat" account info would be exported.

There's a whole bunch of IM/Telepathy specific stuff that Empathy already knows about, we can move it into GOA, but for now it's not important and has been hardcoded using Account.ProviderType.

For reference, the parameters I've included are:

account - e.g. danielle@nyan.cat -- comes from GOA, currently using Account.Identity [1]
server - e.g. talk.google.com -- hardcoded for some services
fallback-servers -- hardcoded for some services
extra-certificate-identities -- hardcoded for some services
require-encryption -- hardcoded for some services

[1] I suppose this may not be correct for all services? It's correct for Google though.

(In reply to comment #4)
> > GTalk account should have fallback-servers and extra-certificate-identities
> > set, see empathy_protocol_chooser_create_account_settings().
> > We should probably set require-encryption as well.
> 
> We just added more parameters to GTalk accounts:

Updated for all of these points.

I'm trying to work out how to determine if the 'Chat' switch is turned on. The account doesn't seem to expose o.g.OA.Chat.

I ressurected this branch and hooked the 'Chat' switch: http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=goa-mc-plugin-652543

How should we store things in mcp_account_manager_goa_set()? MC asks us to store HasBeenOnline, Nickname and NormalizedName, should we request properties to GOA guys for this?
We should probably be able to store AvatarMime and avatar_token as well.

This looks pretty good, except I was hoping for d71691f82c6c64af2a003743177722de9768a841, that rather than not being able to set Enabled, we should call whatever API lets you toggle the Chat switch (I assume the configuration panel must do something).

> How should we store things in mcp_account_manager_goa_set()? MC asks us to
> store HasBeenOnline, Nickname and NormalizedName, should we request properties
> to GOA guys for this?

Returning FALSE (iirc) then tries to call set() on the next storage plugin (i.e. the config file). I think we may as well store these properties in accounts.cfg, since they're meaningless to anything that isn't Telepathy.

I opened bug #653269 about the storage of extra paramters. Ideally we should fix this before merging the plugin.

(In reply to comment #8)
> This looks pretty good, except I was hoping for
> d71691f82c6c64af2a003743177722de9768a841, that rather than not being able to
> set Enabled, we should call whatever API lets you toggle the Chat switch (I
> assume the configuration panel must do something).

I opened bug #657905 for this, but I don't think that's a blocker for getting
this plugin merged.

(In reply to comment #9)
> I opened bug #653269 about the storage of extra paramters. Ideally we should
> fix this before merging the plugin.

I meant 'commented'; the bug was already there.

(In reply to comment #7)
> I ressurected this branch and hooked the 'Chat' switch:
> http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=goa-mc-plugin-652543

I pushed an extra commit cleaning the code a bit.

I just added support for storing the extra parameters to a key file as that seems to way to go (see bug #653269).

+      if (keys == NULL)
+        n = 0;

Is this required? Random aside, I prefer nkeys to n. Tells you what the variables contains.

+      keys = g_key_file_get_keys (priv->store, acct, &n, NULL);

I can't see where this is free'd.

(In reply to comment #14)
> +      if (keys == NULL)
> +        n = 0;
> 
> Is this required? Random aside, I prefer nkeys to n. Tells you what the
> variables contains.

Yes, if there is an error. I renamed the variable.

> +      keys = g_key_file_get_keys (priv->store, acct, &n, NULL);
> 
> I can't see where this is free'd.

Fixed.

(In reply to comment #15)

> Yes, if there is an error. I renamed the variable.

I would have thought GLib would 0 this value on error. If not that's a bug we should file in GLib.

Looks good to me. Ship it!

Merged to master; thanks !

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.

(In reply to comment #16)
> (In reply to comment #15)
> 
> > Yes, if there is an error. I renamed the variable.
> 
> I would have thought GLib would 0 this value on error. If not that's a bug we
> should file in GLib.

Bug #658315