bug-gnulib
[Top][All Lists]
Advanced

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

Re: human interface change?!? [Re: xstrtol.h


From: Jim Meyering
Subject: Re: human interface change?!? [Re: xstrtol.h
Date: Sat, 04 Aug 2007 14:31:05 +0200

Paul Eggert <address@hidden> wrote:
...
> namely, the options to be prefixed with "--" need to be marked.
>
> Here's a proposed patch to address both problems.  It comes in two parts:
> the first to gnulib, the second to coreutils.  I haven't installed the
> gnulib part.

Elegantly twisted :-)
Thanks for working on that.

I'm a little leery of this argument-transposing change:

     xstrtol         The first two arguments of STRTOL_FATAL_ERROR
                     are now an option name and option argument
                     instead of an option argument and a type string,

All callers will have to be updated, else will emit bogus diagnostics.
But that's not a big deal.

Also in your gnulib NEWS, s/standard output/standard error/.

That looks fine for coreutils, as-is.  However, the OPT macros feel like
they're part of the xstrtol API.  And, considering other packages that
use xstrto* functions, what do you think about putting these OPT macros
in xstrtol.h?  Then others wouldn't have to maintain separate copies.
The file-global "opt_str_storage" could be hidden in a static inline
function with a static-scoped local.

Anyhow, that can wait for a separate delta.  You're welcome to
check in the gnulib changes -- assuming no one objects -- or I will.
Once they're in, I'll check in your coreutils changes as is.

> diff --git a/src/system.h b/src/system.h
> index 3c7f49d..cc97f2f 100644
> --- a/src/system.h
> +++ b/src/system.h
> @@ -592,3 +592,20 @@ emit_bug_reporting_address (void)
>       bugs (typically your translation team's web or email address).  */
>    printf (_("\nReport bugs to <%s>.\n"), PACKAGE_BUGREPORT);
>  }
> +
> +/* Use OPT_IDX to decide whether to return either a short option
> +   string "-C", or a long option string derived from LONG_OPTION.
> +   OPT_IDX is -1 if the short option C was used; otherwise it is an
> +   index into LONG_OPTIONS, which should have a name preceded by two
> +   '-' characters.  */
> +static char opt_str_storage[3] = {'-', 0, 0};
> +#define OPT_STR(opt_idx, c, long_options)    \
> +  ((opt_idx) < 0                             \
> +   ? (opt_str_storage[1] = c, opt_str_storage)       \
> +   : LONG_OPT_STR (opt_idx, long_options))
> +
> +/* Likewise, but assume OPT_IDX is nonnegative.  */
> +#define LONG_OPT_STR(opt_idx, long_options) ((long_options)[opt_idx].name - 
> 2)
> +
> +/* Define an option string that will be used with OPT_STR or LONG_OPT_STR.  
> */
> +#define OPT_STR_INIT(name) ("--" name + 2)




reply via email to

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