patchgitweb: Sort projects with undefined ages last

> Sorting gitweb's project list by age ('Last Change') currently shows
> projects with undefined ages at the head of the list. This results in a
> less useful result when there are a number of projects that are missing
> or otherwise faulty and one is trying to see what projects have been
> updated recently.
>
> Fix by sorting these projects with undefined ages at the bottom of the
> list when sorting by age.
>
> Signed-off-by: Matthew Daley <mattjd@gmail.com>
> ---
> I realize this might be a bit bikesheddy, but it does improve the listing
> in the given use case. For an example of the problem, see ie.
> http://git.kernel.org/?o=age or http://repo.or.cz/w?a=project_list;o=age .

> I'm also not a Perl native, so any advice on making the patch good Perl is
> appreciated.
>
>  gitweb/gitweb.perl |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0f207f2..21da1b5 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5541,7 +5541,9 @@ sub sort_projects_list {
>  	if ($oi->{'type'} eq 'str') {
>  		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @$projlist;
>  	} else {
> -		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @$projlist;
> +		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}}
> +		            grep {defined $_->{$oi->{'key'}}} @$projlist;
> +		push @projects, grep {!defined $_->{$oi->{'key'}}} @$projlist;
>  	}

> Yeah, it could be argued that in a very minor corner case showing
> new and empty ones at the top might attract more attention to them,
> but new and empty ones can stay inactive, so this change would be an
> overall improvement for these two sites.  An alternative could be to
> give the mtime of the git directory to the age field if there is no
> commits in the repository, to sink the empty and inactive ones to
> the bottom quickly while showing newly created ones at the top, but
> it shouldn't make any practical difference.

> Two observations:
>
>  * This iterates over the same @$projlist twice with grep, with one
>    "defined" and the other "!defined", which may risk these two
>    complementary grep conditions to go out of sync (it also may
>    affect performance but that is a lessor issue).
>
>    An alternative may be to change the expression used inside sort()
>    to treat an undef as if it were a very large value, something
>    like:
>
>         sort {
>                 defined $a->{$oi->{'key'}}
>                 ? (defined $b->{$oi->{'key'}}
>                   ? ($a->{$oi->{'key'}} <=> $b->{$oi->{'key'}})
>                   : -1)
>                 : (defined $b->{$oi->{'key'}} ? 1 : 0);
>         }
>
>  * This "sort undefs at the end is better than at the beginning" is
>    good only for the "age" field, and we wouldn't know if we would
>    add other keys for which it may be better to sort undef at the
>    beginning.  The order_info{} currently has only one field of the
>    'num' type, so this is not an immediate issue, but in order to
>    future proof, it may make sense to rewrite the sort_projects_list
>    function to map the order field name to a function given to sort,
>    e.g.
>
>         my %order_sort = (
>                 project => sub { $a->{'path'} cmp $b->{'path'} },
>                 descr => sub { $a->{'descr_long'} cmp $b->{'descr_long'} },
>                 owner => sub { $a->{'owner'} cmp $b->{'owner'} },
>                 age => sub { ... the num cmp with undef above ... },
>         );
>         if (!exists $order_sort{$order}) {
>                 return @$projlist;
>         }
>         return sort $order_sort{$order} @$projlist;
>
> I am not sure the second one is worth it, though.

> I thought about both of those variants as well. What about this:
>
> -- >8 --
> Subject: [PATCH] gitweb: Sort projects with undefined ages last
>
> Sorting gitweb's project list by age ('Last Change') currently shows
> projects with undefined ages at the head of the list. This gives a less
> useful result when there are a number of projects that are missing or
> otherwise faulty and one is trying to see what projects have been
> updated recently.
>
> Fix by sorting these projects with undefined ages at the bottom of the
> list when sorting by age.
>
> Signed-off-by: Matthew Daley <mattjd@gmail.com>
> ---

>  gitweb/gitweb.perl |   35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0f207f2..656b324 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5528,23 +5528,30 @@ sub fill_project_list_info {
>  
>  sub sort_projects_list {
>  	my ($projlist, $order) = @_;
> -	my @projects;
>  
> -	my %order_info = (
> -		project => { key => 'path', type => 'str' },
> -		descr => { key => 'descr_long', type => 'str' },
> -		owner => { key => 'owner', type => 'str' },
> -		age => { key => 'age', type => 'num' }
> -	);
> -	my $oi = $order_info{$order};
> -	return @$projlist unless defined $oi;
> -	if ($oi->{'type'} eq 'str') {
> -		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @$projlist;
> -	} else {
> -		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @$projlist;
> +	sub order_str {
> +		my $key = shift;
> +		return sub { $a->{$key} cmp $b->{$key} };
>  	}
>  
> -	return @projects;
> +	sub order_num_then_undef {
> +		my $key = shift;
> +		return sub {
> +			defined $a->{$key} ?
> +				(defined $b->{$key} ? $a->{$key} <=> $b->{$key} : -1) :
> +				(defined $b->{$key} ? 1 : 0)
> +		};
> +	}
> +
> +	my %orderings = (
> +		project => order_str('path'),
> +		descr => order_str('descr_long'),
> +		owner => order_str('owner'),
> +		age => order_num_then_undef('age'),
> +	);
> +
> +	my $ordering = $orderings{$order};
> +	return defined $ordering ? sort $ordering @$projlist : @$projlist;
>  }
>  
>  # returns a hash of categories, containing the list of project