Re: FYI: 6/ Automake::Variable & Automake::VarDef

[Raja R Harinath]

Hi,

Alexandre Duret-Lutz <address@hidden> writes:

> I'm installing this.  This moves approximately 1000 lines out of
> automake.in.  There still remains a few variable-related
> functions that should be moved to Automake::Variable (all the
> recursive traversal things, I think).  I'll do this next week.

Nice.  

A few comments:

[snip]
> Index: lib/Automake/Variable.pm
[snip]
> +sub reset ()
> +{
> +  %_variable_dict = ();
> +  %_appendvar = ();
> +  @_var_order = ();
> +}

I think this should reset %_hooks too.

> +=item C<Automake::Variable::define($varname, $owner, $type, $cond, $value, 
> $comment, $where, $pretty)>
> +
> +Define or append to a new variable.
> +
> +C<$varname>: the name of the variable being defined.
> +
> +C<$owner>: owner of the variable (one of C<VAR_MAKEFILE>,
> +C<VAR_CONFIGURE>, or C<VAR_AUTOMAKE>, defined by L<Automake::VarDef>).
> +Variables can be overriden, provided the new owner is not weaker
> +(C<VAR_AUTOMAKE> < C<VAR_CONFIGURE> < C<VAR_MAKEFILE>).
> +
> +C<$type>: the type of the assignment (C<''> for C<FOO = bar>,
> +C<':'> for C<FOO := bar>, and C<'+'> for C<'FOO += bar'>).
> +
> +C<$cond>: the DisjConditions in which C<$var> is being defined.
                 ^^^^^^^^^^^^^^

???  Isn't this the Automake::Condition.

[snip]
> +sub variable_defined ($;$)

I think this belongs back to 'automake.in'.  That'd be the right
place to encode the information about Automake::Rule, when you add it.

> +sub variable_assert ($$)
> +sub examine_variable ($)
> +sub require_variables ($$$@)
> +sub require_variables_for_variable ($$@)
> +sub variable_value ($)
> +sub variable_value_as_list($$;$)

I think these belong to automake.in, too.  They don't seem to fit the
mold.  It's a tossup, but I think these are more peculiar to the way
Automake uses the variables, than about implementing the variables.
That's not really a distinction :-), but ...

> +=item C<$str = output ($var, address@hidden)>
> +
> +Format all the definitions of C<$var> if C<@cond> is not specified,
> +else only that corresponding to C<@cond>.
> +
> +=cut
> +
> +sub output ($@)
> +{
> +  my ($var, @conds) = @_;
> +
> +  $var = ref ($var) ? $var : rvar ($var);

Since this is used only as a method, this seems redundant.  Document
this as $self->output(address@hidden) instead?

- Hari
-- 
Raja R Harinath ------------------------------ address@hidden