[PATCH] protocol: Add summaries to event parameters
Yong Bakos
junk at humanoriented.com
Thu Mar 31 13:36:56 UTC 2016
> On Mar 30, 2016, at 12:53 PM, Bryce Harrington <bryce at osg.samsung.com> wrote:
>
> On Tue, Mar 29, 2016 at 06:04:49PM -0500, Yong Bakos wrote:
>> From: Yong Bakos <ybakos at humanoriented.com>
>>
>> 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 <ybakos at humanoriented.com>
>
> 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.
>> </description>
>> - <arg name="object_id" type="object"/>
>> - <arg name="code" type="uint"/>
>> - <arg name="message" type="string"/>
>> + <arg name="object_id" type="object" summary="object where the error occurred"/>
>> + <arg name="code" type="uint" summary="error code"/>
>> + <arg name="message" type="string" summary="error description"/>
>> </event>
>>
>> <enum name="error">
>> @@ -96,7 +96,7 @@
>> When the client receive this event, it will know that it can
>> safely reuse the object ID.
>> </description>
>> - <arg name="id" type="uint" />
>> + <arg name="id" type="uint" summary="deleted object id"/>
>> </event>
>> </interface>
>>
>> @@ -141,9 +141,9 @@
>> the given name is now available, and it implements the
>> given version of the given interface.
>> </description>
>> - <arg name="name" type="uint"/>
>> - <arg name="interface" type="string"/>
>> - <arg name="version" type="uint"/>
>> + <arg name="name" type="uint" summary="name of the global object"/>
>> + <arg name="interface" type="string" summary="interface implemented by the object"/>
>> + <arg name="version" type="uint" summary="interface version"/>
>> </event>
>>
>> <event name="global_remove">
>> @@ -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.
>> </description>
>> - <arg name="name" type="uint"/>
>> + <arg name="name" type="uint" summary="name of the global object"/>
>
> 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.
>> </event>
>> </interface>
>>
>> @@ -367,7 +367,7 @@
>> can be used for buffers. Known formats include
>> argb8888 and xrgb8888.
>> </description>
>> - <arg name="format" type="uint" enum="format"/>
>> + <arg name="format" type="uint" enum="format" summary="buffer pixel format"/>
>> </event>
>> </interface>
>>
>> @@ -485,7 +485,7 @@
>> event per offered mime type.
>> </description>
>>
>> - <arg name="mime_type" type="string"/>
>> + <arg name="mime_type" type="string" summary="offered mime type"/>
>> </event>
>>
>> <!-- Version 3 additions -->
>> @@ -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.
>> </description>
>> - <arg name="source_actions" type="uint"/>
>> + <arg name="source_actions" type="uint" summary="actions offered by the data source"/>
>> </event>
>>
>> <event name="action" since="3">
>> @@ -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.
>> </description>
>> - <arg name="dnd_action" type="uint"/>
>> + <arg name="dnd_action" type="uint" summary="action selected by the compositor"/>
>> </event>
>> </interface>
>>
>> @@ -633,7 +633,7 @@
>> Used for feedback during drag-and-drop.
>> </description>
>>
>> - <arg name="mime_type" type="string" allow-null="true"/>
>> + <arg name="mime_type" type="string" allow-null="true" summary="mime type accepted by the target"/>
>> </event>
>>
>> <event name="send">
>> @@ -643,8 +643,8 @@
>> close it.
>> </description>
>>
>> - <arg name="mime_type" type="string"/>
>> - <arg name="fd" type="fd"/>
>> + <arg name="mime_type" type="string" summary="mime type for the data"/>
>> + <arg name="fd" type="fd" summary="file descriptor for the data"/>
>> </event>
>>
>> <event name="cancelled">
>> @@ -746,7 +746,7 @@
>> Clients can trigger cursor surface changes from this point, so
>> they reflect the current action.
>> </description>
>> - <arg name="dnd_action" type="uint"/>
>> + <arg name="dnd_action" type="uint" summary="action selected by the compositor"/>
>> </event>
>> </interface>
>>
>> @@ -821,7 +821,7 @@
>> mime types it offers.
>> </description>
>>
>> - <arg name="id" type="new_id" interface="wl_data_offer"/>
>> + <arg name="id" type="new_id" interface="wl_data_offer" summary="the new data_offer object"/>
>> </event>
>>
>> <event name="enter">
>> @@ -832,11 +832,12 @@
>> local coordinates.
>> </description>
>>
>> - <arg name="serial" type="uint"/>
>> - <arg name="surface" type="object" interface="wl_surface"/>
>> - <arg name="x" type="fixed"/>
>> - <arg name="y" type="fixed"/>
>> - <arg name="id" type="object" interface="wl_data_offer" allow-null="true"/>
>> + <arg name="serial" type="uint" summary="serial of the enter event"/>
>> + <arg name="surface" type="object" interface="wl_surface" summary="client surface entered"/>
>> + <arg name="x" type="fixed" summary="x coordinate in surface-relative coordinates"/>
>> + <arg name="y" type="fixed" summary="y coordinate in surface-relative coordinates"/>
>> + <arg name="id" type="object" interface="wl_data_offer" 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.
>> </event>
>>
>> <event name="leave">
>> @@ -855,8 +856,8 @@
>> coordinates.
>> </description>
>> <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
>> - <arg name="x" type="fixed"/>
>> - <arg name="y" type="fixed"/>
>> + <arg name="x" type="fixed" summary="x coordinate in surface-relative coordinates"/>
>> + <arg name="y" type="fixed" summary="y coordinate in surface-relative coordinates"/>
>> </event>
>>
>> <event name="drop">
>> @@ -891,7 +892,8 @@
>> destroy the previous selection data_offer, if any, upon receiving
>> this event.
>> </description>
>> - <arg name="id" type="object" interface="wl_data_offer" allow-null="true"/>
>> + <arg name="id" type="object" interface="wl_data_offer" allow-null="true"
>> + summary="selection data_offer object"/>
>> </event>
>>
>> <!-- Version 2 additions -->
>> @@ -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.
>> </description>
>> - <arg name="serial" type="uint"/>
>> + <arg name="serial" type="uint" summary="serial of the ping"/>
>> </event>
>
> "serial number of the ping"
Got it, for all subsequent occurrences too.
>> <event name="configure">
>> @@ -1255,9 +1257,9 @@
>> in surface local coordinates.
>> </description>
>>
>> - <arg name="edges" type="uint" enum="resize"/>
>> - <arg name="width" type="int"/>
>> - <arg name="height" type="int"/>
>> + <arg name="edges" type="uint" enum="resize" summary="how the surface was resized"/>
>> + <arg name="width" type="int" summary="new width of the surface"/>
>> + <arg name="height" type="int" summary="new height of the surface"/>
>> </event>
>>
>> <event name="popup_done">
>> @@ -1533,7 +1535,7 @@
>>
>> Note that a surface may be overlapping with zero or more outputs.
>> </description>
>> - <arg name="output" type="object" interface="wl_output"/>
>> + <arg name="output" type="object" interface="wl_output" summary="output entered by the surface"/>
>> </event>
>>
>> <event name="leave">
>> @@ -1542,7 +1544,7 @@
>> results in it no longer having any part of it within the scanout region
>> of an output.
>> </description>
>> - <arg name="output" type="object" interface="wl_output"/>
>> + <arg name="output" type="object" interface="wl_output" summary="output left by the surface"/>
>> </event>
>>
>> <!-- Version 2 additions -->
>> @@ -1701,7 +1703,7 @@
>> The above behavior also applies to wl_keyboard and wl_touch with the
>> keyboard and touch capabilities, respectively.
>> </description>
>> - <arg name="capabilities" type="uint" enum="capability"/>
>> + <arg name="capabilities" type="uint" enum="capability" summary="capabilities of the seat"/>
>> </event>
>>
>> <request name="get_pointer">
>> @@ -1751,7 +1753,7 @@
>> identify which physical devices the seat represents. Based on
>> the seat configuration used by the compositor.
>> </description>
>> - <arg name="name" type="string"/>
>> + <arg name="name" type="string" summary="seat identifier"/>
>> </event>
>>
>> <!-- Version 5 additions -->
>> @@ -1832,8 +1834,8 @@
>> an appropriate pointer image with the set_cursor request.
>> </description>
>>
>> - <arg name="serial" type="uint"/>
>> - <arg name="surface" type="object" interface="wl_surface"/>
>> + <arg name="serial" type="uint" summary="serial of the enter event"/>
>
> serial number of the event
>
>> + <arg name="surface" type="object" interface="wl_surface" summary="surface entered by the pointer"/>
>> <arg name="surface_x" type="fixed" summary="x coordinate in surface-relative coordinates"/>
>> <arg name="surface_y" type="fixed" summary="y coordinate in surface-relative coordinates"/>
>> </event>
>> @@ -1846,8 +1848,8 @@
>> The leave notification is sent before the enter notification
>> for the new focus.
>> </description>
>> - <arg name="serial" type="uint"/>
>> - <arg name="surface" type="object" interface="wl_surface"/>
>> + <arg name="serial" type="uint" summary="serial of the leave event"/>
>
> serial number of the leave event
>
>> + <arg name="surface" type="object" interface="wl_surface" summary="surface left by the pointer"/>
>> </event>
>>
>> <event name="motion">
>> @@ -1881,10 +1883,10 @@
>> granularity, with an undefined base.
>> </description>
>>
>> - <arg name="serial" type="uint"/>
>> + <arg name="serial" type="uint" summary="serial of the button event"/>
>> <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
>> - <arg name="button" type="uint"/>
>> - <arg name="state" type="uint" enum="button_state"/>
>> + <arg name="button" type="uint" summary="button that produced the event"/>
>> + <arg name="state" type="uint" enum="button_state" summary="physical state of the button"/>
>> </event>
>>
>> <enum name="axis">
>> @@ -1916,8 +1918,8 @@
>> </description>
>>
>> <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
>> - <arg name="axis" type="uint" enum="axis"/>
>> - <arg name="value" type="fixed"/>
>> + <arg name="axis" type="uint" enum="axis" summary="axis type"/>
>> + <arg name="value" type="fixed" summary="length of vector in surface-relative coordinate space"/>
>
> I had to double-doublecheck this last one, as the original description
> seemed unclear to me:
>
> For scroll events (vertical and horizontal scroll axes), the
> value parameter is the length of a vector along the specified
> axis in a coordinate space identical to those of motion events,
> representing a relative movement along the specified axis.
>
> Motion events are described as:
>
> Notification of pointer location change. The arguments
> surface_x and surface_y are the location relative to the
> focused surface.
>
> So looks like your summary is correct, after all, it's the vector length
> in surface-relative coordinates.
>
>> </event>
>>
>> <!-- Version 3 additions -->
>> @@ -2020,7 +2022,7 @@
>> The order of wl_pointer.axis_discrete and wl_pointer.axis_source is
>> not guaranteed.
>> </description>
>> - <arg name="axis_source" type="uint" enum="axis_source"/>
>> + <arg name="axis_source" type="uint" enum="axis_source" summary="source of the axis event"/>
>
> How about:
>
> "type of input that generated the axis event"
Here, and elsewhere, I recognized that it is more honest to say that this
enum arg is the "type of" the input source and not the source itself. But
I chose to be more succinct, when something like "type of" can be inferred
from the enum arg. I believe I was also influenced by the writing convention
of similar args summaries elsewhere, but I will double check this.
yong
>> </event>
>>
>> <event name="axis_stop" since="5">
>> @@ -2073,8 +2075,8 @@
>> The order of wl_pointer.axis_discrete and wl_pointer.axis_source is
>> not guaranteed.
>> </description>
>> - <arg name="axis" type="uint" enum="axis" />
>> - <arg name="discrete" type="int"/>
>> + <arg name="axis" type="uint" enum="axis" summary="axis type"/>
>> + <arg name="discrete" type="int" summary="number of steps"/>
>> </event>
>> </interface>
>>
>> @@ -2100,9 +2102,9 @@
>> This event provides a file descriptor to the client which can be
>> memory-mapped to provide a keyboard mapping description.
>> </description>
>> - <arg name="format" type="uint" enum="keymap_format"/>
>> - <arg name="fd" type="fd"/>
>> - <arg name="size" type="uint"/>
>> + <arg name="format" type="uint" enum="keymap_format" summary="keymap format"/>
>> + <arg name="fd" type="fd" summary="keymap file descriptor"/>
>> + <arg name="size" type="uint" summary="keymap size, in bytes"/>
>> </event>
>>
>> <event name="enter">
>> @@ -2110,8 +2112,8 @@
>> Notification that this seat's keyboard focus is on a certain
>> surface.
>> </description>
>> - <arg name="serial" type="uint"/>
>> - <arg name="surface" type="object" interface="wl_surface"/>
>> + <arg name="serial" type="uint" summary="serial of the enter event"/>
>> + <arg name="surface" type="object" interface="wl_surface" summary="surface gaining keyboard focus"/>
>> <arg name="keys" type="array" summary="the currently pressed keys"/>
>> </event>
>>
>> @@ -2123,8 +2125,8 @@
>> The leave notification is sent before the enter notification
>> for the new focus.
>> </description>
>> - <arg name="serial" type="uint"/>
>> - <arg name="surface" type="object" interface="wl_surface"/>
>> + <arg name="serial" type="uint" summary="serial of the leave event"/>
>> + <arg name="surface" type="object" interface="wl_surface" summary="surface that lost keyboard focus"/>
>> </event>
>>
>> <enum name="key_state">
>> @@ -2142,10 +2144,10 @@
>> granularity, with an undefined base.
>> </description>
>>
>> - <arg name="serial" type="uint"/>
>> + <arg name="serial" type="uint" summary="serial of the key event"/>
>
> serial number of the key event
>
>> <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
>> - <arg name="key" type="uint"/>
>> - <arg name="state" type="uint" enum="key_state"/>
>> + <arg name="key" type="uint" summary="key that produced the event"/>
>> + <arg name="state" type="uint" enum="key_state" summary="physical state of the key"/>
>> </event>
>>
>> <event name="modifiers">
>> @@ -2154,11 +2156,11 @@
>> changed, and it should update its local state.
>> </description>
>>
>> - <arg name="serial" type="uint"/>
>> - <arg name="mods_depressed" type="uint"/>
>> - <arg name="mods_latched" type="uint"/>
>> - <arg name="mods_locked" type="uint"/>
>> - <arg name="group" type="uint"/>
>> + <arg name="serial" type="uint" summary="serial for the modifiers event"/>
>> + <arg name="mods_depressed" type="uint" summary="depressed modifiers"/>
>> + <arg name="mods_latched" type="uint" summary="latched modifiers"/>
>> + <arg name="mods_locked" type="uint" summary="locked modifiers"/>
>> + <arg name="group" type="uint" summary="keyboard layout"/>
>> </event>
>>
>> <!-- Version 3 additions -->
>> @@ -2211,9 +2213,9 @@
>> this ID. The ID ceases to be valid after a touch up event and may be
>> re-used in the future.
>> </description>
>> - <arg name="serial" type="uint"/>
>> + <arg name="serial" type="uint" summary="serial of the touch down event"/>
>
> serial number of the touch down event
>
>> <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
>> - <arg name="surface" type="object" interface="wl_surface"/>
>> + <arg name="surface" type="object" interface="wl_surface" summary="surface touched"/>
>> <arg name="id" type="int" summary="the unique ID of this touch point"/>
>> <arg name="x" type="fixed" summary="x coordinate in surface-relative coordinates"/>
>> <arg name="y" type="fixed" summary="y coordinate in surface-relative coordinates"/>
>> @@ -2225,7 +2227,7 @@
>> this touchpoint and the touch point's ID is released and may be
>> re-used in a future touch down event.
>> </description>
>> - <arg name="serial" type="uint"/>
>> + <arg name="serial" type="uint" summary="serial of the touch up event"/>
>> <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
>> <arg name="id" type="int" summary="the unique ID of this touch point"/>
>> </event>
>> --
>> 2.7.2
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
> Thanks,
> Bryce
More information about the wayland-devel
mailing list