On Thu, Nov 05, 2015 at 09:00:21PM -0800, Bryce Harrington wrote: > 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 <[email protected]> > > > > > > > > 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.
I see your points. I suppose zwp_..._vN does have more benefits than zwp_...N, so I'm up for changing. > > 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. Yea, not so sure about putting it in the beginning for the reasons you already pointed out. Jonas > > > 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 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
