On Mon, 25 Aug 2014 08:59:51 +0300 Pekka Paalanen <[email protected]> wrote:
> On Fri, 22 Aug 2014 10:48:02 -0700 > Jason Ekstrand <[email protected]> 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 > [email protected]_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 <[email protected]> > > 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 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
