Re: [PATCH 2/24] Documentation: Add variable-substitution script

[Jakub Narebski <jnareb@gmail.com>]

Thomas Rast <trast@student.ethz.ch> writes:

> diff --git a/Documentation/subst-config.perl b/Documentation/subst-config.perl
> new file mode 100755
> index 0000000..a981670
> --- /dev/null
> +++ b/Documentation/subst-config.perl
> @@ -0,0 +1,74 @@
> +#!/usr/bin/perl
> +use 5.006002;
> +use strict;
> +use warnings;
> +use Getopt::Long;
> +
> +Getopt::Long::Configure qw/ pass_through /;
> +
> +my $rc = GetOptions(
> +    "varlist=s" => \my $varlist,
> +    "input=s"   => \my $input,
> +    "output=s"  => \my $output,
> +);
> +
> +if (!$rc or (!-r $varlist or !-r $input)) {
> +    print "$0 --varlist=<varlist> --input=<in> --output=<out>\n";
> +    exit 1;
> +}
> +
> +my $vars = read_varlist($varlist);                       # [1]
> +substitute_variables($vars, $input, $output);            # [2]
> +exit 0;
> +
> +sub read_varlist {
> +    my ($file) = @_;
> +
> +    open my $fh, "<", $varlist or die "cannot open $varlist: $!";

NITPICK

You are passing global variable $varlist to this function as parameter
$file, see place marked [1].  Why don't you use there $file instead of
$varlist?  Alternatively, why don't you name parameter as $varlist?

> +sub substitute_variables {
> +    my ($varlist, $in, $out) = @_;
> +
> +    open my $infh, "<", $input or die "Can't open $in for reading: $!";
> +    open my $outfh, ">", $output or die "Can't open $out for reading: $!";

Same here: $input or $in, $output or $out?

> +
> +    while (<$infh>) {
> +        if (/^\@\@CONFIG\((\S+)\)\@\@$/) {
> +            my $v = lc $1;
> +            die "Key $v not documented" unless exists $varlist->{$v};
> +            print $outfh @{$varlist->{$v}};
> +            print $outfh "\n";
> +        } else {
> +            print $outfh $_;
> +        }
> +    }
> +
> +    close $infh or die "closing input failed: $!";
> +    close $outfh or die "closing output failed: $!";
> +
> +    return;
> +}

Also, could you consitently start error messages (in die "sth") with
capital letter (upper-case), or with lower-case letter?

-- 
Jakub Narebski
Poland
ShadeHawk on #git