Re: [PATCH] protocol: Add summaries to event parameters
On Thu, 31 Mar 2016 08:36:56 -0500 Yong Bakos wrote: > > On Mar 30, 2016, at 12:53 PM, Bryce Harrington > > wrote: > > > > On Tue, Mar 29, 2016 at 06:04:49PM -0500, Yong Bakos wrote: > >> From: Yong Bakos > >> > >> All event arg elements now have an appropriate summary attribute. > >> This was conducted mostly in response to the undocumented parameter > >> warnings generated during 'make check'. > >> > >> Signed-off-by: Yong Bakos > > > > Will be nice to see fewer warnings! :-) > > > > My review is a cursory doublecheck of each line against the > > corresponding description field in wayland.xml. Everything looks fine, > > but I have a few suggestions/comments below. > > > >> --- > >> protocol/wayland.xml | 128 > >> ++- > >> 1 file changed, 65 insertions(+), 63 deletions(-) > >> - > >> - > >> - > >> - > >> - >> allow-null="true"/> > >> + > >> + >> summary="client surface entered"/> > >> + > >> + > >> + >> allow-null="true" > >> + summary="source data_offer object"/> > > > > I don't know if it really matters, but the description uses the phrasing > > "surface local coordinates" so I wonder if the x and y coordinate > > summaries should follow suit? > > /Yes/! But in this patch I forced myself to stick to precedent for all > summary additions. I have a separate patch coming that standardizes the > use of "surface local," along with numerous grammar/spelling corrections. Hi, I think I'm to blame for the "surface local coordinates" term. In the early days, most people assumed global coordinates would exist and that most things would use global coordinates in the protocol. That is not the case in Wayland, so I wanted to emphasize that the coordinates are really local and relative to the given surface, whether the point is inside or outside the surface. Nowadays I slightly regret having "local" in the term, it feels redundant. "Surface coordinates" already says it all. However, that should not be confused with "surface's coordinates" which refers to surface's position in some other coordinate system. Should we just stick to "surface local coordinates" or what, I'm not sure. I think that term is pretty established by now, even if it sounds clunky. Or maybe no-one else thinks it's clunky? Short and descriptive terms are hard to find. Thanks, pq pgpXJV9kAExi1.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
git (Re: [PATCH] protocol: Add summaries to event parameters)
On Tue, 29 Mar 2016 17:17:06 -0700 Bryce Harrington wrote: > On Tue, Mar 29, 2016 at 06:17:38PM -0500, Yong Bakos wrote: > > > > > > From: Yong Bakos > > > > > > All event arg elements now have an appropriate summary attribute. > > > This was conducted mostly in response to the undocumented parameter > > > warnings generated during 'make check'. > > > > > > Signed-off-by: Yong Bakos > > > --- > > > > Sorry I borked the subject line. Should be [PATCH wayland]... > > Let me know if I should re-send. > > It's fine, I forget to do this myself from time to time. > Fwiw 'wayland' is implied when one omits specifying the project in the > subject line, anyway. > > I've learned you can put this in your weston checkout's .git/config: > > [format] > pretty = fuller > subjectprefix = "PATCH weston" > > But I find in practice I usually have to override it anyway with > --subject-prefix to add the patchset version numbers. There is -v7 command line option applicable to both git-send-email and git-format-patch that puts in the series revision number 7. See 'git help format-patch'. Cheers, pq pgpN3erCSfmsG.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] protocol: Add summaries to event parameters
> On Mar 30, 2016, at 12:53 PM, Bryce Harrington wrote: > > On Tue, Mar 29, 2016 at 06:04:49PM -0500, Yong Bakos wrote: >> From: Yong Bakos >> >> All event arg elements now have an appropriate summary attribute. >> This was conducted mostly in response to the undocumented parameter >> warnings generated during 'make check'. >> >> Signed-off-by: Yong Bakos > > Will be nice to see fewer warnings! :-) > > My review is a cursory doublecheck of each line against the > corresponding description field in wayland.xml. Everything looks fine, > but I have a few suggestions/comments below. > >> --- >> protocol/wayland.xml | 128 >> ++- >> 1 file changed, 65 insertions(+), 63 deletions(-) >> >> diff --git a/protocol/wayland.xml b/protocol/wayland.xml >> index 8739cd3..dffb708 100644 >> --- a/protocol/wayland.xml >> +++ b/protocol/wayland.xml >> @@ -70,9 +70,9 @@ >> own set of error codes. The message is an brief description >> of the error, for (debugging) convenience. >> >> - >> - >> - >> + >> + >> + >> >> >> >> @@ -96,7 +96,7 @@ >> When the client receive this event, it will know that it can >> safely reuse the object ID. >> >> - >> + >> >> >> >> @@ -141,9 +141,9 @@ >> the given name is now available, and it implements the >> given version of the given interface. >> >> - >> - >> - >> + >> + >> + >> >> >> >> @@ -159,7 +159,7 @@ >> ignored until the client destroys it, to avoid races between >> the global going away and a client sending a request to it. >> >> - >> + > > Generally names are strings, so might be worth in this case indicating > that this is something different? I'm not actually sure what this uint > name parameter is exactly, is it an index into a list of strings? Or is > it not really a name but more like an ID number? I'll dig into this to clarify; I believe I based this off another similar property description elsewhere. >> >> >> >> @@ -367,7 +367,7 @@ >> can be used for buffers. Known formats include >> argb and xrgb. >> >> - >> + >> >> >> >> @@ -485,7 +485,7 @@ >> event per offered mime type. >> >> >> - >> + >> >> >> >> @@ -550,7 +550,7 @@ >> will be sent right after wl_data_device.enter, or anytime the source >> side changes its offered actions through wl_data_source.set_actions. >> >> - >> + >> >> >> >> @@ -591,7 +591,7 @@ >> final wl_data_offer.set_actions and wl_data_offer.accept requests >> must happen before the call to wl_data_offer.finish. >> >> - >> + >> >> >> >> @@ -633,7 +633,7 @@ >> Used for feedback during drag-and-drop. >> >> >> - >> + >> >> >> >> @@ -643,8 +643,8 @@ >> close it. >> >> >> - >> - >> + >> + >> >> >> >> @@ -746,7 +746,7 @@ >> Clients can trigger cursor surface changes from this point, so >> they reflect the current action. >> >> - >> + >> >> >> >> @@ -821,7 +821,7 @@ >> mime types it offers. >> >> >> - >> + >> >> >> >> @@ -832,11 +832,12 @@ >> local coordinates. >> >> >> - >> - >> - >> - >> - > allow-null="true"/> >> + >> + > summary="client surface entered"/> >> + >> + >> + > allow-null="true" >> + summary="source data_offer object"/> > > I don't know if it really matters, but the description uses the phrasing > "surface local coordinates" so I wonder if the x and y coordinate > summaries should follow suit? /Yes/! But in this patch I forced myself to stick to precedent for all summary additions. I have a separate patch coming that standardizes the use of "surface local," along with numerous grammar/spelling corrections. >> >> >> >> @@ -855,8 +856,8 @@ >> coordinates. >> >> >> - >> - >> + >> + >> >> >> >> @@ -891,7 +892,8 @@ >> destroy the previous selection data_offer, if any, upon receiving >> this event. >> >> - > allow-null="true"/> >> + > allow-null="true" >> + summary="selection data_offer object"/> >> >> >> >> @@ -1231,7 +1233,7 @@ >> Ping a client to check if it is receiving events and sending >> requests. A client is expected to reply with a pong request. >> >> - >> + >> > > "serial number of the ping" Got it, for all subsequent occurrences too. >> >> @@ -1255,9 +1257,9 @@ >> in surface local coordinates. >> >> >> - >> - >> - >> + >> + >> + >> >> >> >> @@ -1533,7 +1535,7 @@ >> >> Note t
Re: [PATCH] protocol: Add summaries to event parameters
On Tue, Mar 29, 2016 at 06:04:49PM -0500, Yong Bakos wrote: > From: Yong Bakos > > All event arg elements now have an appropriate summary attribute. > This was conducted mostly in response to the undocumented parameter > warnings generated during 'make check'. > > Signed-off-by: Yong Bakos Will be nice to see fewer warnings! :-) My review is a cursory doublecheck of each line against the corresponding description field in wayland.xml. Everything looks fine, but I have a few suggestions/comments below. > --- > protocol/wayland.xml | 128 > ++- > 1 file changed, 65 insertions(+), 63 deletions(-) > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > index 8739cd3..dffb708 100644 > --- a/protocol/wayland.xml > +++ b/protocol/wayland.xml > @@ -70,9 +70,9 @@ > own set of error codes. The message is an brief description > of the error, for (debugging) convenience. > > - > - > - > + > + > + > > > > @@ -96,7 +96,7 @@ > When the client receive this event, it will know that it can > safely reuse the object ID. > > - > + > > > > @@ -141,9 +141,9 @@ > the given name is now available, and it implements the > given version of the given interface. > > - > - > - > + > + > + > > > > @@ -159,7 +159,7 @@ > ignored until the client destroys it, to avoid races between > the global going away and a client sending a request to it. > > - > + Generally names are strings, so might be worth in this case indicating that this is something different? I'm not actually sure what this uint name parameter is exactly, is it an index into a list of strings? Or is it not really a name but more like an ID number? > > > > @@ -367,7 +367,7 @@ > can be used for buffers. Known formats include > argb and xrgb. > > - > + > > > > @@ -485,7 +485,7 @@ > event per offered mime type. > > > - > + > > > > @@ -550,7 +550,7 @@ > will be sent right after wl_data_device.enter, or anytime the source > side changes its offered actions through wl_data_source.set_actions. > > - > + > > > > @@ -591,7 +591,7 @@ > final wl_data_offer.set_actions and wl_data_offer.accept requests > must happen before the call to wl_data_offer.finish. > > - > + > > > > @@ -633,7 +633,7 @@ > Used for feedback during drag-and-drop. > > > - > + > > > > @@ -643,8 +643,8 @@ > close it. > > > - > - > + > + > > > > @@ -746,7 +746,7 @@ > Clients can trigger cursor surface changes from this point, so > they reflect the current action. > > - > + > > > > @@ -821,7 +821,7 @@ > mime types it offers. > > > - > + > > > > @@ -832,11 +832,12 @@ > local coordinates. > > > - > - > - > - > - allow-null="true"/> > + > + summary="client surface entered"/> > + > + > + allow-null="true" > +summary="source data_offer object"/> I don't know if it really matters, but the description uses the phrasing "surface local coordinates" so I wonder if the x and y coordinate summaries should follow suit? > > > > @@ -855,8 +856,8 @@ > coordinates. > > > - > - > + > + > > > > @@ -891,7 +892,8 @@ > destroy the previous selection data_offer, if any, upon receiving > this event. > > - allow-null="true"/> > + allow-null="true" > +summary="selection data_offer object"/> > > > > @@ -1231,7 +1233,7 @@ > Ping a client to check if it is receiving events and sending > requests. A client is expected to reply with a pong request. > > - > + > "serial number of the ping" > > @@ -1255,9 +1257,9 @@ > in surface local coordinates. > > > - > - > - > + > + > + > > > > @@ -1533,7 +1535,7 @@ > > Note that a surface may be overlapping with zero or more outputs. > > - > + > > > > @@ -1542,7 +1544,7 @@ > results in it no longer having any part of it within the scanout region > of an output. > > - > + > > > > @@ -1701,7 +1703,7 @@ > The above behavior also applies to wl_keyboard and wl_touch with the > keyboard and touch capabilities, respectively. > > - > + summary="capabilities of the seat"/> > > > > @@ -1751,7 +1753,7 @@ > identify
Re: [PATCH] protocol: Add summaries to event parameters
On Tue, Mar 29, 2016 at 05:17:06PM -0700, Bryce Harrington wrote: > On Tue, Mar 29, 2016 at 06:17:38PM -0500, Yong Bakos wrote: > > > > > > From: Yong Bakos > > > > > > All event arg elements now have an appropriate summary attribute. > > > This was conducted mostly in response to the undocumented parameter > > > warnings generated during 'make check'. > > > > > > Signed-off-by: Yong Bakos > > > --- > > > > Sorry I borked the subject line. Should be [PATCH wayland]... > > Let me know if I should re-send. > > It's fine, I forget to do this myself from time to time. > Fwiw 'wayland' is implied when one omits specifying the project in the > subject line, anyway. > > I've learned you can put this in your weston checkout's .git/config: > > [format] > pretty = fuller > subjectprefix = "PATCH weston" > > But I find in practice I usually have to override it anyway with > --subject-prefix to add the patchset version numbers. fwiw, I've started running this in any new repo, it's pretty much universally applicable: git config --add format.subjectprefix "PATCH `basename $PWD`" Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] protocol: Add summaries to event parameters
On Tue, Mar 29, 2016 at 06:17:38PM -0500, Yong Bakos wrote: > > > > From: Yong Bakos > > > > All event arg elements now have an appropriate summary attribute. > > This was conducted mostly in response to the undocumented parameter > > warnings generated during 'make check'. > > > > Signed-off-by: Yong Bakos > > --- > > Sorry I borked the subject line. Should be [PATCH wayland]... > Let me know if I should re-send. It's fine, I forget to do this myself from time to time. Fwiw 'wayland' is implied when one omits specifying the project in the subject line, anyway. I've learned you can put this in your weston checkout's .git/config: [format] pretty = fuller subjectprefix = "PATCH weston" But I find in practice I usually have to override it anyway with --subject-prefix to add the patchset version numbers. Bryce > yong > > > > protocol/wayland.xml | 128 > > ++- > > 1 file changed, 65 insertions(+), 63 deletions(-) > > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > > index 8739cd3..dffb708 100644 > > --- a/protocol/wayland.xml > > +++ b/protocol/wayland.xml > > @@ -70,9 +70,9 @@ > > own set of error codes. The message is an brief description > > of the error, for (debugging) convenience. > > > > - > > - > > - > > + > > + > > + > > > > > > > > @@ -96,7 +96,7 @@ > > When the client receive this event, it will know that it can > > safely reuse the object ID. > > > > - > > + > > > > > > > > @@ -141,9 +141,9 @@ > > the given name is now available, and it implements the > > given version of the given interface. > > > > - > > - > > - > > + > > + > > + > > > > > > > > @@ -159,7 +159,7 @@ > > ignored until the client destroys it, to avoid races between > > the global going away and a client sending a request to it. > > > > - > > + > > > > > > > > @@ -367,7 +367,7 @@ > > can be used for buffers. Known formats include > > argb and xrgb. > > > > - > > + > > > > > > > > @@ -485,7 +485,7 @@ > > event per offered mime type. > > > > > > - > > + > > > > > > > > @@ -550,7 +550,7 @@ > > will be sent right after wl_data_device.enter, or anytime the source > > side changes its offered actions through wl_data_source.set_actions. > > > > - > > + > > > > > > > > @@ -591,7 +591,7 @@ > > final wl_data_offer.set_actions and wl_data_offer.accept requests > > must happen before the call to wl_data_offer.finish. > > > > - > > + > > > > > > > > @@ -633,7 +633,7 @@ > > Used for feedback during drag-and-drop. > > > > > > - > > + > > > > > > > > @@ -643,8 +643,8 @@ > > close it. > > > > > > - > > - > > + > > + > > > > > > > > @@ -746,7 +746,7 @@ > > Clients can trigger cursor surface changes from this point, so > > they reflect the current action. > > > > - > > + > > > > > > > > @@ -821,7 +821,7 @@ > > mime types it offers. > > > > > > - > > + > > > > > > > > @@ -832,11 +832,12 @@ > > local coordinates. > > > > > > - > > - > > - > > - > > - > allow-null="true"/> > > + > > + > summary="client surface entered"/> > > + > > + > > + > allow-null="true" > > + summary="source data_offer object"/> > > > > > > > > @@ -855,8 +856,8 @@ > > coordinates. > > > > > > - > > - > > + > > + > > > > > > > > @@ -891,7 +892,8 @@ > > destroy the previous selection data_offer, if any, upon receiving > > this event. > > > > - > allow-null="true"/> > > + > allow-null="true" > > + summary="selection data_offer object"/> > > > > > > > > @@ -1231,7 +1233,7 @@ > > Ping a client to check if it is receiving events and sending > > requests. A client is expected to reply with a pong request. > > > > - > > + > > > > > > > > @@ -1255,9 +1257,9 @@ > > in surface local coordinates. > > > > > > - > > - > > - > > + > > + > > + > > > > > > > > @@ -1533,7 +1535,7 @@ > > > > Note that a surface may be overlapping with zero or more outputs. > > > > - > > + > summary="output entered by the surface"/> > > > > > > > > @@ -1542,7 +1544,7 @@ > > results in it no longer having any part of it within the scanout region > > of an output. > > > > - > > + > summary="output left by the surface"/> > > > > > > > > @@ -1701,7 +1703,7 @@ > > The above behavior also
Re: [PATCH] protocol: Add summaries to event parameters
> > From: Yong Bakos > > All event arg elements now have an appropriate summary attribute. > This was conducted mostly in response to the undocumented parameter > warnings generated during 'make check'. > > Signed-off-by: Yong Bakos > --- Sorry I borked the subject line. Should be [PATCH wayland]... Let me know if I should re-send. yong > protocol/wayland.xml | 128 ++- > 1 file changed, 65 insertions(+), 63 deletions(-) > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > index 8739cd3..dffb708 100644 > --- a/protocol/wayland.xml > +++ b/protocol/wayland.xml > @@ -70,9 +70,9 @@ > own set of error codes. The message is an brief description > of the error, for (debugging) convenience. > > - > - > - > + > + > + > > > > @@ -96,7 +96,7 @@ > When the client receive this event, it will know that it can > safely reuse the object ID. > > - > + > > > > @@ -141,9 +141,9 @@ > the given name is now available, and it implements the > given version of the given interface. > > - > - > - > + > + > + > > > > @@ -159,7 +159,7 @@ > ignored until the client destroys it, to avoid races between > the global going away and a client sending a request to it. > > - > + > > > > @@ -367,7 +367,7 @@ > can be used for buffers. Known formats include > argb and xrgb. > > - > + > > > > @@ -485,7 +485,7 @@ > event per offered mime type. > > > - > + > > > > @@ -550,7 +550,7 @@ > will be sent right after wl_data_device.enter, or anytime the source > side changes its offered actions through wl_data_source.set_actions. > > - > + > > > > @@ -591,7 +591,7 @@ > final wl_data_offer.set_actions and wl_data_offer.accept requests > must happen before the call to wl_data_offer.finish. > > - > + > > > > @@ -633,7 +633,7 @@ > Used for feedback during drag-and-drop. > > > - > + > > > > @@ -643,8 +643,8 @@ > close it. > > > - > - > + > + > > > > @@ -746,7 +746,7 @@ > Clients can trigger cursor surface changes from this point, so > they reflect the current action. > > - > + > > > > @@ -821,7 +821,7 @@ > mime types it offers. > > > - > + > > > > @@ -832,11 +832,12 @@ > local coordinates. > > > - > - > - > - > - allow-null="true"/> > + > + summary="client surface entered"/> > + > + > + allow-null="true" > +summary="source data_offer object"/> > > > > @@ -855,8 +856,8 @@ > coordinates. > > > - > - > + > + > > > > @@ -891,7 +892,8 @@ > destroy the previous selection data_offer, if any, upon receiving > this event. > > - allow-null="true"/> > + allow-null="true" > +summary="selection data_offer object"/> > > > > @@ -1231,7 +1233,7 @@ > Ping a client to check if it is receiving events and sending > requests. A client is expected to reply with a pong request. > > - > + > > > > @@ -1255,9 +1257,9 @@ > in surface local coordinates. > > > - > - > - > + > + > + > > > > @@ -1533,7 +1535,7 @@ > > Note that a surface may be overlapping with zero or more outputs. > > - > + > > > > @@ -1542,7 +1544,7 @@ > results in it no longer having any part of it within the scanout region > of an output. > > - > + > > > > @@ -1701,7 +1703,7 @@ > The above behavior also applies to wl_keyboard and wl_touch with the > keyboard and touch capabilities, respectively. > > - > + summary="capabilities of the seat"/> > > > > @@ -1751,7 +1753,7 @@ > identify which physical devices the seat represents. Based on > the seat configuration used by the compositor. > > - > + > > > > @@ -1832,8 +1834,8 @@ > an appropriate pointer image with the set_cursor request. > > > - > - > + > + summary="surface entered by the pointer"/> > > > > @@ -1846,8 +1848,8 @@ > The leave notification is sent before the enter notification > for the new focus. > > - > - > + > + summary="surface left by the pointer"/> > > > > @@ -1881,10 +1883,10 @@ > granularity, with an undefined base. > > > - > + > > - > - > +
