I think it would be a pain to do that, and might not be clear for wl_seat and wl_data_device. If you believe in it, all power to you. <div class="gmail_quote">On 22 Sep 2014 02:10, "Pekka Paalanen" <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, 25 Aug 2014 08:59:51 +0300 Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> wrote: > On Fri, 22 Aug 2014 10:48:02 -0700 > Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > > > As I mentioned on IRC, I don't really like adding and using an enum value > > without bumping protocol version. However, the only time we ever use it is > > to kill the client so the worst thing that can happen is that the client > > doesn't report the error correctly. I think I'm ok with it. > > Yeah, I don't see a problem in adding values to an error enum. > > However, I do wonder if this is logical. > > Let's say xdg_shell used this. A client issues request > xdg_shell@33.get_xdg_surface(wl_surface@20). There are quite likely > several other requests on wl_surface@20, and since I assume > get_xdg_surface will be raising a protocol error, there will also be an > earlier request on some other interface already giving a role to > wl_surface@20. And all these requests can be in the same burst. > > Using the below enum value, the compositor would send the error on > wl_surface@20, but you don't know which interface call caused it. > > Using an error enum value in xdg_shell, the compositor would be sending > the error on xdg_shell@33, but you don't know for which surface. > > One solution to both "don't know"s is to add the missing information to > the error message for humans to see. > > Both ways could work in practice, but this would be the first (legal) > case of sending the error on a different object than whose request > caused it to trigger. > > It may make sense in this one case, but if we do it here, we generally > allow it everywhere, which I believe can bring confusion on what errors > can be raised where. It will no longer be true, that the possible > errors are the ones defined in the interface containing the request, > plus wl_display errors. You will be also adding all errors on all > argument objects to the set, per request. > > That is why I am hesitant to commit this. I'm sure someone will find > a case where this would make something like writing language bindings > more difficult for him, even if it seems obscure or even silly to us. > > It's not like we share e.g. pixel format enums between, say, wl_shm and > wl_drm, even though they are the same AFAIK, and there is a > considerable amount of duplication. (Yes, that is a bit different case, > because there we would be breaking also the namespacing.) > > > Thanks, > pq > > > On Fri, Aug 22, 2014 at 10:37 AM, Jasper St. Pierre <<a href="mailto:jstpierre@mecheye.net">jstpierre@mecheye.net</a>> > > wrote: > > > > > This will be used by implementations to send out errors if clients try > > > to set roles on surfaces that already have roles. > > > --- > > > protocol/wayland.xml | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > > > index bb457bc..f6eb54d 100644 > > > --- a/protocol/wayland.xml > > > +++ b/protocol/wayland.xml > > > @@ -983,6 +983,7 @@ > > > </description> > > > <entry name="invalid_scale" value="0" summary="buffer scale value > > > is invalid"/> > > > <entry name="invalid_transform" value="1" summary="buffer transform > > > value is invalid"/> > > > + <entry name="surface_has_existing_role" value="2" summary="the > > > surface passed already has a role"/> > > > </enum> Hi, does anyone still want to defend this patch, or should we add "existing role" error definitions to all interfaces assigning roles instead? Thanks, pq </blockquote></div>