[PATCH] git-mv.perl: use stderr for error output and cleanup

[PATCH] git-mv.perl: use stderr for error output and cleanup

[Alex Riesen]

[-- Attachment #1: Type: text/plain, Size: 142 bytes --]

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
It is cleaned up in the "Perl' meaning" of process: trivial loops
replaced with map{}

[-- Attachment #2: 0005-use-stderr-for-error-output-and-cleanup.txt --]
[-- Type: text/plain, Size: 3458 bytes --]

Subject: [PATCH] use stderr for error output and cleanup

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>


---

 git-mv.perl |   38 ++++++++++++++------------------------
 1 files changed, 14 insertions(+), 24 deletions(-)

487484f85df999ad949123800e1ab4a24458370b
diff --git a/git-mv.perl b/git-mv.perl
index 83dc7e4..5e7addf 100755
--- a/git-mv.perl
+++ b/git-mv.perl
@@ -37,7 +37,7 @@ if (-d $ARGV[$argCount-1]) {
 	# remove any trailing slash
 	$dstDir =~ s/\/$//;
 	@srcArgs = @ARGV[0..$argCount-2];
-	
+
 	foreach $src (@srcArgs) {
 		$base = $src;
 		$base =~ s/^.*\///;
@@ -47,10 +47,9 @@ if (-d $ARGV[$argCount-1]) {
 }
 else {
     if ($argCount != 2) {
-	print "Error: moving to directory '"
+	die "Error: moving to directory '"
 	    . $ARGV[$argCount-1]
-	    . "' not possible; not exisiting\n";
-	exit(1);
+	    . "' not possible; not existing\n";
     }
     @srcArgs = ($ARGV[0]);
     @dstArgs = ($ARGV[1]);
@@ -63,7 +62,7 @@ my (%overwritten, %srcForDst);
 
 $/ = "\0";
 open(F, 'git-ls-files -z |')
-        or die "Failed to open pipe from git-ls-files: " . $!;
+        or die "Failed to open pipe from git-ls-files: $!\n";
 
 @allfiles = map { chomp; $_; } <F>;
 close(F);
@@ -92,7 +91,7 @@ while(scalar @srcArgs > 0) {
 	if ($opt_f) {
 	    # only files can overwrite each other: check both source and destination
 	    if (-f $dst && (scalar @srcfiles == 1)) {
-		print "Warning: $bad; will overwrite!\n";
+		warn "Warning: $bad; will overwrite!\n";
 		$bad = "";
 		$overwritten{$dst} = 1;
 	    }
@@ -101,7 +100,7 @@ while(scalar @srcArgs > 0) {
 	    }
 	}
     }
-    
+
     if (($bad eq "") && ($dst =~ /^$safesrc\//)) {
 	$bad = "can not move directory '$src' into itself";
     }
@@ -124,11 +123,10 @@ while(scalar @srcArgs > 0) {
 
     if ($bad ne "") {
 	if ($opt_k) {
-	    print "Warning: $bad; skipping\n";
+	    warn "Warning: $bad; skipping\n";
 	    next;
 	}
-	print "Error: $bad\n";
-	exit(1);
+	die "Error: $bad\n";
     }
     push @srcs, $src;
     push @dsts, $dst;
@@ -146,7 +144,7 @@ while(scalar @srcs > 0) {
 	if (!rename($src,$dst)) {
 	    $bad = "renaming '$src' failed: $!";
 	    if ($opt_k) {
-		print "Warning: skipped: $bad\n";
+		warn "Warning: skipped: $bad\n";
 		$bad = "";
 		next;
 	    }
@@ -162,6 +160,7 @@ while(scalar @srcs > 0) {
     push @deletedfiles, @srcfiles;
     if (scalar @srcfiles == 1) {
 	# $dst can be a directory with 1 file inside
+
 	if ($overwritten{$dst} ==1) {
 	    push @changedfiles, $dstfiles[0];
 
@@ -189,30 +188,21 @@ else {
     if (@changedfiles) {
 	open(H, "| git-update-index -z --stdin")
 		or die "git-update-index failed to update changed files with code $!\n";
-	foreach my $fileName (@changedfiles) {
-		print H "$fileName\0";
-	}
+	print H map {"$_\0"} @changedfiles;
 	close(H);
     }
     if (@addedfiles) {
 	open(H, "| git-update-index --add -z --stdin")
 		or die "git-update-index failed to add new names with code $!\n";
-	foreach my $fileName (@addedfiles) {
-		print H "$fileName\0";
-	}
+	print H map {"$_\0"} @addedfiles;
 	close(H);
     }
     if (@deletedfiles) {
 	open(H, "| git-update-index --remove -z --stdin")
 		or die "git-update-index failed to remove old names with code $!\n";
-	foreach my $fileName (@deletedfiles) {
-		print H "$fileName\0";
-	}
+	print H map {"$_\0"} @deletedfiles;
 	close(H);
     }
 }
 
-if ($bad ne "") {
-    print "Error: $bad\n";
-    exit(1);
-}
+die "Error: $bad\n" if $bad ne "";
-- 
1.0.GIT

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> It is cleaned up in the "Perl' meaning" of process: trivial loops
> replaced with map{}

I agree with dies and warns, but I am slightly negative about
this one.

> -	foreach my $fileName (@deletedfiles) {
> -		print H "$fileName\0";
> -	}
> +	print H map {"$_\0"} @deletedfiles;

Although there is always one more way to do the same thing in
Perl, and there are good idioms that make well written Perl
programs very compact and clear, I tend to value readability for
(competent) programmers whose native language happens to be
something other than Perl.

I think using map{} to build a list to be printed without
inter-element spaces is far less intuitive than how the code was
originally written for non Perl readers.  Using join("\0", @df)
might be easier to read but you would then need terminating NUL,
and either one of the following are worse:

  do { local ($") = "\0"; print "@deletedfiles\0" } # this one is buggy
  do { local ($\) = "\0"; print for (@deletedfiles); } # impossible to read

So I'd prefer not touching for (@df) { print H "$_\n" } loops.

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Randal L. Schwartz]

>>>>> "Junio" == Junio C Hamano <junkio@cox.net> writes:

Junio> So I'd prefer not touching for (@df) { print H "$_\n" } loops.

Being as I'm a *bit* familiar with Perl, I'd write that as:

  print H "$_\0" for @deletedfiles;

if you want to write "for" as "foreach", I wouldn't complain either.
After all, that's spelled "f o r", but pronounced "foreach". :)

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Linus Torvalds]

On Fri, 6 Jan 2006, Randal L. Schwartz wrote:

> >>>>> "Junio" == Junio C Hamano <junkio@cox.net> writes:
> 
> Junio> So I'd prefer not touching for (@df) { print H "$_\n" } loops.
> 
> Being as I'm a *bit* familiar with Perl, I'd write that as:
> 
>   print H "$_\0" for @deletedfiles;
> 
> if you want to write "for" as "foreach", I wouldn't complain either.
> After all, that's spelled "f o r", but pronounced "foreach". :)

It is official: perl people are crazy.

I'm sorry, Randal, but you're clearly a totally different species.

I just hope you don't cross-breed. Shudder.

		Linus

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Randal L. Schwartz]

>>>>> "Linus" == Linus Torvalds <torvalds@osdl.org> writes:

Linus> I just hope you don't cross-breed. Shudder.

Don't worry.  So far, I haven't had any offers.

:-)

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Linus Torvalds]

On Fri, 6 Jan 2006, Randal L. Schwartz wrote:
> 
> Linus> I just hope you don't cross-breed. Shudder.
> 
> Don't worry.  So far, I haven't had any offers.

I was more worried about the "collective" of perl people, not any 
particular individual developer.

If it was just you, I could just poison you over lunch some time..

Which reminds me - you owe me lunch. A non-poisoned one.

		Linus

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

>> After all, that's spelled "f o r", but pronounced "foreach". :)
>
> It is official: perl people are crazy.

I sometimes pronounce "foreach" when I see "ef oh ar" in Perl
programs, but I do not do that aloud around other people.

My point was that things like this:

>>   print H "$_\0" for @deletedfiles;

is perfectly fine among Perl people (and I would probably even
encourage rewriting git-*.perl scripts in more Perlish style if
the project were about an SCM primarily targetted for Perl
people --- it is not), but look obfuscated without merit to
people who do not talk Perl.

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Alex Riesen]

Randal L. Schwartz, Fri, Jan 06, 2006 23:55:54 +0100:
> >>>>> "Junio" == Junio C Hamano <junkio@cox.net> writes:
> 
> Junio> So I'd prefer not touching for (@df) { print H "$_\n" } loops.
> 
> Being as I'm a *bit* familiar with Perl, I'd write that as:
> 
>   print H "$_\0" for @deletedfiles;
> 

Does not work for old Perl

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> Randal L. Schwartz, Fri, Jan 06, 2006 23:55:54 +0100:
>>
>>   print H "$_\0" for @deletedfiles;
>
> Does not work for old Perl

How old may I ask?

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Randal L. Schwartz]

>>>>> "Alex" == Alex Riesen <raa.lkml@gmail.com> writes:

Alex> Randal L. Schwartz, Fri, Jan 06, 2006 23:55:54 +0100:
>> >>>>> "Junio" == Junio C Hamano <junkio@cox.net> writes:
>> 
Junio> So I'd prefer not touching for (@df) { print H "$_\n" } loops.
>> 
>> Being as I'm a *bit* familiar with Perl, I'd write that as:
>> 
>> print H "$_\0" for @deletedfiles;
>> 

Alex> Does not work for old Perl

Correct.  It was added for Perl 5.5, first released on 22 July 1998.

Are you really saying you need this code to run on Perl 5.4?  There
are a number of other things that would have to be fixed as well.
(We had this conversation a while back.)

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Junio C Hamano]

merlyn@stonehenge.com (Randal L. Schwartz) writes:

>>> Being as I'm a *bit* familiar with Perl, I'd write that as:
>>> 
>>> print H "$_\0" for @deletedfiles;
>
> ...  It was added for Perl 5.5, first released on 22 July 1998.

BTW, I never understood the usefulness of Statement Modifiers.
Even reading the examples in perlsyn.pod:

    print "Basset hounds got long ears" if length $ear >= 10;
    go_outside() and play() unless $is_raining;

seeing "do these things" upfront and then realize "ah, but that
is done only when this holds true", interrupts the flow of
understanding while reading a program by somebody else [*1*].
It is worse if the Statement Modifier is a loop control.

(flamebait) Compound Statements take BLOCK and people who want
to do a one-liner could not do so without braces.  I've always
thought Statement Modifies as a lame workaround for that
problem.

[Footnote]

*1* This is true even for me; my mother language is Japanese,
which tends to hang negation at the tail of the sentence, and
double negation is not unusual.  This makes it impossible to
know what a sentence states before reading it to the end, so I
am used to that "coming back to the main part after reading
through the sentence", but I do not welcome that when I am
reading programs.

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Andreas Ericsson]

Junio C Hamano wrote:
> 
> BTW, I never understood the usefulness of Statement Modifiers.
> Even reading the examples in perlsyn.pod:
> 
>     print "Basset hounds got long ears" if length $ear >= 10;
>     go_outside() and play() unless $is_raining;
> 
> seeing "do these things" upfront and then realize "ah, but that
> is done only when this holds true", interrupts the flow of
> understanding while reading a program by somebody else [*1*].


Here's what "Programming Perl" by O'Reilly (third edition, somewhere 
around page 604) has to say about it:
---%<---%<---%<---
*) Do things the most readable way. For instance:

     open(FOO, $foo) or die "Can't open $foo: $!";

is  better than

     die "Can't open $foo: $!" unless open(FOO, $foo);

because the second way hides the main point of the statement in a modifier.
---%<---%<---%<---

Considering the book was co-authored by Larry Wall, I'd say that's as 
good as an apology.


> It is worse if the Statement Modifier is a loop control.
> 
> (flamebait) Compound Statements take BLOCK and people who want
> to do a one-liner could not do so without braces.  I've always
> thought Statement Modifies as a lame workaround for that
> problem.
> 

Most things can be done without braces, so long as one doesn't spell out 
the 'if'.

	$opt_h and usage();
	-d $directory or mkdir($directory);

is perfectly valid perl. This way of writing it is familiar enough for 
shell-scripters. Hardcore C-programmers will look twice when they see a 
line that so obviously looks completely wrong and will almost certainly, 
with some revulsion, understand it.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Alex Riesen]

Junio C Hamano, Sat, Jan 07, 2006 11:29:46 +0100:
> >>
> >>   print H "$_\0" for @deletedfiles;
> >
> > Does not work for old Perl
> 
> How old may I ask?
> 

5.4, probably. I never seen it anymore. Was a standard installation of
some Solaris box.

Re: [PATCH] git-mv.perl: use stderr for error output and cleanup

[Alex Riesen]

Randal L. Schwartz, Sat, Jan 07, 2006 11:34:23 +0100:
> >> >>>>> "Junio" == Junio C Hamano <junkio@cox.net> writes:
> Junio> So I'd prefer not touching for (@df) { print H "$_\n" } loops.
> >> 
> >> Being as I'm a *bit* familiar with Perl, I'd write that as:
> >> 
> >> print H "$_\0" for @deletedfiles;
> >> 
> 
> Alex> Does not work for old Perl
> 
> Correct.  It was added for Perl 5.5, first released on 22 July 1998.
> 
> Are you really saying you need this code to run on Perl 5.4?

No, probably not. I definitely don't care, just wanted to point it
out as another one kind of a strange setup (as if activestat perl +
cygwin isn't enough...)