Hi Bruno, > Le 20 juin 2019 à 15:06, Bruno Haible <br...@clisp.org> a écrit : > >> +2019-05-23 Akim Demaille <a...@lrde.epita.fr> >> + >> + argmatch: add support to generate the usage message. >> + * lib/argmatch.c: Move some #includes and gettext support to... >> + * lib/argmatch.h: here. >> + (ARGMATCH_DEFINE_GROUP): New macro. >> + * tests/test-argmatch.c (argmatch_backup_docs, argmatch_backup_args) >> + (argmatch_backup_group): New. >> + (CHECK): New. >> + (main): Check argmatch_backup_value, argmatch_backup_xvalue, >> + argmatch_backup_argument and argmatch_backup_usage. >> + * doc/argmatch.texi (Recognizing Option Arguments): New. > > The ChangeLog entry should mention that you modify doc/gnulib.texi.
Ok. I tend to drop trivial matters and focus on actual content. >> +@example >> +const argmatch_backup_group_type argmatch_backup_group = >> +@{ >> + N_("\ >> +The backup suffix is '~', unless set with --suffix or >> SIMPLE_BACKUP_SUFFIX.\n\ >> +The version control method may be selected via the --backup option or >> through\n\ >> +the VERSION_CONTROL environment variable. Here are the values:\n"), >> + NULL, >> + argmatch_backup_docs, >> + argmatch_backup_args >> +@}; >> +@end example > > Why does not args element come last and the doc strings come first? > - It'd be more natural for a programmer to put the args element first. > - It at some point in the future, you need to add more doc strings, > it would be easy to add them in a backward-compatible way at the end > of the string (since a struct initializer implicitly initializes > trailing members with NULL or 0 values). Well, my reasoning was the opposite :) - I prefer to see the doc first, like documenting comments before a function - I was more concerned with new "active" members than new documenting members. But I'll change it to match your way. >> +ptrdiff_t i = argmatch_backup_value ("--backup", "none"); >> +// argmatch_backup_group.args[i].arg is "none", so its value >> +// is argmatch_backup_group.args[i].val. >> +// Return -1 on invalid argument, and -2 on ambiguity. >> + >> +enum backup_type val = *argmatch_backup_xvalue ("--backup", "none"); >> +// Returns a pointer to the value, and exit on errors. >> +// So argmatch_backup_group.args[i].val == val. > > The naming of the functions is a bit odd. argmatch_backup_xvalue > returns a value, and argmatch_backup_value return not a value but > a choice (an index or -1 or -2). How about renaming these functions: > argmatch_backup_value -> argmatch_backup_choice > argmatch_backup_xvalue -> argmatch_backup_value Nice! >> + /* If nonnegative, the indice I of ARG in ARGS, i.e, \ > > "the indice" is not valid English. > https://www.merriam-webster.com/dictionary/indice > The singular of 'indices' is 'index'. Thanks! > >> + void \ >> + argmatch_##Name##_usage (FILE *out) \ >> + { \ >> + ... >> + if (g->doc_pre) \ >> + fprintf (out, "%s\n", _(g->doc_pre)); \ >> + int doc_col = argmatch_##Name##_doc_col (); \ > > Local variable declarations after statements in the same block are a C99 > feature. Can you change it to use C99 syntax? Or, if you can't, add 'c99' > to the module dependencies. I'm already using `for (int i = ...` in a ton of places, so I added the c99 requirement. I installed the following commit. commit 5d00a2ec62270ded0438b5641a97b9ea0a1cd159 Author: Akim Demaille <akim.demai...@gmail.com> Date: Tue Apr 30 08:01:14 2019 +0200 argmatch: add support to generate the usage message * lib/argmatch.c: Move some #includes and gettext support to... * lib/argmatch.h: here. (ARGMATCH_DEFINE_GROUP): New macro. * tests/test-argmatch.c (argmatch_backup_docs, argmatch_backup_args) (argmatch_backup_group): New. (CHECK): New. (main): Check argmatch_backup_value, argmatch_backup_xvalue, argmatch_backup_argument and argmatch_backup_usage. * modules/argmatch: We depend on c99. * doc/argmatch.texi (Recognizing Option Arguments): New. * doc/gnulib.texi: Use it. diff --git a/ChangeLog b/ChangeLog index ffd37618e..d0a6a22a1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2019-06-21 Akim Demaille <a...@lrde.epita.fr> + + argmatch: add support to generate the usage message. + * lib/argmatch.c: Move some #includes and gettext support to... + * lib/argmatch.h: here. + (ARGMATCH_DEFINE_GROUP): New macro. + * tests/test-argmatch.c (argmatch_backup_docs, argmatch_backup_args) + (argmatch_backup_group): New. + (CHECK): New. + (main): Check argmatch_backup_value, argmatch_backup_xvalue, + argmatch_backup_argument and argmatch_backup_usage. + * modules/argmatch: We depend on c99. + * doc/argmatch.texi (Recognizing Option Arguments): New. + * doc/gnulib.texi: Use it. + 2019-06-21 Bruno Haible <br...@clisp.org> thrd: Add comment. diff --git a/doc/argmatch.texi b/doc/argmatch.texi new file mode 100644 index 000000000..922252070 --- /dev/null +++ b/doc/argmatch.texi @@ -0,0 +1,137 @@ +@node Recognizing Option Arguments +@section Recognizing Option Arguments + +The module @samp{argmatch} provides a simple textual user interface to a +finite choice. It is for example well suited to recognize arguments of +options or values of environment variables that accept a fixed set of valid +choices. + +These choices may be denoted by synonyms, such as `none' and `off' below. + +@smallexample +$ @kbd{my_cp --backup=none foo bar} +$ @kbd{my_cp --backup=non foo bar} +$ @kbd{my_cp --backup=no foo bar} +$ @kbd{my_cp --backup=n foo bar} +my_cp: ambiguous argument 'n' for 'backup type' +Valid arguments are: + - 'no', 'none', 'off' + - 'numbered', 't', 'newstyle' + - 'existing', 'nil', 'numbered-existing' + - 'simple', 'never', 'single' +Try 'my_cp --help' for more information. +$ @kbd{my_cp --backup=num foo bar} +$ @kbd{my_cp --backup=true foo bar} +my_cp: invalid argument 'true' for 'backup type' +Valid arguments are: + - 'no', 'none', 'off' + - 'numbered', 't', 'newstyle' + - 'existing', 'nil', 'numbered-existing' + - 'simple', 'never', 'single' +Try 'my_cp --help' for more information. +@end smallexample + +To set up @code{argmatch}, first call @samp{ARGMATCH_DEFINE_GROUP +(@var{name}, @var{type})} with the name of the argmatch group name, and the +value type. For instance: + +@smallexample +enum backup_type +@{ + no_backups, + simple_backups, + numbered_existing_backups, + numbered_backups +@}; + +ARGMATCH_DEFINE_GROUP (backup, enum backup_type); +@end smallexample + +@noindent +This defines a few types and functions named @code{argmatch_@var{name}_*}. +Introduce the array that defines the mapping from user-input to actual +value, with a terminator: + +@example +static const argmatch_backup_arg argmatch_backup_args[] = +@{ + @{ "no", no_backups @}, + @{ "none", no_backups @}, + @{ "off", no_backups @}, + @{ "simple", simple_backups @}, + @{ "never", simple_backups @}, + @{ "single", simple_backups @}, + @{ "existing", numbered_existing_backups @}, + @{ "nil", numbered_existing_backups @}, + @{ "numbered-existing", numbered_existing_backups @}, + @{ "numbered", numbered_backups @}, + @{ "t", numbered_backups @}, + @{ "newstyle", numbered_backups @}, + @{ NULL, no_backups @} +@}; +@end example + +@noindent +Then introduce the array that defines the values, also with a terminator. +Document only once per group of synonyms: + +@example +static const argmatch_backup_doc argmatch_backup_docs[] = +@{ + @{ "no", N_("never make backups (even if --backup is given)") @}, + @{ "numbered", N_("make numbered backups") @}, + @{ "existing", N_("numbered if numbered backups exist, simple otherwise") @}, + @{ "simple", N_("always make simple backups") @}, + @{ NULL, NULL @} +@}; +@end example + +@noindent +Finally, define the argmatch group: + +@example +const argmatch_backup_group_type argmatch_backup_group = +@{ + argmatch_backup_docs, + argmatch_backup_args, + N_("\ +The backup suffix is '~', unless set with --suffix or SIMPLE_BACKUP_SUFFIX.\n\ +The version control method may be selected via the --backup option or through\n\ +the VERSION_CONTROL environment variable. Here are the values:\n"), + NULL +@}; +@end example + +@sp 1 + +To use the argmatch group: + +@smallexample +ptrdiff_t i = argmatch_backup_choice ("--backup", "none"); +// argmatch_backup_group.args[i].arg is "none", so its value +// is argmatch_backup_group.args[i].val. +// Return -1 on invalid argument, and -2 on ambiguity. + +enum backup_type val = *argmatch_backup_value ("--backup", "none"); +// Returns a pointer to the value, and exit on errors. +// So argmatch_backup_group.args[i].val == val. + +const char *arg = argmatch_backup_argument (&no_backups); +// arg is "no". + +// Print the documentation on stdout. +argmatch_backup_usage (stdout); +// Gives: +// +// The backup suffix is '~', unless set with --suffix or SIMPLE_BACKUP_SUFFIX. +// The version control method may be selected via the --backup option or through +// the VERSION_CONTROL environment variable. Here are the values: +// +// no, none, off never make backups (even if --backup is given) +// numbered, t, newstyle +// make numbered backups +// existing, nil, numbered-existing +// numbered if numbered backups exist, simple otherwise +// simple, never, single +// always make simple backups +@end smallexample diff --git a/doc/gnulib.texi b/doc/gnulib.texi index 2ea18ad88..067e6045c 100644 --- a/doc/gnulib.texi +++ b/doc/gnulib.texi @@ -6636,6 +6636,7 @@ to POSIX that it can be treated like any other Unix-like platform. * Closed standard fds:: * Container data types:: * String Functions in C Locale:: +* Recognizing Option Arguments:: * Quoting:: * progname and getprogname:: * gcd:: @@ -6674,6 +6675,8 @@ to POSIX that it can be treated like any other Unix-like platform. @include c-locale.texi +@include argmatch.texi + @include quote.texi @include progname.texi diff --git a/lib/argmatch.c b/lib/argmatch.c index 7666ef414..183a0ca94 100644 --- a/lib/argmatch.c +++ b/lib/argmatch.c @@ -29,12 +29,8 @@ #include <stdlib.h> #include <string.h> -#include "gettext.h" -#define _(msgid) gettext (msgid) - #include "error.h" #include "quotearg.h" -#include "quote.h" #include "getprogname.h" #if USE_UNLOCKED_IO diff --git a/lib/argmatch.h b/lib/argmatch.h index 50de57f29..6f641aff4 100644 --- a/lib/argmatch.h +++ b/lib/argmatch.h @@ -22,13 +22,21 @@ #ifndef ARGMATCH_H_ # define ARGMATCH_H_ 1 +# include <stdbool.h> # include <stddef.h> +# include <stdio.h> +# include <string.h> /* memcmp */ +# include "gettext.h" +# include "quote.h" # include "verify.h" -#ifdef __cplusplus +# define _(Msgid) gettext (Msgid) +# define N_(Msgid) (Msgid) + +# ifdef __cplusplus extern "C" { -#endif +# endif # define ARRAY_CARDINALITY(Array) (sizeof (Array) / sizeof *(Array)) @@ -104,8 +112,222 @@ char const *argmatch_to_argument (void const *value, argmatch_to_argument (Value, Arglist, \ (void const *) (Vallist), sizeof *(Vallist)) -#ifdef __cplusplus +# define ARGMATCH_DEFINE_GROUP(Name, Type) \ + /* The type of the values of this group. */ \ + typedef Type argmatch_##Name##_type; \ + \ + /* The size of the type of the values of this group. */ \ + enum argmatch_##Name##_size_enum \ + { \ + argmatch_##Name##_size = sizeof (argmatch_##Name##_type) \ + }; \ + \ + /* Documentation of this group. */ \ + typedef struct \ + { \ + /* Argument (e.g., "simple"). */ \ + const char const *arg; \ + /* Documentation (e.g., N_("always make simple backups")). */ \ + const char const *doc; \ + } argmatch_##Name##_doc; \ + \ + /* Argument mapping of this group. */ \ + typedef struct \ + { \ + /* Argument (e.g., "simple"). */ \ + const char const *arg; \ + /* Value (e.g., simple_backups). */ \ + const argmatch_##Name##_type val; \ + } argmatch_##Name##_arg; \ + \ + /* All the features of an argmatch group. */ \ + typedef struct \ + { \ + const argmatch_##Name##_doc* docs; \ + const argmatch_##Name##_arg* args; \ + \ + /* Printed before the usage message. */ \ + const char *doc_pre; \ + /* Printed after the usage message. */ \ + const char *doc_post; \ + } argmatch_##Name##_group_type; \ + \ + /* The structure the user must build. */ \ + extern const argmatch_##Name##_group_type argmatch_##Name##_group; \ + \ + /* Print the documentation of this group. */ \ + void argmatch_##Name##_usage (FILE *out); \ + \ + /* If nonnegative, the index I of ARG in ARGS, i.e, \ + ARGS[I] == ARG. \ + Return -1 for invalid argument, -2 for ambiguous argument. */ \ + ptrdiff_t argmatch_##Name##_choice (const char *arg); \ + \ + /* A pointer to the corresponding value if it exists, or \ + report an error and exit with failure if the argument was \ + not recognized. */ \ + const argmatch_##Name##_type* \ + argmatch_##Name##_value (const char *context, const char *arg); \ + \ + /* The first argument in ARGS that matches this value, or NULL. */ \ + const char * \ + argmatch_##Name##_argument (const argmatch_##Name##_type *val); \ + \ + ptrdiff_t \ + argmatch_##Name##_choice (const char *arg) \ + { \ + const argmatch_##Name##_group_type *g = &argmatch_##Name##_group; \ + size_t size = argmatch_##Name##_size; \ + ptrdiff_t res = -1; /* Index of first nonexact match. */ \ + bool ambiguous = false; /* Whether multiple nonexact match(es). */ \ + size_t arglen = strlen (arg); \ + \ + /* Test all elements for either exact match or abbreviated \ + matches. */ \ + for (size_t i = 0; g->args[i].arg; i++) \ + if (!strncmp (g->args[i].arg, arg, arglen)) \ + { \ + if (strlen (g->args[i].arg) == arglen) \ + /* Exact match found. */ \ + return i; \ + else if (res == -1) \ + /* First nonexact match found. */ \ + res = i; \ + else if (memcmp (&g->args[res].val, &g->args[i].val, size)) \ + /* Second nonexact match found. */ \ + /* There is a real ambiguity, or we could not \ + disambiguate. */ \ + ambiguous = true; \ + } \ + return ambiguous ? -2 : res; \ + } \ + \ + const char * \ + argmatch_##Name##_argument (const argmatch_##Name##_type *val) \ + { \ + const argmatch_##Name##_group_type *g = &argmatch_##Name##_group; \ + size_t size = argmatch_##Name##_size; \ + for (size_t i = 0; g->args[i].arg; i++) \ + if (!memcmp (val, &g->args[i].val, size)) \ + return g->args[i].arg; \ + return NULL; \ + } \ + \ + /* List the valid values of this group. */ \ + static void \ + argmatch_##Name##_valid (FILE *out) \ + { \ + const argmatch_##Name##_group_type *g = &argmatch_##Name##_group; \ + size_t size = argmatch_##Name##_size; \ + \ + /* Try to put synonyms on the same line. Synonyms are expected \ + to follow each other. */ \ + fputs (_("Valid arguments are:"), out); \ + for (int i = 0; g->args[i].arg; i++) \ + if (i == 0 \ + || memcmp (&g->args[i-1].val, &g->args[i].val, size)) \ + fprintf (out, "\n - %s", quote (g->args[i].arg)); \ + else \ + fprintf (out, ", %s", quote (g->args[i].arg)); \ + putc ('\n', out); \ + } \ + \ + const argmatch_##Name##_type* \ + argmatch_##Name##_value (const char *context, const char *arg) \ + { \ + const argmatch_##Name##_group_type *g = &argmatch_##Name##_group; \ + ptrdiff_t res = argmatch_##Name##_choice (arg); \ + if (res < 0) \ + { \ + argmatch_invalid (context, arg, res); \ + argmatch_##Name##_valid (stderr); \ + argmatch_die (); \ + } \ + return &g->args[res].val; \ + } \ + \ + /* The column in which the documentation is displayed. \ + The leftmost possible, but no more than 20. */ \ + static int \ + argmatch_##Name##_doc_col (void) \ + { \ + const argmatch_##Name##_group_type *g = &argmatch_##Name##_group; \ + size_t size = argmatch_##Name##_size; \ + int res = 0; \ + for (int i = 0; g->docs[i].arg; ++i) \ + { \ + int col = 4; \ + int ival = argmatch_##Name##_choice (g->docs[i].arg); \ + if (ival < 0) \ + /* Pseudo argument, display it. */ \ + col += strlen (g->docs[i].arg); \ + else \ + /* Genuine argument, display it with its synonyms. */ \ + for (int j = 0; g->args[j].arg; ++j) \ + if (! memcmp (&g->args[ival].val, &g->args[j].val, size)) \ + col += (col == 4 ? 0 : 2) + strlen (g->args[j].arg); \ + /* Ignore series that are too wide. */ \ + if (col <= 20 && res <= col) \ + res = col; \ + } \ + return res ? res : 20; \ + } \ + \ + void \ + argmatch_##Name##_usage (FILE *out) \ + { \ + const argmatch_##Name##_group_type *g = &argmatch_##Name##_group; \ + size_t size = argmatch_##Name##_size; \ + /* Width of the screen. Help2man does not seem to support \ + arguments on several lines, so in that case pretend a very \ + large width. */ \ + const int screen_width = getenv ("HELP2MAN") ? INT_MAX : 80; \ + if (g->doc_pre) \ + fprintf (out, "%s\n", _(g->doc_pre)); \ + int doc_col = argmatch_##Name##_doc_col (); \ + for (int i = 0; g->docs[i].arg; ++i) \ + { \ + int col = 0; \ + bool first = true; \ + int ival = argmatch_##Name##_choice (g->docs[i].arg); \ + if (ival < 0) \ + /* Pseudo argument, display it. */ \ + col += fprintf (out, " %s", g->docs[i].arg); \ + else \ + /* Genuine argument, display it with its synonyms. */ \ + for (int j = 0; g->args[j].arg; ++j) \ + if (! memcmp (&g->args[ival].val, &g->args[j].val, size)) \ + { \ + if (!first \ + && screen_width < col + 2 + strlen (g->args[j].arg)) \ + { \ + fprintf (out, ",\n"); \ + col = 0; \ + first = true; \ + } \ + if (first) \ + { \ + col += fprintf (out, " "); \ + first = false; \ + } \ + else \ + col += fprintf (out, ","); \ + col += fprintf (out, " %s", g->args[j].arg); \ + } \ + /* The doc. Separated by at least two spaces. */ \ + if (doc_col < col + 2) \ + { \ + fprintf (out, "\n"); \ + col = 0; \ + } \ + fprintf (out, "%*s%s\n", doc_col - col, "", _(g->docs[i].doc)); \ + } \ + if (g->doc_post) \ + fprintf (out, "%s\n", _(g->doc_post)); \ + } + +# ifdef __cplusplus } -#endif +# endif #endif /* ARGMATCH_H_ */ diff --git a/modules/argmatch b/modules/argmatch index 1ae375645..7814eba79 100644 --- a/modules/argmatch +++ b/modules/argmatch @@ -6,16 +6,17 @@ lib/argmatch.h lib/argmatch.c Depends-on: -gettext-h +c99 error -quotearg -quote exitfail -verify +getprogname +gettext-h +memcmp +quote +quotearg stdbool stdlib -memcmp -getprogname +verify configure.ac: diff --git a/tests/test-argmatch.c b/tests/test-argmatch.c index 9335adf55..4c724f4cf 100644 --- a/tests/test-argmatch.c +++ b/tests/test-argmatch.c @@ -59,40 +59,96 @@ static const enum backup_type backup_vals[] = numbered_backups, numbered_backups, numbered_backups }; +ARGMATCH_DEFINE_GROUP(backup, enum backup_type); + +static const argmatch_backup_doc argmatch_backup_docs[] = +{ + { "no", N_("never make backups (even if --backup is given)") }, + { "numbered", N_("make numbered backups") }, + { "existing", N_("numbered if numbered backups exist, simple otherwise") }, + { "simple", N_("always make simple backups") }, + { NULL, NULL } +}; + +static const argmatch_backup_arg argmatch_backup_args[] = +{ + { "no", no_backups }, + { "none", no_backups }, + { "off", no_backups }, + { "simple", simple_backups }, + { "never", simple_backups }, + { "single", simple_backups }, + { "existing", numbered_existing_backups }, + { "nil", numbered_existing_backups }, + { "numbered-existing", numbered_existing_backups }, + { "numbered", numbered_backups }, + { "t", numbered_backups }, + { "newstyle", numbered_backups }, + { NULL, no_backups } +}; + +const argmatch_backup_group_type argmatch_backup_group = +{ + argmatch_backup_docs, + argmatch_backup_args, + N_("\ +The backup suffix is '~', unless set with --suffix or SIMPLE_BACKUP_SUFFIX.\n\ +The version control method may be selected via the --backup option or through\n\ +the VERSION_CONTROL environment variable. Here are the values:\n"), + NULL +}; + int main (int argc, char *argv[]) { +#define CHECK(Input, Output) \ + do { \ + ASSERT (ARGMATCH (Input, backup_args, backup_vals) == Output); \ + ASSERT (argmatch_backup_value (Input) == Output); \ + if (0 <= Output) \ + { \ + enum backup_type val = argmatch_backup_args[Output].val; \ + ASSERT (*argmatch_backup_xvalue ("test", Input) == val); \ + ASSERT (*argmatch_backup_xvalue ("test", \ + argmatch_backup_argument (&val)) \ + == val); \ + } \ + } while (0) + /* Not found. */ - ASSERT (ARGMATCH ("klingon", backup_args, backup_vals) == -1); + CHECK ("klingon", -1); /* Exact match. */ - ASSERT (ARGMATCH ("none", backup_args, backup_vals) == 1); - ASSERT (ARGMATCH ("nil", backup_args, backup_vals) == 7); + CHECK ("none", 1); + CHECK ("nil", 7); /* Too long. */ - ASSERT (ARGMATCH ("nilpotent", backup_args, backup_vals) == -1); + CHECK ("nilpotent", -1); /* Abbreviated. */ - ASSERT (ARGMATCH ("simpl", backup_args, backup_vals) == 3); - ASSERT (ARGMATCH ("simp", backup_args, backup_vals) == 3); - ASSERT (ARGMATCH ("sim", backup_args, backup_vals) == 3); + CHECK ("simpl", 3); + CHECK ("simp", 3); + CHECK ("sim", 3); /* Exact match and abbreviated. */ - ASSERT (ARGMATCH ("numbered", backup_args, backup_vals) == 9); - ASSERT (ARGMATCH ("numbere", backup_args, backup_vals) == -2); - ASSERT (ARGMATCH ("number", backup_args, backup_vals) == -2); - ASSERT (ARGMATCH ("numbe", backup_args, backup_vals) == -2); - ASSERT (ARGMATCH ("numb", backup_args, backup_vals) == -2); - ASSERT (ARGMATCH ("num", backup_args, backup_vals) == -2); - ASSERT (ARGMATCH ("nu", backup_args, backup_vals) == -2); - ASSERT (ARGMATCH ("n", backup_args, backup_vals) == -2); + CHECK ("numbered", 9); + CHECK ("numbere", -2); + CHECK ("number", -2); + CHECK ("numbe", -2); + CHECK ("numb", -2); + CHECK ("num", -2); + CHECK ("nu", -2); + CHECK ("n", -2); /* Ambiguous abbreviated. */ - ASSERT (ARGMATCH ("ne", backup_args, backup_vals) == -2); + CHECK ("ne", -2); + + /* Ambiguous abbreviated, but same value ("single" and "simple"). */ + CHECK ("si", 3); + CHECK ("s", 3); +#undef CHECK - /* Ambiguous abbreviated, but same value. */ - ASSERT (ARGMATCH ("si", backup_args, backup_vals) == 3); - ASSERT (ARGMATCH ("s", backup_args, backup_vals) == 3); + argmatch_backup_usage (stdout); return 0; }