Re: [PATCH] getopt: fix diagnostic for missing mandatory option argument

[Jim Meyering]

Stefano Lattarini wrote:
> On 01/15/2012 09:35 AM, Stefano Lattarini wrote:
>> On 01/15/2012 09:18 AM, Stefano Lattarini wrote:
>>> On 01/15/2012 06:59 AM, Paul Eggert wrote:
>>>> Thanks, that patch looks good to me; please install.
>>>>
>>> Done.
>>>
>> And introduced a new bug in doing so, sigh.  The attached patch
>> should fix it.  OK to apply?
>
> And as a follow-up, another nice simplification which also offer some
> bug fixes for free.  OK to apply?

That looks like a fine change.

>  ChangeLog               |   33 +++++++++++++++++++++++++++++++++
>  lib/Autom4te/General.pm |   10 +---------
>  2 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index b5af5bb..0832b8a 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,38 @@
>  2011-01-15  Stefano Lattarini  <address@hidden>
>
> +     getopt: remove hack for special handling of "-" argument
> +
> +     Older versions of Getopt::Long acted bogusly and died when they
> +     where configured with the 'bundling' flag and an argument '-' was
> +     seen on the command line they were parsing.  This is not the case

Grammar tweak: s/This is.*has/

  That is no longer the case though, and has

> +     today anymore though, and has not been for quite a long time: the
> +     bug is no more present in the 5.6.2 version of perl and the 2.25

s/no more/no longer/

> +     version of Getopt::Long (and today, the latest versions of perl
> +     and Getopt::Long are respectively 5.14.2 and 2.38).  The obsolete
> +     workaround for that Getopt::Long bug can thus be removed from our
> +     'getopt' function.
> +
> +     It is also worth noting that such a workaround was quite buggy
> +     and brittle itself; for example, a command like this:
> +       "autom4te --output -"
> +     would have caused the incorrect diagnostic:
> +       "autom4te: option `--output' requires an argument"
> +     Much worse, a command like this:
> +       "autom4te --language=autoconf --output - configure.ac"
> +     would have caused the standard input of autom4te to be processed
> +     and copied into the 'configure.ac' file, deleting its pre-existing
> +     content!  Surely not what a user would have expected.
> +
> +     After this change, a command like this:
> +       autom4te --language=autoconf --output - - <configure.ac >out
> +     works as expected, processing the input from 'configure.ac' and
> +     writing it to the 'out' file.
> +
> +     * lib/Autom4te/General.pm (use): Require perl version 5.6.2.
> +     (getopt): Remove the old workaround.
> +
> +2011-01-15  Stefano Lattarini  <address@hidden>
> +
>       getopt: avoid perl warning upon incorrect command line usages
>       After the previous change 'v2.68-113-g5bc3e85', incorrect
>       command line usages (like autom4te being called without any
> diff --git a/lib/Autom4te/General.pm b/lib/Autom4te/General.pm
> index 1be4f90..3355a49 100644
> --- a/lib/Autom4te/General.pm
> +++ b/lib/Autom4te/General.pm
> @@ -32,7 +32,7 @@ used in several executables of the Autoconf and Automake 
> packages.
>
>  =cut
>
> -use 5.005_03;
> +use 5.006_002;
>  use Exporter;
>  use Autom4te::ChannelDefs;
>  use Autom4te::Channels;
> @@ -244,16 +244,11 @@ rejecting it as a broken option.
>  # getopt (%OPTION)
>  # ----------------
>  # Handle the %OPTION, plus all the common options.
> -# Work around Getopt bugs wrt `-'.
>  sub getopt (%)
>  {
>    my (%option) = @_;
>    use Getopt::Long;
>
> -  # F*k.  Getopt seems bogus and dies when given `-' with `bundling'.
> -  # If fixed some day, use this: '' => sub { push @ARGV, "-" }
> -  my $stdin = grep /^-$/, @ARGV;
> -  @ARGV = grep !/^-$/, @ARGV;
>    %option = ("h|help"     => sub { print $help; exit 0 },
>            "V|version"  => sub { print $version; exit 0 },
>
> @@ -297,9 +292,6 @@ sub getopt (%)
>       }
>      }
>
> -  push @ARGV, '-'
> -    if $stdin;
> -
>    setup_channel 'note', silent => !$verbose;
>    setup_channel 'verb', silent => !$verbose;
>  }