[PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Petr Baudis]

This is important for the list of clone urls, where if there are
no per-repository clone URL configured, the default base URLs
are never used for URL construction with this patch.

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 491a3f4..d5505a4 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1512,7 +1512,7 @@ sub config_to_int {
 sub config_to_multi {
 	my $val = shift;
 
-	return ref($val) ? $val : [ $val ];
+	return ref($val) ? $val : $val ? [ $val ] : [];
 }
 
 sub git_get_project_config {

Re: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Jakub Narebski]

Petr Baudis <pasky@suse.cz> writes:

> This is important for the list of clone urls, where if there are
> no per-repository clone URL configured, the default base URLs
> are never used for URL construction with this patch.

Thanks.
 
> @@ -1512,7 +1512,7 @@ sub config_to_int {
>  sub config_to_multi {
>  	my $val = shift;
>  
> -	return ref($val) ? $val : [ $val ];
> +	return ref($val) ? $val : $val ? [ $val ] : [];
>  }

Shouldn't it be

  +	return ref($val) ? $val : defined($val) ? [ $val ] : [];

-- 
Jakub Narebski

Re: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Petr Baudis <pasky@suse.cz> writes:
>
>> This is important for the list of clone urls, where if there are
>> no per-repository clone URL configured, the default base URLs
>> are never used for URL construction with this patch.
>
> Thanks.
>  
>> @@ -1512,7 +1512,7 @@ sub config_to_int {
>>  sub config_to_multi {
>>  	my $val = shift;
>>  
>> -	return ref($val) ? $val : [ $val ];
>> +	return ref($val) ? $val : $val ? [ $val ] : [];
>>  }
>
> Shouldn't it be
>
>   +	return ref($val) ? $val : defined($val) ? [ $val ] : [];

I think so.  Pasky?

[PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Jakub Narebski]

From: Petr Baudis <pasky@suse.cz>
Date: Sat, 8 Dec 2007 12:30:59 +0100
Subject: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

This is important for the list of clone urls, where if there are
no per-repository clone URL configured, the default base URLs
are never used for URL construction without this patch.

Add tests for different ways of setting project URLs, just in case.
Note that those tests in current form wouldn't detect breakage fixed
by this patch, as it only checks for errors and not for expected
output.

Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Originally by Petr Baudis, changed to use defined($val). I have also
added parentheses for better readibility.

I have added tests _then_ I have realized that in current form they
cannot detect regression corrected by this patch. So if you want, you
can not apply changes to test (and remove paragraph about test from
commit message).

The fact that patch was not applied might be cause by the lack of Ack
from pasky.

 gitweb/gitweb.perl                     |    2 +-
 t/t9500-gitweb-standalone-no-errors.sh |   25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 24b3158..a746a85 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1511,7 +1511,7 @@ sub config_to_int {
 sub config_to_multi {
        my $val = shift;
 
-       return ref($val) ? $val : [ $val ];
+       return ref($val) ? $val : (defined($val) ? [ $val ] : []);
 }
 
 sub git_get_project_config {
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 35fff3d..2d3d4e8 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -558,6 +558,31 @@ test_expect_success \
 test_debug 'cat gitweb.log'
 
 # ----------------------------------------------------------------------
+# testing config_to_multi / cloneurl
+
+test_expect_success \
+       'URL: no project URLs, no base URL' \
+       'gitweb_run "p=.git;a=summary"'
+test_debug 'cat gitweb.log'
+
+test_expect_success \
+       'URL: project URLs via gitweb.url' \
+       'git config --add gitweb.url git://example.com/git/trash.git &&
+        git config --add gitweb.url http://example.com/git/trash.git &&
+        gitweb_run "p=.git;a=summary"'
+test_debug 'cat gitweb.log'
+
+cat >.git/cloneurl <<\EOF
+git://example.com/git/trash.git
+http://example.com/git/trash.git
+EOF
+
+test_expect_success \
+       'URL: project URLs via cloneurl file' \
+       'gitweb_run "p=.git;a=summary"'
+test_debug 'cat gitweb.log'
+
+# ----------------------------------------------------------------------
 # gitweb config and repo config
 
 cat >>gitweb_config.perl <<EOF
-- 
1.5.3.7

Re: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Petr Baudis]

On Sat, Dec 15, 2007 at 03:36:32PM +0100, Jakub Narebski wrote:
> From: Petr Baudis <pasky@suse.cz>
> Date: Sat, 8 Dec 2007 12:30:59 +0100
> Subject: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]
> 
> This is important for the list of clone urls, where if there are
> no per-repository clone URL configured, the default base URLs
> are never used for URL construction without this patch.
> 
> Add tests for different ways of setting project URLs, just in case.
> Note that those tests in current form wouldn't detect breakage fixed
> by this patch, as it only checks for errors and not for expected
> output.
> 
> Signed-off-by: Petr Baudis <pasky@suse.cz>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>

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

Thank you all for catching the defined($val). :-)

-- 
				Petr "Pasky" Baudis
We don't know who it was that discovered water, but we're pretty sure
that it wasn't a fish.		-- Marshall McLuhan

Re: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Jakub Narebski]

On Sat, 15 Dec 2007, Jakub Narebski wrote:

> This is important for the list of clone urls, where if there are
> no per-repository clone URL configured, the default base URLs
> are never used for URL construction without this patch.
> 
> Add tests for different ways of setting project URLs, just in case.
> Note that those tests in current form wouldn't detect breakage fixed
> by this patch, as it only checks for errors and not for expected
> output.

The patch applied to master bc8b95ae4a4b21753e84bbfd28cbcbf1b3f6e0a8
does have above paragraph in commit message, but DOES NOT have the
tests itself.
 
> I have added tests _then_ I have realized that in current form they
> cannot detect regression corrected by this patch. So if you want, you
> can not apply changes to test (and remove paragraph about test from
> commit message).

See above...

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 24b3158..a746a85 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1511,7 +1511,7 @@ sub config_to_int {
>  sub config_to_multi {
>         my $val = shift;
>  
> -       return ref($val) ? $val : [ $val ];
> +       return ref($val) ? $val : (defined($val) ? [ $val ] : []);
>  }

And somehow (I don't know how[*1*]) patch got whitespace corrupted,
and now the line is indented with spaces, instead of with tab.

[*1*] Other patches are fine.
-- 
Jakub Narebski
Poland

Re: [PATCH] gitweb: Make config_to_multi return [] instead of [undef]

[Junio C Hamano]

Please send updates if something is not right.  Thanks.