automake-patches
[Top][All Lists]
Advanced

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

Re: [PATCH 7/9] Warnings win over strictness in AUTOMAKE_OPTIONS.


From: Stefano Lattarini
Subject: Re: [PATCH 7/9] Warnings win over strictness in AUTOMAKE_OPTIONS.
Date: Fri, 14 Jan 2011 00:15:34 +0100
User-agent: KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

On Thursday 13 January 2011, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Tue, Jan 04, 2011 at 06:52:32PM CET:
> > Warnings win over strictness in AUTOMAKE_OPTIONS.
> > 
> > This change ensures that, for what concerns the options specified
> 
> s/This change ensures/Ensure/
>
OK (even if it disrupts line wrapping somewhat).

> > in AUTOMAKE_OPTIONS,  explicitly-defined warnings always take
> > precedence over implicit strictness-implied warnings.  This finally
> > fixes Automake bug#7669 a.k.a. PR/547.
> > 
> > * automake.in (handle_options): Call 'process_option_list' (from
> > Automake::Option) in way that ensures that explicit warnings are
> > parsed only after the strictness level has been set.
> 
> Call 'process_option_list' only once per set of options.
> (Shorter, and also clearer IMVHO.)
>
Agreed.

> > * tests/warnings-win-over-strictness.test: Extend.
> 
> > --- a/automake.in
> > +++ b/automake.in
> > @@ -1246,13 +1246,11 @@ sub handle_options
> >       msg_var ('unsupported', $var,
> >                "`AUTOMAKE_OPTIONS' cannot have conditional contents");
> >     }
> > -      foreach my $locvals ($var->value_as_list_recursive (cond_filter => 
> > TRUE,
> > -                                                     location => 1))
> > -   {
> > -     my ($where, $value) = @$locvals;
> > -     return 1 if process_option_list ({ option => $value,
> > -                                              where => $where});
> > -   }
> > +      my (@locvals, @options);
> > +      @locvals = $var->value_as_list_recursive (cond_filter => TRUE,
> > +                                           location => 1);
> > +      @options = map { { option => $_->[1], where => $_->[0] } } @locvals;
> 
> Why not just
> 
>          my @options = map { option => $_->[1], where => $_->[0] }
>                            $var->value_as_list_recursive (cond_filter => TRUE,
>                                                           location => 1);
> 
>
Because I'm never sure of how the perl parser really works or how smart
it is (and I've been bitten several times by unexpected behaviours), so
I usually prefer to err on the side of caution.  But your proposed
simplification works correctly, so I'll happily go with it.

> > +      return 1 if process_option_list (@options);
> >      }
> >  
> >    # Override portability-recursive warning.
> 
> OK with that fixed.
> 
> Thanks,
> Ralf
> 

Regards,
   Stefano



reply via email to

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