[PATCH weston 01/10] Use fullscreen-shell.xml from wayland-protocols

[Bryce Harrington]

On Fri, Nov 06, 2015 at 10:51:09AM +0800, Jonas Ådahl wrote:
> On Thu, Nov 05, 2015 at 11:46:46AM -0800, Bryce Harrington wrote:
> > On Wed, Nov 04, 2015 at 04:49:50PM +0800, Jonas Ådahl wrote:
> > > Use the fullscreen-shell protocol XML from the wayland-protocols
> > > installation, and remove the one we provide ourself.
> > > 
> > > Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> > >
> > > diff --git a/clients/fullscreen.c b/clients/fullscreen.c
> > > index 4fcca3d..be316d0 100644
> > > --- a/clients/fullscreen.c
> > > +++ b/clients/fullscreen.c
> > > @@ -35,7 +35,7 @@
> > >  #include <linux/input.h>
> > >  #include <wayland-client.h>
> > >  #include "window.h"
> > > -#include "fullscreen-shell-client-protocol.h"
> > > +#include "fullscreen-shell-unstable-v1-client-protocol.h"
> > 
> > Angle brackets should be used here and elsewhere, since with this change
> > the header file now is located externally from the system rather than
> > being locally present in the codebase.
> > 
> >   #include <fullscreen-shell-unstable-v1-client-protocol.h>
> 
> Good point.
> 
> >   
> > >  struct fs_output {
> > >  	struct wl_list link;
> > > @@ -46,8 +46,8 @@ struct fullscreen {
> > >  	struct display *display;
> > >  	struct window *window;
> > >  	struct widget *widget;
> > > -	struct _wl_fullscreen_shell *fshell;
> > > -	enum _wl_fullscreen_shell_present_method present_method;
> > > +	struct zwl_fullscreen_shell1 *fshell;
> > > +	enum zwl_fullscreen_shell1_present_method present_method;
> > >  	int width, height;
> > >  	int fullscreen;
> > >  	float pointer_x, pointer_y;
> > > @@ -293,10 +293,10 @@ key_handler(struct window *window, struct input *input, uint32_t time,
> > >  		if (fullscreen->current_output)
> > >  			wl_output = output_get_wl_output(fullscreen->current_output->output);
> > >  		fullscreen->present_method = (fullscreen->present_method + 1) % 5;
> > > -		_wl_fullscreen_shell_present_surface(fullscreen->fshell,
> > > -						     window_get_wl_surface(fullscreen->window),
> > > -						     fullscreen->present_method,
> > > -						     wl_output);
> > > +		zwl_fullscreen_shell1_present_surface(fullscreen->fshell,
> > > +						      window_get_wl_surface(fullscreen->window),
> > > +						      fullscreen->present_method,
> > > +						      wl_output);
> > >  		window_schedule_redraw(window);
> > >  		break;
> > >  
> > > @@ -308,8 +308,8 @@ key_handler(struct window *window, struct input *input, uint32_t time,
> > >  		wl_output = fsout ? output_get_wl_output(fsout->output) : NULL;
> > >  
> > >  		/* Clear the current presentation */
> > > -		_wl_fullscreen_shell_present_surface(fullscreen->fshell, NULL,
> > > -						     0, wl_output);
> > > +		zwl_fullscreen_shell1_present_surface(fullscreen->fshell, NULL,
> > > +						      0, wl_output);
> > 
> > Hmm.  With l's and 1's looking so similar in certain fonts, "shell1" is
> > going to look like a typo to some users.  IMO it would be better to
> > distinguish this version number with at least an underscore.
> > "_shell_v1_" would feel more consistent with the scheme being used in
> > the header file name, protocol file name, macro definitions, etc.
> 
> A slight implicit point of it is to make it a bit awkward, so that it's
> ever so slightly more obvious that this is intended to be temporary.
> 
> The other more important reason is I think we already have very long
> names, and I tried to minimize the extra name length needed.

Being intentionally awkward seems like an anti-pattern to me.  But I
would point out that if it is a goal, then making the names overly long
certainly would achieve that.

Increasing the length of the names does seem like a valid problem,
though.  I almost mentioned it but in thinking it out, I figure: a) the
symbols are intentionally temporary, and making them intentionally
longer might even encourage making the base names slightly shorter,
which is in everyone's interest; b) the scheme is already adding 2
characters, we'd be adding 4 instead, which doesn't seem *that*
excessive; c) clarity is more important to me than brevity; d) making
the names more consistent with header, macro names, etc. seems way more
important than typing convenience.

If increasing the length is still considered a problem, Bill's
suggestion to move the version to the prefix might be worth further
consideration.  It addresses making things clearer without increasing
character count, although I'm uncertain about its usability since it'll
impact name searching and sorting.
 
> I guess I can be convinced otherwise, but personally I prefer the way it
> is.

Well, we all are going to have to live with the scheme...

> > > diff --git a/configure.ac b/configure.ac
> > > index e5afbc0..cfac579 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -181,6 +181,13 @@ fi
> > >  PKG_CHECK_MODULES(LIBINPUT_BACKEND, [libinput >= 0.8.0])
> > >  PKG_CHECK_MODULES(COMPOSITOR, [$COMPOSITOR_MODULES])
> > >  
> > > +PKG_CHECK_MODULES(WAYLAND_PROTOCOLS, [wayland-protocols >= 0.1.0],
> > 
> > Please also update RELEASING with a sentence or two about needing to
> > check and update this protocol package version number.
> 
> Will do.
> 
> 
> Jonas

Bryce