On Mon, 2 May 2016 12:27:50 +0200 Armin Krezović <[email protected]> wrote:
> This adds a command line argument to print wayland-scanner version. > > It also makes wayland-scanner emit a comment with wayland library > version to every file it generates. > > Signed-off-by: Armin Krezović <[email protected]> > --- > src/scanner.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) Hi Armin, this is nice. Some tiny details below, otherwise looking good. > > diff --git a/src/scanner.c b/src/scanner.c > index 52c07a6..93c4204 100644 > --- a/src/scanner.c > +++ b/src/scanner.c > @@ -26,6 +26,7 @@ > */ > > #include "config.h" > +#include "wayland-version.h" > > #include <stdbool.h> > #include <stdio.h> > @@ -64,12 +65,21 @@ usage(int ret) > "headers, server headers, or protocol marshalling > code.\n\n"); > fprintf(stderr, "options:\n"); > fprintf(stderr, " -h, --help display this help and > exit.\n" > + " -v, --version print the wayland > library version that\n" > + " the scanner was built > against.\n" > " -c, --include-core-only include the core > version of the headers,\n" > " that is e.g. > wayland-client-core.h instead\n" > " of > wayland-client.h.\n"); > exit(ret); > } > > +static int > +scanner_version(int ret) > +{ > + fprintf(stderr, "wayland-scanner %s\n", WAYLAND_VERSION); > + exit(ret); > +} Usage may be used with error-exit, but this never, so the argument is not necessary. However, the EXIT_SUCCESS in the call is a strong suggestion that this call will exit, so it's good for documentation. Well, that's all just cosmetics. Fine as is, too. > + > static bool > is_dtd_valid(FILE *input, const char *filename) > { > @@ -1420,6 +1430,8 @@ emit_header(struct protocol *protocol, enum side side) > const char *s = (side == SERVER) ? "SERVER" : "CLIENT"; > char **p, *prev; > > + printf("/* Generated by wayland-scanner %s */\n\n", WAYLAND_VERSION); > + > printf("#ifndef %s_%s_PROTOCOL_H\n" > "#define %s_%s_PROTOCOL_H\n" > "\n" > @@ -1621,6 +1633,8 @@ emit_code(struct protocol *protocol) > struct wl_array types; > char **p, *prev; > > + printf("/* Generated by wayland-scanner %s */\n\n", WAYLAND_VERSION); > + > if (protocol->copyright) > format_text_to_comment(protocol->copyright, true); > > @@ -1698,7 +1712,7 @@ int main(int argc, char *argv[]) > char *input_filename = NULL; > int len; > void *buf; > - bool help = false, core_headers = false; > + bool help = false, core_headers = false, version = false; I'd prefer a whole new statement. > bool fail = false; > int opt; > enum { > @@ -1710,11 +1724,12 @@ int main(int argc, char *argv[]) > static const struct option options[] = { > { "help", no_argument, NULL, 'h' }, > { "include-core-only", no_argument, NULL, 'c' }, > + { "version", no_argument, NULL, 'v' }, > { 0, 0, NULL, 0 } > }; > > while (1) { > - opt = getopt_long(argc, argv, "hc", options, NULL); > + opt = getopt_long(argc, argv, "hvc", options, NULL); > > if (opt == -1) > break; > @@ -1723,6 +1738,9 @@ int main(int argc, char *argv[]) > case 'h': > help = true; > break; > + case 'v': > + version = true; > + break; > case 'c': > core_headers = true; > break; > @@ -1737,10 +1755,14 @@ int main(int argc, char *argv[]) > > if (help) > usage(EXIT_SUCCESS); > + else if (version) > + scanner_version(EXIT_SUCCESS); Ok. > else if ((argc != 1 && argc != 3) || fail) > usage(EXIT_FAILURE); > else if (strcmp(argv[0], "help") == 0) > usage(EXIT_SUCCESS); > + else if (strcmp(argv[0], "version") == 0) > + scanner_version(EXIT_SUCCESS); This is not necessary. I think it is enough if we have 'wayland-scanner -v' and 'wayland-scanner --version' working, we don't need 'wayland-scanner version' too. > else if (strcmp(argv[0], "client-header") == 0) > mode = CLIENT_HEADER; > else if (strcmp(argv[0], "server-header") == 0) So why do we have this old style arguments here? I suppose because they are not modifiers per se, but one of them is mandatory on each invocation to generate anything. Thanks, pq
pgpoqAseNCXLw.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
