Re: [css3-color] bug in table-based test

On Wednesday 2007-10-24 15:35 -0400, fantasai wrote:
> It's caused by using a generic XML parser without reading in the XHTML 
> entity files.
> I've written a patch to preprocess the files first (since getting the 
> parser module
> to read in the entities from a local file doesn't seem to be possible). Let 
> me know
> if it looks good and I'll check it in.

You're running the preprocessor over the copied files, not the
original source files, right?

I suppose this looks ok, although I tend to think we're better off
keeping the preprocessing to a minimum and just fixing the tests to
use   rather than   (which is what I was planning to do
before your patch).  It can be dangerous to change the preprocessing
for existing tests.

Did you diff the 2.1 test suite output before and after your changes
to make sure the changes make sense?  I'm a little worried about
some of the whitespace tests and things like that.  I suppose this
shouldn't really change anything, but it might in some user agents,
and there could be some tests testing both entities and raw
characters.

Some slightly more detailed (but not all that important) comments on
the code below.

-David

> +my $tempext = '.input';
> +
> +sub processFiles {
> +
> +  while (my $filename = shift @_) {
> +
> +    # Open for read/write
> +    rename $filename, "$filename$tempext";
> +    open IN, "$filename$tempext" or die "error reading disk: $!\n";
> +    open OUT, ">$filename" or die "error writing to disk: $!\n";
> +
> +    my $inBody = 0;
> +    while (<IN>) {
> +
> +      # Named Entity Substitution
> +      $_ = name2decimal_xml($_);
> +
> +      print OUT;
> +    }
> +
> +    # Close out
> +    close IN;
> +    close OUT;
> +    unlink "$filename$tempext";

In general (although it doesn't really matter here), the safer way
of replacing a file is to write the new contents to a location with
a different name, and then rename the oddly-named file back to the
original name.  Then again, "perl -pi" does what you do.  (And
mktemp may be a more robust way to create a temporary file name;
after all, ".input" could be used.  I've never used it in perl,
though.)

One other comment is that if you do "local $/;", which sets the
newline separator to empty, you can slurp in the whole file without
a while loop.  For example:

  local $/;
  my $text = <IN>;
  print OUT name2decimal_xml($text);

and probably even:

  local $/;
  print OUT name2decimal_xml(<IN>);

Then again, you could probably simplify the whole thing by using
"perl -pi" and just making preprocess.pl just act as a stream
editor, which I think would mean it would just be one line of code.

-David

-- 
L. David Baron                                 http://dbaron.org/
Mozilla Corporation                       http://www.mozilla.com/

Received on Tuesday, 30 October 2007 06:49:18 UTC