[PATCH v2 1/2] weston-info: Add support for tablet-unstable-v2

[Pekka Paalanen]

On Sat, 28 Apr 2018 10:01:43 +0200
Markus Ongyerth <wl at ongy.net> wrote:

> On 2018/April/27 04:55, Peter Hutterer wrote:
> > On Thu, Apr 26, 2018 at 05:01:23PM +0200, wl at ongy.net wrote:  

> > > +	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?

If you grep for it, you'll find that the weston code base uses bool in
many places.

The exact C standard version + which extensions has always been unclear
to me though, we probably just take the compiler default, and stick to
mostly what existing code does.

> > > +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.

I usually agree with ongy here, but Peter's suggestion makes sense to me
in this case - because the variables are foo and tmp_foo, not only
because they are of the same type. But that is just a personal quirk of
mine. Going with separate lines is never bad, IMO.


> > > +		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.

All existing code (99%?) in Weston uses the line-up Peter asked for, as
does the coding style doc. We rely heavily on tab stops being strictly
at columns divisible by 8.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180430/c60f0e32/attachment.sig>