Proposal for an extended callbacks field

[Etan Reisner]

On Thu, Jul 26, 2007 at 03:44:56AM +0200, Andreas Monitzer wrote:
> On Jul 26, 2007, at 00:01, Ethan Blanton wrote:

<snip>

> It's not a single showstopper I could point my finger at, but the
> issue crept up every now and then, and it really gets annoying over
> time.

It had no chance of becoming anything but annoying since you didn't bring
it up when you ran into it except that very first time. As I've said
before, discussing things is almost always better than not discussing
them. Especially when you think you weren't understood clearly the first
time or that the original decision was the wrong one.

> The functions that are already implemented and have to be used from
> the application but are not in the prpl struct right now are:
>
> purple_account_set_register_callback

As I said in my previous email this isn't a prpl_info member and does not
need to be, having the prpl call known functions in the core which then
handle calling ui_ops and/or emitting signals is a perfectly good way to
handle this and lets any ui or plugin handle it however they want to.
Whereas enabling only a single callback function does not.

> jabber_register_gateway

Account Action

> jabber_adhoc_execute

Account Action and blist-node-extended-menu action for most things, if we
need a service discovery browser or an enter-a-jid-to-disco action those
can come later and likely be done with the existing mechanisms.

> jabber_ping_jid

Account Action and/or blist-node-extended-menu action.

> jabber_user_search

User search already exists for XMPP or do you mean something other than
the JUD based searching? The UI for this is an Account Action, I seem to
recall you stating at one point that Adium didn't implement the Account
Actions, if that is in fact the case then you should check out pidgin and
see what they are and how they work.

> Note that the first one should probably be replaced by a registration
> callback in the _PurpleAccountUiOps struct (so you don't have to
> define a callback in addition to defining this struct). Sean
> suggested the current solution, since he didn't want to use up one of
> those four reserved spots on the struct.

Given the above suggestions of using Account Actions (is there a reason
that doesn't work), and my suggested purple_account_registered function
none of the above requires any prpl_info changes. Can you see why we don't
immediately jump on the idea that everything needs to go into the
prpl_info struct? Is there some reason my suggestions about don't work?

> One additional function to be added to the prpl struct would be
> unregister (as already mentioned).

And as I already mentioned, I think it should be added to the struct.

> > If something *is* a status, it should be handled as a status.  It's
> > that simple.
>
> I agree on that one, back then I didn't really know how the status
> system works. However, the system is still not suited as the ultimate
> solution to all of the status problems. It's not the concept, but the
> particular implementation, that needs a fix.

Please work on fixing the implementation, the XMPP PEP stuff is the first
real application of the status API in any but the most simple way. It is
only natural to assume that you will need to enhance it to make it work
well. Do so.

> > "Easier to implement" isn't the only metric, and "bugs in the initial
> > implementation" is a nearly worthless metric.  Things like "logical",
> > "consistent", "maintainable", and "correct" are very important.
>
> I agree on that in theory. However, the current workaround I had to
> implement is _not_ easily maintainable, but still required.
> In case you don't know, that's how it's implemented right now:
> On every status change function call, the function compares the new
> status values to the old ones stored in the JabberStream struct. If
> those are different, it sends the appropriate stanza (either
> <presence/>, the user tune, the user nick, the user avatar or the
> user mood right now). Afterwards, it copies the new values to the
> JabberStream-struct.
> This means that every time a new status key is added, the
> JabberStream struct and the status update function have to be
> adapted. I hope that nobody forgets about this, otherwise things can
> get really messy.
>
> This is required since there's no way to determine what status entry
> has changed.

Yes, this certainly sounds like the status api could use fixing up. At
first blush it would seem that adding a GList of changed status keys would
do what was necessary for this circumstance, it would simplify the
do-I-care check immensely.

Is that a good solution? I don't know, I don't know the status API really
at all. But I would welcome a discussion about what would be a good change
and what would accomplish the goals here and be open enough to scale to
future needs.

> andy

	-Etan