Hi, thanks for the feedback, I'll provide a v3 shortly. I just have a few follow up questions, before I do so.
On 2018/April/27 04:55, Peter Hutterer wrote: > On Thu, Apr 26, 2018 at 05:01:23PM +0200, w...@ongy.net wrote: > > From: Markus Ongyerth <w...@ongy.net> > > > > +struct tablet_v2_path { > > + struct wl_list link; > > + char *path; > > +}; > > + > > +struct tablet_tool_info { > > + struct wl_list link; > > + struct zwp_tablet_tool_v2 *tool; > > + > > + uint64_t hardware_serial; > > + uint64_t hardware_id_wacom; > > + enum zwp_tablet_tool_v2_type type; > > + > > + int tilt; > > + int pressure; > > + int distance; > > + int rotation; > > + int slider; > > + int wheel; > > can we make those bool? and ideally, rename them to has_tilt, has_pressure, > etc. The bool type is a C99 feature, I haven't tested it, but the file looks like it should be C90 compatible. Looking at the Makefile.am/configure.ac I didn't find any -std= flag. What's the C standard weston (clients) should follow? > > +static void > > +destroy_tablet_pad_info(struct tablet_pad_info *info) > > +{ > > + struct tablet_v2_path *path; > > + struct tablet_v2_path *tmp_path; > > + struct tablet_pad_group_info *group; > > + struct tablet_pad_group_info *tmp_group; > > nitpick: this is a case where collating the two lines would be acceptable, > imo, i.e. struct tablet_v2_path *path, *tmp_path; I really dislike that syntax, and when in doubt I refer to kernel guidelines which forbids it. Unless there's a strong wish to change it, I'd prefer to keep it as is for style reasons. > > + if (wl_list_empty(&seat->pads) && > > + wl_list_empty(&seat->tablets) && > > + wl_list_empty(&seat->tools)) { > > confusing indentation, imo, line up with the first wl_list_empy I'm not attached to the current indention style, but I'm not a fan of that alignment. Many (un/badly -configured) viewers display tabs as 4 spaces these days (I just notcied, so does my email client), which will align the content with the continuating clauses since `if (` is 4 chars. While it's not a problem for proper working environments, it's something I'd like to avoid for times where code is read on the cgit (not sure how that's configured) or github. I'll take a look at other files in the repository and how it's done there.
signature.asc
Description: PGP signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel