Processing variables

[blnukem]

Hi All

I use this script to update my website it is called via a second script that
uses strict. The problem is in the in the last routine "#Print the new web
page" section on the bottom the "foreach (@HtmlTemplate)" routine will only
process a "few" of the pages in the @MainLinks Array and then is stops. But
if I null out the entire "foreach (@HtmlTemplate)" routine and put print
"$HTMLPageData"; it will work fine but without the template data. I cant
seem to figure it out.

Thnaks in advance
blnukem

PERL CODE:

#!/usr/bin/perl -w

sub RebuildWebSite {

my ($DataBase,$PageType,$PageName,$LinkStatus);

######################################
# Get the master template
######################################
open (MASTERTEMPLATE,
"<$FolderPath/cgi-bin/data/webbuilder/templates/mastertemplate.dat") ||
print "ERROR: Could not open MASTERTEMPLATE
$FolderPath/cgi-bin/data/webbuilder/templates/mastertemplate.dat to read the
data: $!";
my @HtmlTemplate = <MASTERTEMPLATE>;
close(MASTERTEMPLATE);

######################################
# Read all of the sites pages into one array and reverse them
######################################
opendir (DIR,
"/usr/local/etc/httpd/htdocs/localhost/cgi-bin/data/webbuilder/sitelinks")
|| print "ERROR: Cannot open directory: $!";
@ARGV = map
"/usr/local/etc/httpd/htdocs/localhost/cgi-bin/data/webbuilder/sitelinks/$_"
, grep ! -d, readdir DIR;
closedir DIR;

@MainLinks = <>;
@MainLinks = reverse(@MainLinks);

print "@MainLinks"; #<-- For testing to make sure the array is complete.

foreach my $LinkName (@MainLinks) {
($DataBase,$PageType,$PageName,$LinkStatus) = split (/\|/, $LinkName);

$LinkStatus=~ s/\r//ig;
$LinkStatus =~ s/\n//ig;

######################################
# If the page is a blindlink skip it (for java menu).
######################################
if ($PageType eq "blindlink"){
next;
}

$LowercaseName = "$PageName";
$LowercaseName =~ s/\W//ig; # Remove all junk
$LowercaseName = lc($LowercaseName);

$HtmlPageToPrint = "$LowercaseName";

######################################
# We look for the index page
######################################
if ($PageType eq "index"){
$HtmlPageToPrint = "index";
}

######################################
# Get the text for the particular page in question
######################################
open (HTMLTEXT,
"<$FolderPath/cgi-bin/data/webbuilder/sitepages/$LowercaseName.dat") ||
print "ERROR: Could not open HTMLTEXT
$FolderPath/cgi-bin/data/webbuilder/sitepages/$LowercaseName.dat to read the
data: $!";
my @HtmlText = <HTMLTEXT>;
close(HTMLTEXT);

my $HTMLPageData = "@HtmlText"; #<-- $HTMLPageData is the varaible were
the text should go on the main template.

######################################
# Print the new web page #<-- Here is the trouble spot
######################################
open (HTLMLPAGE, ">$FolderPath/$HtmlPageToPrint.htm") || print "ERROR: Could
not open HTLMLPAGE $FolderPath/$HtmlPageToPrint.htm to print the data: $!";

foreach (@HtmlTemplate) {
my $xdata = $_;
$xdata =~ s/(\$\w+)/$1/eeg; # Process the variables
print HTLMLPAGE "$xdata";
}

close(HTLMLPAGE);

}# End of foreach
} # end of sub

1;

############# This also works fully but doesn't use template.
#
# #foreach (@HtmlTemplate) {
# #my $xdata = $_;
# #$xdata =~ s/(\$\w+)/$1/eeg; # Process the
variables
# #print HTLMLPAGE "$xdata";
# print HTLMLPAGE "$HTMLPageData";
# #}
# close(HTLMLPAGE);

 

[Ben Morrow]

