On Thu, 16 Jun 2016 14:33:44 -0400
Jonas Ådahl <[email protected]> wrote:

> On Thu, Jun 16, 2016 at 11:27:31AM -0500, Derek Foreman wrote:
> > On 19/03/16 11:14 AM, Giulio Camuffo wrote:  
> > > The wl_proxy_add_listener function is ill-named, because it suggests
> > > that a proxy can have many listeners. Instead it can only have one so
> > > deprecate the old function and add a new wl_proxy_set_listener. The
> > > implementation of the new function is exactly the same implementation
> > > wl_proxy_add_listener had, and that is now implemented by calling
> > > wl_proxy_set_listener.
> > > Also make the scanner output new <interface>_set_listener functions.  
> > 
> > I like this, and it's
> > Reviewed-by: Derek Foreman <[email protected]>
> > 
> > However, since set_listener is new, this is a chance to make
> > proxy_set_listener() bail more aggressively (perhaps abort) on a second
> > set (while still letting add_listener continue to work as it always has)
> > 
> > Do we want to do that now before it's too late?  I have no strong
> > opinion either way, so I'll defer to your judgment here.  
> 
> Do we really want to? What if you want to switch listener for whatever
> reason? Is there any reason we wouldn't want to allow it?
> 
> Given that we change the name to "set", and if we make it clear that
> setting a listener replaces the previous one, it should be obvious
> enough that it is indeed simply replaced, I don't see any reason for not
> allowing it.

That's one way to go with it, sure.

If we look at user_data, add/set_listener is not the only one setting
it, there is also set_user_data.

It seems to be a common confusion to call both set_user_data and
add_listener. If you pass different pointers to each without realizing
they both set the same member, you would be in for a surprise. Do we
really want to hold the users' hand so much to try to detect that? I
think good API design principles say we have already lost that case.

It is not unthinkable to want to re-set user_data, a little less so to
re-set the listener, but still no solid reason to forbid either.

What is the path of least surprise? I believe that is to not forbid
re-setting, so I'm with Jonas on this.

My actual patch review will be in another email.


Thanks,
pq

Attachment: pgpw8r3AAKgQd.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to