Revision f6a67901e79d8d35e6bf30f0766b2417740090b7

[Andreas Monitzer]

On Aug 28, 2007, at 14:03, Nathan Walp wrote:

> Andreas Monitzer wrote:
>> First off, thank you for reviewing the code.
>> On Aug 28, 2007, at 03:28, Nathan Walp wrote:
>>> - I dislike implementing experimental JEPs in libpurple
>> The problem is that the XSF is very slow in moving XEPs to more  
>> stable states, it usually takes a few years. I asked people in the  
>> know about all experimental XEPs I considered implementing before  
>> doing so. All of them are not expected to change much or getting  
>> thrown out again. That's the reason I didn't implement User Profile.
>> XEP-0115: Entity Capabilities has changed since I implemented it,  
>> the implementation will need some modifications.
>
> This is *exactly* why I don't like implementing them.  They do  
> change out from under you.

Note that XEP-0115 is draft, not experimental.

> I would suggest we either put the experimental ones behind a  
> #ifdef, or keep them in a branch until they mature a bit more.

Having them in a branch would mean that somebody has to watch the  
libpurple commits all the time and look for other people changing  
something that is of concern to these features, and update the code  
accordingly (see the attention API change from last week). I  
certainly don't have the time for this, if you'd be willing to do  
that, it'd be fine by me.

> Most of the ones that have been sitting for eternity have been  
> doing so because they depend on PEP, which took its dear sweet  
> time.  Now that PEP is wrapping up, I think we should start to see  
> these trickle to Draft.  When they're Draft, and "stable" I'm  
> usually more comfortable unleashing them upon the masses.

The XEPs depending on PEP were more mature than PEP itself.

>>> - forces buddy icons to PNGs, which will kill animated buddy icons
>> XEP-0084 says that PNG is required. It's possible to specify  
>> multiple alternatives (at least one of them has to be PNG), but  
>> libpurple's API doesn't allow for that, so I had to force it to  
>> PNG only.
>>> - the prpl now knows way too much about PNGs.  That functionality  
>>> should
>>> be abstracted out to somewhere else, core or otherwise.
>> Yes, knowing the image size would be great. I hope you understand  
>> that I hesitated rewriting the core API for my project.
>
> Unfortunately, people get really cranky when their buddy icons  
> don't animate.

That's news to me. Adium specifically checks for animated avatars and  
disallows them, and I haven't heard a single person complaining about  
that.

> If we have the proper abilities in the core, then we should be able  
> to take whatever we get as a buddy icon, get both its mime type and  
> size, and if the mime type isn't image/png, convert it to one so we  
> have both, and post both in the PEP portion, and the original (with  
> correct mime type) in the vCard (for the next 6 months to a year,  
> then I'd like to see that code go the way of the do do, if the PEP  
> implementations have "caught on" in the rest of the jabber world).

Yes agreed, that would be the clean solution. However, if you don't  
want to write the image conversion code yourself, you'd have to add  
another dependency like DevIL. This is something I tried to avoid.

> By the way, I haven't looked at your 0084 receiving code, but if it  
> doesn't already do so, it should probably not prefer PNG, so we  
> make sure we read the (potentially animated) GIF if it is there.

Right now it uses the last image/png data stream that's advertised,  
but that's easy to change.

> Granted I haven't talked to stpeter about this, but I guess I don't  
> understand why we wouldn't get a disco#info result from an  
> "offline" gateway.

How do you know which buddy is a gateway? If you already knew, you  
wouldn't have to ask. Sending a disco#info to all contacts on your  
roster on logging in would get stpeter up in arms about libpurple.

> And we should be able to avoid the storm where possible by waiting  
> a few seconds to receive presence from the gateways that we'll get  
> it from, and then disco the rest.

You'd get an error reply from offline gateways (or rather, the server  
speaking in behalf of the non-existing JID), so you wouldn't be any  
wiser.

<error code='503' type='cancel'>
	<service-unavailable xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>
</error>

> It seems to me (and by all means, correct me if I'm mistaken), that  
> the only way we're going to have an "offline" gateway is if one of  
> the following has happened:
>
> - We've purposefully logged out of it this session, and therefore  
> already know it is a gateway.

This could also happen when the gateway got disconnected from the  
legacy network.

> - We've just registered with it, so we already know it is a gateway.
> - The server is busted
> - It isn't a gateway.

- The gateway is not running
- The gatway couldn't log in (due to some network or authentication  
error for instance)

> I'm not sure what the expected behavior is when a busted server  
> comes back online,

I don't know either, but my gateways don't do anything when they get  
back online. I have to send them a presence packet to get them to  
recognize me.

> but otherwise, I don't see a problem with not assuming it is a  
> gateway until we actually know.

You'd get a hen-and-egg-problem. You couldn't log in because you  
don't know whether it's a gateway, and you need to log in to  
recognize it as a gateway.

> I don't use gateways, haven't used them in probably 7 or 8 years  
> now (hence the lack of implementation up to this point ;-)), so if  
> I've missed something, it won't surprise me terribly.

I've been using them for about 7 or 8 years now for AIM and ICQ  
exclusively.

>>> - inconsistent indenting throughout
>> Could you give an example?
>
> Use an editor that shows you the difference between tabs and  
> spaces, or set your tab spacing to something other than 8, and it  
> should become readily apparent.

Visible whitespace adds considerable noise to the code (up to the  
point where I can't see anything), and my text editor is set to tab  
width 4. I looked through all files I touched when I switched to tabs  
(you might remember that huge commit back then), and I explicitly  
checked for tabs on all new code I wrote after that.

andy