I use this script to update my website it is called via a second script that
uses strict. The problem is in the in the last routine "#Print the new web
page" section on the bottom the "foreach (@HtmlTemplate)" routine will only
process a "few" of the pages in the @MainLinks Array and then is stops. But
if I null out the entire "foreach (@HtmlTemplate)" routine and put print
"$HTMLPageData"; it will work fine but without the template data. I cant
seem to figure it out.

PERL CODE:

#!/usr/bin/perl -w

use warnings is better than -w.
use strict will not have propagated from the other script: put it here
as well.

sub RebuildWebSite {

my ($DataBase,$PageType,$PageName,$LinkStatus);

Fix your indenting. Declare your variables in the smallest scope
possible: i.e. just before you use them.

######################################
# Get the master template
######################################

Girt big boxes like this are not helpful.

open (MASTERTEMPLATE,
"<$FolderPath/cgi-bin/data/webbuilder/templates/mastertemplate.dat")
|| print "ERROR: Could not open MASTERTEMPLATE
$FolderPath/cgi-bin/data/webbuilder/templates/mastertemplate.dat to read the
data: $!";
my @HtmlTemplate = <MASTERTEMPLATE>;
close(MASTERTEMPLATE);

Use or rather than ||; put the newlines in meaningful places; use
lexical FHs; use die for fatal errors as the message goes to stderr
and the program halts; don't *ever* type such a long string twice, use
a variable:

my $webbuilder = "$FolderPath/cgi-bin/data/webbuilder";
my $master_template = "$webbuilder/templates/mastertemplate.dat";

my @HtmlTemplate = do {
open my $MASTERTEMPLATE, '<', $master_template
or die "can't open $master_template: $!";

<$MASTERTEMPLATE>;
};

The file will be closed at the end of the do block.

######################################
# Read all of the sites pages into one array and reverse them
######################################

Comments should not explain what the code does: that should be clear
from the code. They should explain why.

opendir (DIR,
"/usr/local/etc/httpd/htdocs/localhost/cgi-bin/data/webbuilder/sitelinks")

Is this /usr/local/.../localhost the same as $FolderPath above, or
not? In either case, it should be in a variable.

|| print "ERROR: Cannot open directory: $!";
@ARGV = map
"/usr/local/etc/httpd/htdocs/localhost/cgi-bin/data/webbuilder/sitelinks/$_"
, grep ! -d, readdir DIR;
closedir DIR;

@MainLinks = <>;
@MainLinks = reverse(@MainLinks);

print "@MainLinks"; #<-- For testing to make sure the array is
complete.

Useless use of double-quotes: unless you specifically wanted $" rather
than $,?

foreach my $LinkName (@MainLinks) {
($DataBase,$PageType,$PageName,$LinkStatus) = split (/\|/, $LinkName);

$LinkStatus=~ s/\r//ig;
$LinkStatus =~ s/\n//ig;

Neither of those is alphabetic: the /i is useless.

######################################
# If the page is a blindlink skip it (for java menu).
######################################
if ($PageType eq "blindlink"){
next;
}

next if $PageType eq 'blindlink';

or

$PageType eq 'blindlink' and next;

according to taste.

$LowercaseName = "$PageName";

^^ my

$LowercaseName =~ s/\W//ig; # Remove all junk
$LowercaseName = lc($LowercaseName);

$HtmlPageToPrint = "$LowercaseName";

What exactly is the point of this?

######################################
# We look for the index page
######################################
if ($PageType eq "index"){
$HtmlPageToPrint = "index";
}

Oh, I see.

$HtmlPageToPrint = ($PageType eq 'index') ?
'index' :
$LowercaseName;

Or something... I'm never entirely sure how to indent those.

######################################
# Get the text for the particular page in question
######################################
open (HTMLTEXT,
"<$FolderPath/cgi-bin/data/webbuilder/sitepages/$LowercaseName.dat") ||
print "ERROR: Could not open HTMLTEXT
$FolderPath/cgi-bin/data/webbuilder/sitepages/$LowercaseName.dat to read the
data: $!";
my @HtmlText = <HTMLTEXT>;
close(HTMLTEXT);

my $HTMLPageData = "@HtmlText"; #<-- $HTMLPageData is the varaible were
the text should go on the main template.

What's wrong with doing

my $HTMLPageData = do { local $/; <HTMLTEXT> };

in the first place? (OK, that should be 'local $/ = $";', but I bet
it's undef.)

######################################
# Print the new web page #<-- Here is the trouble spot
######################################
open (HTLMLPAGE, ">$FolderPath/$HtmlPageToPrint.htm") || print "ERROR: Could
not open HTLMLPAGE $FolderPath/$HtmlPageToPrint.htm to print the data: $!";

foreach (@HtmlTemplate) {
my $xdata = $_;
$xdata =~ s/(\$\w+)/$1/eeg; # Process the variables

CAREFUL. You would be much better off using something like the
Template Toolkit for doing this; or at least use a real hash instead
of the symbol table:

my %templates;
# and then, instead of creating $HTMLPageData above,
$templates{HTMLPageData} = ...;

...

$xdata =~ s/\$(\w+)/$templates{$1} || ????/eg;

Note that this doesn't require /ee, which is dangerous as the template
can contain arditrary code; and also that it shows up templates you
haven't given a value to.

I have no idea where your original problem lies: I can't see anything
here that would cause it.

print HTLMLPAGE "$xdata";

Useless use of double quotes.

}

close(HTLMLPAGE);

}# End of foreach
} # end of sub

If you'd done your indenting properly, you wouldn't need these
comments.

Ben

 

[Tore Aursand]

#!/usr/bin/perl -w

Change this to:

#!/usr/bin/perl
#
use strict;
use warnings;

open (MASTERTEMPLATE,
"<$FolderPath/cgi-bin/data/webbuilder/templates/mastertemplate.dat") ||
print "ERROR: Could not open MASTERTEMPLATE
$FolderPath/cgi-bin/data/webbuilder/templates/mastertemplate.dat to read the
data: $!";

Why do you want to continue if you're not able to open this file? You
might also want to assign the _whole_ filename to a variable.

opendir (DIR,
"/usr/local/etc/httpd/htdocs/localhost/cgi-bin/data/webbuilder/sitelinks")
|| print "ERROR: Cannot open directory: $!";

Again; Opening this directory seems _very_ essential for _if_ your script
will work or not. Why do you continue if this operation fails?

@ARGV = map
"/usr/local/etc/httpd/htdocs/localhost/cgi-bin/data/webbuilder/sitelinks/$_"
, grep ! -d, readdir DIR;

Are you sure you want to assign to the @ARGV array?

@MainLinks = <>;
@MainLinks = reverse(@MainLinks);

Hmm. To me it seems like you only need the last line...?

($DataBase,$PageType,$PageName,$LinkStatus) = split (/\|/, $LinkName);

It's always nice to tell split how many you want returned;

split( /\|/, $LinkName, 4 );

$LinkStatus=~ s/\r//ig;
$LinkStatus =~ s/\n//ig;

No need to make this substitution case-insensitive;

$LinkStatus =~ s,\r,,g;
$LinkStatus =~ s,\n,,g;

if ($PageType eq "blindlink"){
next;
}

To me, this is easier to read:

next if ( $PageType eq 'blindlink' );

$LowercaseName = "$PageName";
$LowercaseName =~ s/\W//ig; # Remove all junk
$LowercaseName = lc($LowercaseName);

Could be written as:

$LowercaseName =~ s,\W,,g;
$LowercaseName = lc( $LowercaseName );

$HtmlPageToPrint = "$LowercaseName";

Don't use double quotes - or quotes at all - if you don't need to;

$HtmlPageToPrint = $LowercaseName;

And why do you use two different variables for the same data?

foreach (@HtmlTemplate) {
my $xdata = $_;
$xdata =~ s/(\$\w+)/$1/eeg; # Process the variables
print HTLMLPAGE "$xdata";
}

Could be written as:

foreach ( @HtmlTemplate ) {
s,(\$\w+),$1,eg; # Are you sure this works?
print HTLMLPAGE $_;
}