git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gitweb: Fix @git_base_url_list usage
@ 2006-09-20  0:09 Petr Baudis
  2006-09-20  0:28 ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Petr Baudis @ 2006-09-20  0:09 UTC (permalink / raw)
  To: git

As it is now, that array was never used because the customurl accessor was
broken and ''unless @url_list'' never happenned.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2d0d56f..1b029ac 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -731,7 +731,7 @@ sub git_get_project_description {
 sub git_get_project_url_list {
 	my $path = shift;
 
-	open my $fd, "$projectroot/$path/cloneurl" or return undef;
+	open my $fd, "$projectroot/$path/cloneurl" or return wantarray ? () : undef;
 	my @git_project_url_list = map { chomp; $_ } <$fd>;
 	close $fd;
 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] gitweb: Fix @git_base_url_list usage
  2006-09-20  0:09 [PATCH] gitweb: Fix @git_base_url_list usage Petr Baudis
@ 2006-09-20  0:28 ` Junio C Hamano
  2006-09-20  0:43   ` Junio C Hamano
  2006-09-20  0:49   ` Petr Baudis
  0 siblings, 2 replies; 5+ messages in thread
From: Junio C Hamano @ 2006-09-20  0:28 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git

Petr Baudis <pasky@suse.cz> writes:

> As it is now, that array was never used because the customurl accessor was
> broken and ''unless @url_list'' never happenned.
>
> Signed-off-by: Petr Baudis <pasky@suse.cz>
> ---
>...
>  sub git_get_project_url_list {
>  	my $path = shift;
>  
> -	open my $fd, "$projectroot/$path/cloneurl" or return undef;
> +	open my $fd, "$projectroot/$path/cloneurl" or return wantarray ? () : undef;
>  	my @git_project_url_list = map { chomp; $_ } <$fd>;
>  	close $fd;

Why on earth do you want to use wantarray for something like
this?

It's not like you are implementinging any fancy DWIM magic.

Isn't

	open my $fd, "foobar" or return;

much easier to read?

#!/usr/bin/perl

sub return_undef {
	open my $fd, "no-such-file"
	    or return wantarray ? () : undef;
}
sub return_broken {
	open my $fd, "no-such-file"
	    or return undef;
}

sub return_empty {
	open my $fd, "no-such-file"
	    or return;
}

my $returned_undef_1 = return_undef();
my ($returned_undef_2) = return_undef();
my @returned_undef = return_undef();

my $returned_broken_1 = return_broken();
my ($returned_broken_2) = return_broken();
my @returned_broken = return_broken();

my $returned_empty_1 = return_empty();
my ($returned_empty_2) = return_empty();
my @returned_empty = return_empty();

printf "U %d B %d E %d\n",
    scalar(@returned_undef),
    scalar(@returned_broken),
    scalar(@returned_empty);

for ($returned_undef_1,
     $returned_undef_2,
     $returned_broken_1,
     $returned_broken_2,
     $returned_empty_1,
     $returned_empty_2) {
	print "What I said was bogus.\n" if (defined $_);
}

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] gitweb: Fix @git_base_url_list usage
  2006-09-20  0:28 ` Junio C Hamano
@ 2006-09-20  0:43   ` Junio C Hamano
  2006-09-20  0:49   ` Petr Baudis
  1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2006-09-20  0:43 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git

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

> Why on earth do you want to use wantarray for something like
> this?
>
> It's not like you are implementinging any fancy DWIM magic.

Side note.  I really hate it when people abuse wantarray.

I can think of only two valid reasons (well, perhaps three but
the last one is a logical consequence of the first two) to use
wantarray.  They are:

 (1) The information you are giving back the caller is list of
     things in nature, but there is a natural representation for
     that list as a single scalar, and that representation is
     not the last element of the list.  You would want to help
     the caller by DWIMmery.

     Perl's built-in localtime() is an excellent example for
     wanting to do something like this.  It returns broken-down
     "struct tm" in list context, but returns string ctime(3) in
     scalar context.

 (2) You want to avoid complex computations when the caller does
     not need full return values from you.  A good example is
     found in perlfunc.pod:

        return unless defined wantarray;	# don't bother doing more
        my @a = complex_calculation();
        return wantarray ? @a : "@a";

 (3) You are emulating Perl's built-in function that DWIMs for
     one of the above reasons, usually (1).

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] gitweb: Fix @git_base_url_list usage
  2006-09-20  0:28 ` Junio C Hamano
  2006-09-20  0:43   ` Junio C Hamano
@ 2006-09-20  0:49   ` Petr Baudis
  2006-09-22 23:15     ` Petr Baudis
  1 sibling, 1 reply; 5+ messages in thread
From: Petr Baudis @ 2006-09-20  0:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Dear diary, on Wed, Sep 20, 2006 at 02:28:28AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > As it is now, that array was never used because the customurl accessor was
> > broken and ''unless @url_list'' never happenned.
> >
> > Signed-off-by: Petr Baudis <pasky@suse.cz>
> > ---
> >...
> >  sub git_get_project_url_list {
> >  	my $path = shift;
> >  
> > -	open my $fd, "$projectroot/$path/cloneurl" or return undef;
> > +	open my $fd, "$projectroot/$path/cloneurl" or return wantarray ? () : undef;
> >  	my @git_project_url_list = map { chomp; $_ } <$fd>;
> >  	close $fd;
> 
> Why on earth do you want to use wantarray for something like
> this?
> 
> It's not like you are implementinging any fancy DWIM magic.
> 
> Isn't
> 
> 	open my $fd, "foobar" or return;
> 
> much easier to read?

Sure, that's what I meant, I'm only already a bit tired so I just
mimicked the other return in that sub, sorry.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] gitweb: Fix @git_base_url_list usage
  2006-09-20  0:49   ` Petr Baudis
@ 2006-09-22 23:15     ` Petr Baudis
  0 siblings, 0 replies; 5+ messages in thread
From: Petr Baudis @ 2006-09-22 23:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

As it is now, that array was never used because the customurl accessor was
broken and ''unless @url_list'' never happenned.
---

 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f7e7d10..a357604 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -752,7 +752,7 @@ sub git_get_project_description {
 sub git_get_project_url_list {
 	my $path = shift;
 
-	open my $fd, "$projectroot/$path/cloneurl" or return undef;
+	open my $fd, "$projectroot/$path/cloneurl" or return;
 	my @git_project_url_list = map { chomp; $_ } <$fd>;
 	close $fd;
 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2006-09-22 23:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-20  0:09 [PATCH] gitweb: Fix @git_base_url_list usage Petr Baudis
2006-09-20  0:28 ` Junio C Hamano
2006-09-20  0:43   ` Junio C Hamano
2006-09-20  0:49   ` Petr Baudis
2006-09-22 23:15     ` Petr Baudis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).