Proposal for an extended callbacks field

[Etan Reisner]

On Wed, Jul 25, 2007 at 11:19:23PM +0200, Andreas Monitzer wrote:
> On Jul 25, 2007, at 18:16, Etan Reisner wrote:
>
> >>The second occurrence was the registration callback. This wasn't
> >>solved, but a workaround was introduced by having an out-of-struct
> >>function for it.
> >
> >That is a solution, the fact that it wasn't your initial proposal
> >isn't
> >the point. The point is that having looked over the requirements it
> >was
> >decided that a callback method was not the right one, especially since
> >your proposal included passing ui callbacks directly into the prpl.
>
> What's so bad about that? The prpl can't do anything malicious with
> that ui callback pointer.

What's bad about this is *exactly* the sort of thing that makes Evan cry
every time we use the request API. Which is that is *assumes* a given
interface set up, in this case that the core never needs to do anything
with such information. Unless of course you intend to start passing core
callbacks in in which case having the prpl call a well-known public core
function instead of a callback works just as well.

The fact that your currently implemented method is an ugly one (and by the
way not at all the one I suggested when this first came up which was to
have a purple_account_registered function and an associated signal (and
possibly a ui_op if needed) which is simpler and should be able to do the
same things as your current method, correct me if it can't) is not a very
good argument against our assertions that using a prpl_info struct wasn't
a good idea and is in fact the point I have been trying to make in most of
these emails. There are any number of ways of implementing the same
features, the trick is to finding the best one given a large number of
constraints.

> >In this case I felt like you had over-reacted in that the
> >presentation of
> >this issue seemed to be that you had been explicitely barred from ever
> >adding anything to the prpl_info struct even when that was
> >obviously the
> >right answer.
>
> That's the feeling I got. Additionally, some things Adium needs to be
> exposed from the XMPP plugin will never go into the prpl struct (like
> sending ad-hoc commands, or gateway control), since they're XMPP-
> specific. Right now I just leverage the fact that Adium doesn't use
> the plugin architecture and compiles everything into the library,
> which means that I can call those plugin functions directly. This
> also means that pidgin will never be able to use them (not that you'd
> want to in those particular cases, though).

Ad-hoc commands are very much something I would like to get pidgin to
support, but I can not see my way clear to adding them to the prpl_info
struct because they are inherently protocol specific. I can however see
adding them in a generic fashion as an Account Action (perhaps for actions
on the server itself) and as blist-node-extended-menu actions for
individual buddies. Both of which can be added to pidgin in even a micro
version.

> >Adding a prpl_info callback to unregister an account is *exactly* the
> >right thing to do. I support doing that whole-heartedly. I didn't
> >see you
> >bring that up anywhere that I look, I don't routinely watch adium
> >bugs had
> >you brought it up here I would have immediately supported it.
>
> I didn't try to bring it up, since I considered this endeavor to be a
> waste of my time better spent on enhancing Adium instead (or solving
> the prpl problem altogether).

Not bringing things up is almost never the right answer, and it is
certainly not the right answer if you later try to use the fact that they
weren't considered or handled appropriately against the people you didn't
bring them up to. And as clearly indicated in my previous email I would
have immediately supported this suggestion.

<snip>

> >And no, adding this hashtable wouldn't really allow for libpurple
> >UIs to
> >add arbitrary features without needing a pidgin release unless they
> >intend
> >to be tracking libpurple from MTN, because libpurple itself will still
> >need the code added to actually do things with the added callbacks,
> >and
> >those additions will all block on a release, they may only block on a
> >minor/micro release as opposed to possibly/occasionally blocking on a
> >major release (but that should basically never happen, because that
> >is the
> >point of the struct padding). And if the UI is going to be tracking
> >MTN
> >then they can maintain any arbitrary changes they want in their
> >libpurple
> >build because they won't be able to use the installed version of
> >libpurple
> >anyway.
>
> 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.

I didn't see anyone suggest you weren't supposed to require a major
version bump, and I don't know about what Adium is pushing you for but I
do not believe that the pidgin team is requiring anything to hit a release
by the end of the summer and (my knowledge of the SoC terms is limited) I
do not believe Google does either. My understanding was only that the code
needed to be accepted into the project, and by my definition waiting for
inclusion until a major bump can occur is 'accepted'.

As Ethan commented, if the Adium people are pushing you for results for
results sake I think that is an issue best discussed with your mentors.

> >Long story short: TALK to us when you are trying to figure out how to
> >implement something, if it really does need a prpl_info struct
> >change that
> >is fine, but when it doesn't accept that and move on
>
> The problem is that I can't just move on and scrap that feature,
> since I'm not on your payroll, but on Adium's. If the Adium project
> wants a certain feature, I have to find a way to get it implemented,
> no matter how (since Adium is very user-driven). Generally, I don't
> like scrapping features just because they don't fit into the current
> design.

Ethan most adequately explained my point here.

> >(this is where you tend to over-react by the way, because you
> >assume that your original
> >design must be the best and get offended/angry when told otherwise
> >and you
> >assume that we are being unreasonable ogres when suggesting you use a
> >different method despite our giving reasons for the suggestion).
>
> I'm very much in favor of implementing other people's ideas, if
> they're reasonable and don't require 10x the amount of code than my
> solution. Unfortunately, this hasn't been the case with your
> solutions. Having more code inevitably results in more bugs. In fact,
> the current status-based user tune implementation did create a very
> obscure bug I first had to discover, identify and fix. That alone
> cost me an evening.
> My solutions aren't bound to be the best, but they're reasonable most
> of the time (otherwise I wouldn't propose them). If anybody comes up
> with a solution that's easier to implement and thus causes less bugs,
> I'm all for it.

Each time I have commented on a proposal you have suggested I have done so
because I have found them in fact unreasonable given the context they were
made in. And each time I have endeavored to best understand the impetus
behind your suggestion and further endeavored to come up with a better
solution which accomplishes the goal.

You have each time seemed at best grudging and at worst down right
offended by the suggestions that perhaps your original idea wasn't the
best one. And on more than one occasion insinuated that we clearly had no
idea what was going on. Neither of which indicates much towards your being
"in favor of implementing other people's ideas", I apologize if that was
your intent and I just failed to see it through all of the rest of the
stuff.

As to your comments about bugs and metrics and the like, I will once again
refer to Ethan's comments. An open source project runs differently than a
commercial one. In a commercial product 'working' is the ultimate goal, in
an open source one (especially one as prominent as pidgin) the overall
coherence of the code is at least as important (if not even more
important) then simply shipping a working product.

> andy

	-Etan