bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: argmatch: accept perfect matches in documented arglists


From: Bruno Haible
Subject: Re: argmatch: accept perfect matches in documented arglists
Date: Sun, 19 May 2019 13:05:39 +0200
User-agent: KMail/5.1.3 (Linux/4.4.0-145-generic; KDE/5.18.0; x86_64; ; )

Hi Akim,

> >> +static const char *const backup_docs[] =
> >> +{
> >> +  N_("never make backups (even if --backup is given)"),
> >> +  N_("make numbered backups"),
> >> +  N_("numbered if numbered backups exist, simple otherwise"),
> >> +  N_("always make simple backups"),
> >> +  NULL
> >> +};
> >> +
> >> +static const enum backup_type backup_doc_vals[] =
> >> +{
> >> +  no_backups,
> >> +  simple_backups,
> >> +  numbered_existing_backups,
> >> +  numbered_backups
> >> +};
> > 
> > This is hard to maintain. Better put things next to each other that
> > belong together. Namely, can you put the enum value and each docstring
> > into a single struct?
> > 
> >    { no_backups, N_("never make backups (even if --backup is given)") },
> >    { simple_backups, N_("make numbered backups") },
> >    { numbered_existing_backups, N_("numbered if numbered backups exist, 
> > simple otherwise") },
> >    { numbered_backups, N_("always make simple backups") },
> 
> Yes, of course, it would be nicer, but argmatch does not force the size of 
> the values.  And I didn't want to force the user to provide an lvalue for 
> each single value.  So I'm using the same approach as the rest of argmatch: 
> pass a pointer to an array, and the size of the data.
> 
> We can provide another type restricted to ints for this common case though.

I can't really follow what you wrote here.

What I mean is that any facility where the programmer has to write two arrays
filled with items that belong together, in a 1:1 correspondence, is 
ill-designed.
It is just too easy to update one of the arrays and forget about the other one.

This problem is not something new; it is also evident from the test case
(test-argmatch.c):

  static const char *const backup_args[] =
  {
    "no", "none", "off",
    "simple", "never", "single",
    "existing", "nil", "numbered-existing",
    "numbered", "t", "newstyle",
    NULL
  };

  static const enum backup_type backup_vals[] =
  {
    no_backups, no_backups, no_backups,
    simple_backups, simple_backups, simple_backups,
    numbered_existing_backups, numbered_existing_backups, 
numbered_existing_backups,
    numbered_backups, numbered_backups, numbered_backups
  };

In the case of argmatch, the original design from 1990 was good (one array to
pass); the redesign from 1998-12-31 introduced this problem.

Before you add the documentation string feature to 'argmatch', it would be good
to have this design problem fixed in the first place. Either by adding new API
to the argmatch facility, or by creating a new, independent facility.

Bruno




reply via email to

[Prev in Thread] Current Thread [Next in Thread]