[GSoC] GObjectification Summary

[Ankit Vani]

Hi

On 24 October 2013 13:57, Mark Doliner <mark at kingant.net> wrote:
> - Do non-privileged trac users have access to
> https://developer.pidgin.im/newticket? We previously linked to
> simpleticket. In doc/finch.1.in and doc/pidgin.1.in

simpleticket redirects to newticket, which asks users to log in.

> - pidgin_permit_added_removed() and pidgin_deny_added_removed() should
> either be listed in a header file or changed to only be referenced
> from a single file and made static. It looks like they're used as a
> ui_op... maybe they could be accessed that way?

I did this after making them static:

 void
 pidgin_privacy_init(void)
 {
+ PurpleAccountUiOps *ops = pidgin_accounts_get_ui_ops();
+
+ ops->permit_added = ops->permit_removed = pidgin_permit_added_removed;
+ ops->deny_added = ops->deny_removed = pidgin_deny_added_removed;
 }

> - Not important, but we should fix the name of the #ifndef for
> libpurple/finch/gntxfer.h libpurple/protocols/gg/blist.h
> pidgin/gtkxfer.h
> - Need to address FIXME comment in libpurple/protocols/jabber/jutil.c?

Done.

> - Need to address FIXME comment at the bottom of
> libpurple/protocols/jabber/parser.c?

I added purple_connection_get_error_info(), which returns a
PurpleConnectionErrorInfo instance if the connection has an error, otherwise
NULL -- thus providing an API to retrieve connection error and its
description after an error has occured. This has been used in place of the
!js->gc->disconnect_timeout check in parser.c.

It made more sense than adding a function that returned a boolean result of
(disconnect_timeout > 0), which I doubt would have been used anywhere else.

> - A single file-wide static conn_close_timeout doesn't handle multiple
> jabber accounts correctly in libpurple/protocols/jabber/jabber.c
> because there is only one copy of the static var being used by
> multiple accounts. Though, I think this code is no worse than it was
> before. In any case we should at least add a TODO comment to fix this.

Ohh I missed that. Moved conn_close_timeout to the JabberStream struct.
So now we have like
js->conn_close_timeout = purple_timeout_add(0, conn_close_cb, js);

> - Need to do anything about the #if 0'ed code in
> libpurple/plugins/tcl/tcl_signals.c?
> - Need to do anything about the #if 0'ed code in
> libpurple/plugins/perl/perl-common.c purple_perl_sv_from_vargs()?
> - Need to do anything about the #if 0'ed code in
> libpurple/plugins/perl/perl-handlers.c perl_signal_cb()?

I don't think anything needs to be done about this code, but I'm not 100%
sure about it, since I haven't really messed around with the loader stuff.

Those #if 0's are mostly due to removal of PurpleValue and the concept of
'outgoing' values (instead of which we just use pointers now).

When merging soc.2013.gobjectification.plugins in the future, these files
will become obsolete (loaders are handled by GPlugin and bindings handled by
introspection).

> - Our example plugins shouldn't #include <internal.h>, right? I guess
> it doesn't matter if the code is in our tree. Even better would be for
> these things to live in a separate repo or package outside our tree,
> and for us to distribute simple autoconf and automake files. (Some
> files that include internal.h that maybe shouldn't:
> libpurple/plugins/dbus-example.c libpurple/plugins/debug_example.c
> libpurple/plugins/helloworld.c libpurple/plugins/notify_example.c)

Yeah I agree that internal.h should not be included in these plugins.
However, internal.h is used for the translation stuff and stuff like
DISPLAY_VERSION and PURPLE_WEBSITE.

What do you suggest?

> In an earlier email you said:
> "These entities are temporarily GBoxed, with a TODO mentioning so in their
> purple_entity_get_type() function's documentation, and should eventually be
> converted to GObjects:
>  GBoxed
>   +----PurpleCertificatePool
>   +----PurpleSavedStatus
>   +----PurpleStoredImage
>   +----PurpleLog
>   +----PurpleXfer
>   +----PurplePlugin"
>
> Do we need to do anything about these before merging? Or after merging?

These are the components that should really be GObjects, but haven't been
GObjectified yet since they're not an immediate concern. The only reason they
are boxed is so that we have a GType for them for use in loaders, as
PurpleType no longer exists. I suppose this can be dealt with after merging.

However, of these listed, PurpleXfer is already a GObject now. And
PurplePlugin stuff is taken care of by the new plugin API in
soc.2013.gobjectification.plugins.

Ankit