[PATCH] Add a new wl_proxy_set_listener function
ppaalanen at gmail.com
Fri Jun 17 10:04:47 UTC 2016
On Thu, 16 Jun 2016 14:33:44 -0400
Jonas Ådahl <jadahl at gmail.com> 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 <derekf at osg.samsung.com>
> > 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 811 bytes
Desc: OpenPGP digital signature
More information about the wayland-devel