Proposal for an extended callbacks field

[Andreas Monitzer]

On Jul 26, 2007, at 00:01, Ethan Blanton wrote:

> There is definitely some tension here; Ad-hoc commands, for example,
> are certainly interesting, but are really XMPP-specific.  This means
> that they probably don't belong in a global prpl information
> structure, as nothing else is likely to be able to use them.

However, that wouldn't matter in the hashtable. If the key exists,  
the application can use it. If it doesn't exist, there's no mention  
of them in any header, so there's no pollution with XMPP-specific  
stuff happening.
Of course, the type safety is a valid concern. This would have to be  
solved by some typedefs and casts.

> Perusing
> the ad-hoc commands XEP briefly, it looks to me like account actions
> and the request API are the Right Way to handle this -- that's a first
> hit, though, and it may not be correct.

That's actually one way it's implemented on my libpurple branch right  
now. You can access your server's ad-hoc commands the from contacts  
on your buddy list via their respective menu, as demonstrated here:
http://www.monitzer.com/adium/2007/screenshots/Picture%201.png
(Note that the format has changed since I took that screenshot).  
However, every xmpp node can theoretically offer such ad-hoc  
commands, which might be desired to be accessed. This is implemented  
in the service discovery browser in Adium that uses the xml console  
API for getting the required information:
http://www.monitzer.com/adium/2007/screenshots/Picture%2017.png
I can't just send the ad-hoc commands directly using jabber_send_raw,  
since the ad-hoc API has to register its callback to the IQ packet.

The multi-step user interaction described in the XEP is implemented  
using the fields request API.

> Learning about good design is certainly not a waste of your time, and
> nor is solving things in a sane and supportable fashion.

I admit that I'm only used to object-oriented design, which includes  
encapsulation and separation of concerns. Having a large procedural  
codebase is pretty new to me, so my solutions might not always be  
similar to the libpurple's dev's solutions.
In Objective C (the main programming language used by the Adium  
project), the issue at hand is actually solved using linked lists  
with the method name as the search key right in the runtime. I didn't  
invent this idea out of thin air.

> I'm sorry if you are being pressured to perform over learning and  
> doing things
> right; I would be surprised if this is truly the case, and perhaps you
> should have a discussion with your mentor and the other Adium
> developers about this.

During the summer, I'm programming. Learning is what I'm doing the  
rest of the year. FYI, I do have a Bachelor's degree in Software  
Engineering and already wrote some successful commercial  
applications, so I'm not a complete noob in that area.
However, I did learn the libpurple design to a certain degree, so I  
don't think I'm stepping on your toes every time I'm adding something.

> I get the impression that you believe that the responses to your
> suggestions and implementations coming from the Pidgin/libpurple team
> have been unreasonable.  I'm sorry that you feel that way, but it
> really has not been the case.  Perhaps more buy-in from your mentor,
> the Adium team, or interaction with the Pidgin developers would help
> ease this apparent friction.

In case you didn't know, Augie Fackler, who started this thread, is  
my SoC mentor, so he does help me in this area. I don't have anybody  
at the pidgin/libpurple team to talk to, which might be a problem. I  
only talk to Sean sometimes on IRC when I can catch him online.

>> Maybe my underlying problem is that I'm supposed to implement major
>> features into the XMPP plugin during a 3 months period without
>> resulting in a major version bump.
>
> So far, I haven't seen any features that would *require* a major
> version bump, other than voice and video.  As Etan has requested
> numerous times, if you could tell us what you're *trying* to do, we
> can help you get it done.

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.
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
jabber_register_gateway
jabber_adhoc_execute
jabber_ping_jid
jabber_user_search

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.

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

> 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.

> "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.

andy