1. Use File::Find with File::Glob. One for digging into sub directories, the other one for matching .shtml files. Much less coding on your own.
2. When you read files, call read with a big buffer, to read in the whole shtml file by one call. This improves performance. Don't handle line by line, too slow. Memory is not an issue in your case(, unless you have huge shtml files). When you s///, use /g modifier.
3. Don't consider +< in this case, as your new content is shorter than the old one. (I am not saying you cannot use it, but using it in this case, requires more coding effort)

use File::Find;
use File::Glob ':glob';

use strict;

find(\&wanted, "c:/perl58/bin"); #replace with your directory

sub wanted {
    if ((-d $File::Find::name) && ($_ ne ".") && ($_ ne "..")) {
        my @shtml_files = bsd_glob("*.shtml");
        foreach my $shtml_file (@shtml_files) {
            print $shtml_file;
            open(SHTMLFILE, "<", $shtml_file);
            my $buffer;
            read(SHTMLFILE, $buffer, 10000); #give some big number, wh
+ich exceeds the size of all your .shtml files   
            close(SHTMLFILE); 
            $buffer =~ s/<a href="main\.php\?page=(.*?)\"/<a href="mai
+n.php?id=$1"/g;
            open(SHTMLFILE, ">", $shtml_file); 
            close(SHTMLFILE); 
        }
    }       
}
[download]

For each file or directory you find with File::Find, you're finding all of the files in its containing directory with File::Glob. You don't really need File::Glob.

I know this. The purpose of using File::Glob here is to reduce coding effort, so you don't need to match file name patterns on your own. When File::Glob can do this for you, what is the point to reinvent it?

I would agree File::Glob is too much a waste, if File::Find is improved to support patterns, and only return those entities, whose name match a certain pattern (ideally a regexp). But why the Perl community didn't do it? Reason is simple, because of the existance of File::Glob, there is no point to repeat/reinvent the same functionality in another class.

If you look at this from an OO view, it does make a lot of sense. Although File::Find and File::Glob might take care of different but tightly related tasks in some programs, they still should be abstracted as two different classes.

Consider using File::Find together with one of the HTML::Parser modules, like HTML::TokeParser. That will save you a lot of work and trouble.

After Compline,
Zaxo

Problem #1 : You are invoking opendir(DIR) recursively, meaning that closedir(DIR) will generate system errors on the way back up. Still, this should not present a problem, as you are invoking readdir(DIR) in list context before recursing, so your results should not be affected. Either move the closedir() immediately after the readdir(), or use a lexical (my $dir) instead of a global (DIR).

Problem #2 : The regexp /.shtml/i is not anchored to the end of string (/.shtml$/i or /.shtml\z/i), and the '.' is not escaped to make it literal (/\.shtml$/i or /\.shtml\z/i). Meaning - any file that contains the string "shtml" after the first character will match. Again, this is not probable, and the problem would be that too many files were processed, instead of too few, so this is not likely to be your problem.

Sorry... I don't see any obvious errors other than the above two. What behaviour are you seeing that you find to be unexpected?

Nothing unexpected, I just can't make any good way to replace every instance of an anchor href of "main.php?page=..." to "/?id=..." in every .shtml file.

---

“A script is what you give the actors. A program is what you give the audience.” ~ Larry Wall

The easiest solution (although not the most efficient) would be to invoke Perl from within Perl. For example, use the loops you already have defined to gather a set of path names into an array. Then, invoke:

system('perl', '-I.bak', '-pe', 's[main.php\?page=][/?id=]g', @pathnam
+es) == 0
    or die "Subcommand failed with return code $?.\n";
[download]

Try to make the regexp as accurate and complete as possible to avoid incorrect alterations. If you want a longer term solution that is a bit more efficient, see the perlrun manpage to see an example of the code that approximates the behaviour of -pI.bak.

NOTE: If the above system() invocation does not work, try changing 'perl' to be $^X ($EXECUTABLE_NAME with 'use English'). If the regular expression becomes very complicated, it may be easier to store the command in a perl script, and use the sub-script name instead of -e '...' in the system() invocation.

First open the file and read the content into a scalar
Then do a

