Re: [PATCH] git-cvsimport: add suport for CVS pserver method HTTP/1.x proxying

Thanks.  The patch looks very sane, isolated to be safe enough,
and useful.

Except that this statement made me go "huh?" wondering what it
would do to the $filehandle to evaluate <$filehandle> in a void
context:

+			# Skip the empty line of the proxy server output
+			<$s>;

and I ended up looking in perlop.pod and came up empty.

The "I/O Operators" section talks about evaluating <$s> in a
scalar context (i.e. "$rep = <$s>"), which we all know would
return a single line, and in list context, which swallows
everything up to EOF, an obvious disaster for this particular
use.  I couldn't find how it is defined to behave in a void
context.  By experiments I know this returns only one line, but
it leaves me feeling somewhat uneasy.

Also it has a style inconsistency between "if(expression) {" and
"if(expression){", and I do not like either of them, but fixing
that should be left to a separate patch.

I'll apply this unless Martin or other people on the list who
have stake in cvsimport objects.

Junio C Hamano wrote:

Except that this statement made me go "huh?" wondering what it
would do to the $filehandle to evaluate <$filehandle> in a void
context:

+			# Skip the empty line of the proxy server output
+			<$s>;

It's a perl idiom that will discard one line of the $filehandle. If we 
are 200% certain that it is empty, then it's fine. OTOH, it may well be 
a bug in the particular proxy implementation Iñaki is using -- I don't 
know enough about CVS proxying to tell.

The "I/O Operators" section talks about evaluating <$s> in a
scalar context (i.e. "$rep = <$s>"), which we all know would
return a single line, and in list context, which swallows

This is in scalar context, and that's safe to rely on. Whether it is 
clear enough in this non-Perl-native project... is a good flamewar 
waiting to happen :-)

cheers,


martin
-- 
-----------------------------------------------------------------------
Martin @ Catalyst .Net .NZ  Ltd, PO Box 11-053, Manners St,  Wellington
WEB: http://catalyst.net.nz/           PHYS: Level 2, 150-154 Willis St
OFFICE: +64(4)916-7224                              MOB: +64(21)364-017
       Make things as simple as possible, but no simpler - Einstein