At 1:12 PM +0200 8/24/07, Jos I. Boumans wrote:
>On 21 Aug 2007, at 05:05, John E. Malmberg wrote:
>
>>In Extract.pm, upper case options to commands must be enclosed in double 
>>quotes.  This seems to only affect the UNZIP command.
>>
>>
>>In 01_Archive-Extract.t, to compare directory components in VMS syntax, the 
>>trailing "]" needs to be removed.
>
>>--- /rsync_root/perl/lib/Archive/Extract/t/01_Archive-Extract.t       Mon May 
>>28 07:33:43 2007
>>+++ lib/Archive/Extract/t/01_Archive-Extract.t        Thu Aug 16 23:06:18 2007
>>@@ -362,6 +362,10 @@
>>                         ### if something went wrong with determining the out
>>                         ### path, don't go deleting stuff.. might be Really 
>> Bad
>>                         my $out_re = quotemeta( $OutDir );
>>+
>>+                     # Remove the directory terminator from regex
>>+                     my $out_re = s/\\\]// if IS_VMS;
>>+
>>                         if( $ae->extract_path !~ /^$out_re/ ) {
>>                             ok( 0, "Extractpath WRONG 
>> (".$ae->extract_path.")");
>>                             skip(  "Unsafe operation -- skip cleanup!!!" ), 
>> 1;
>
>This doesn't seem to do at all what you intend -- it *redefines* $out_re
>to be the result of a substitution on $_... what did you mean to do exactly?
>
>And shouldn't this be done in the library rather than in the test suite?

Good catch.  It should of course be C<=~> rather than C<=>.  Somehow
I've managed to be both slow and hasty in my patch review and
application lately.  I will fix this and the bizarre use of
whitespace while I'm at it.  John, in the future, please try to
follow the use of spaces and tabs that the surrounding code uses.

As far as whether this should be in the library rather than the test,
let me explain what's happening and then you tell me.   As I
understand it, the test is trying to determine if C</disk1/foo/bar>
is part of C</disk1/foo/bar/baz>.  Except in VMS syntax, that would
mean trying to determine whether C<disk1:[foo.bar]> is part of
C<disk1:[foo.bar.baz]>.  Because we have both a directory delimiter
(dot) and a directory spec terminator (right bracket), we have to
trim the right bracket from the first one to make it successfully
match the second one.  Since we're asserting the same truth -- that
one path spec is the leading part of the other -- it seems to me ok
to have this in the test only.
-- 
________________________________________
Craig A. Berry
mailto:[EMAIL PROTECTED]

"... getting out of a sonnet is much more
 difficult than getting in."
                 Brad Leithauser

Reply via email to