$fileContent =~ s/(<a href=")main\.php\?page\=/$1\/\?id\=/g;
[download]

And then write the new contents back to the same file (or a new one with an added extension like .new or so

Would this not be a case where File::Find would be appropriate to find those files you need? Perhaps something along the lines of:

#
# UNTESTED CODE
#
use File::Find;

# If desired, change following to absolute path(s) to search
my @directories = ('.');
find(\&search_and_replace, @directories);

sub search_and_replace {
  # Item from search is a file
  -f && 
  # Item's full name ends with '.shtml'
  $File::Find::name =~ m/\.shtml$/ &&
  # Rename file so there is a backup
  rename($File::Find::name, $File::Find::name . '.bak');
  # Read from original (now .bak), writing to target
  open(INF, $File::Find::name . '.bak') 
      or die('Input: ', $!, "\n");
  open(OUTF, '>' . $File::Find::name) 
      or die('Output: ', $!, "\n");
  {
    # Localize $_ to prevent potential problems with call
    local($_);
    # Loop thru file, doing replacements
    while ($line = <INF>) {
      $line =~ s!(href="?)main\.page\?page=!$1/id=!g;
      print(OUTF $line);
    }
  }
  close(OUTF);
  close(INF);
}
[download]

Hope that the idea above at least helps.

Update: Added comments for clarification.

#!/usr/bin/perl -w
use strict;

use File::Find::Rule;
use HTML::TokeParser::Simple;

print "Processing:\n";

for my $file (File::Find::Rule->file->name('*.shtml')->in(@ARGV)) {
    print "\t$file,\n";

    rename $file, "$file.old" or die "Cannot rename $file to $file.old
+: $!";
    open my $fh, ">", $file or die "Cannot open $file for writing: $!"
+;

    my $p = HTML::TokeParser::Simple->new("$file.old");
    while (my $token = $p->get_token) {
        print $fh +(
           $token->is_start_tag('a') ?
               new_a_tag($token->return_attr, $token->return_attrseq)
               : $token->as_is
        );
    }
    close $fh;
}

print "done.\n";

sub new_a_tag {
    my ($attr, $attrseq) = @_;
    $attr->{href} =~ s</main\.php\?page=!></?id=!> if exists $attr->{h
+ref};
    map "<a $_>", join ' ', map qq<$_="$attr->{ $_ }">, @$attrseq;
}
[download]

Makeshifts last the longest.

Example:

# Try to match the expected input line format (from "ls" output)
#
if  ("$_" =~ /^\-.+ ([0-9]+) ([A-Z|a-z]+ [ ]?[0-9]+ [ ]?[:|0-9]+) (.+)
+$/)
    {

    # Set some defaults to avoid potentially problematic missing field
+s
    #
    $file1 = "FULLNAME";
    $file2 = "BASENAME";
    $fext  = "NO EXTENSION";

    # Set file size, date and compelte filename variables
    $fsize =$1;
    $date = $2;
    $file1 = $3;

    if ("$file1" =~/^([\.]?.+)\.(.+))$/)
         {
             $file2 = $1;
             $fext = $2;
              }
[download]

1) How many times do you plan to run this, let's be honest if your only going to run this once you don't need a perfectly efficient piece of code. even though I hate to admit that.

2) How many files and the size of the files you'll be reading in.

Then as you hit the line you could either do a s// or just replace the contents of the substring. I like to cheat with a sanity test since I substitute operations seem to always do bad things to me data.

if ($_ =~ /<a href="main.php?page/)
{
s/main.php?page=/main.php/?id=/g
}
[download]

Dave -- Saving the world one node at a time

Bad advice.

File::Find has been part of the core Perl distribution forever. If it isn't available on your host, it means their installation of Perl is incomplete. Complain to them and if they don't react, move to somewhere else. There is no excuse for not offering File::Find.

I feed in a ls -laR listing

How robust is your ls parsing pattern? And why not use find for the job of find? Something like the following does all you want, with minimal coding of your own. $ file . -type f -name '*.shtml' -print0 | xargs -0 ./myscript.pl Iterate over @ARGV using the diamond operator; it might even suffice to do something like $ file . -type f -name '*.shtml' -print0 | xargs -0 perl -i.old -pe's!/main\.php\?page=!/id=!g' See perldoc perlrun. Use the tools intended for your job to do your job, don't reinvent round wheels.

Makeshifts last the longest.

Hi Aristotle,

Had no idea File::Find was part of the core distribution. The reason I did not use a find function in my example was I had to parse each file in the filesystem.

Why? We had a 2 terrabyte file system from a litigation that we needed to type, index, hash and store in a mysql database. Then when we needed to find files of certain types, patterns, sizes, dates we could query a hashed index instead of running find each time. I agree that if you are looking for a certain type of file this is not the best idea, however in my situation it had to be done. However, if you know of a better way to do this, PPLLEEASSEE let me know.

Not the best advice, but it worked for me.

Dave -- Saving the world one node at a time