Re: [RFD] gitweb: href() function to generate URLs for CGI

From

Junio C Hamano <junkio@cox.net>

Date

Aug 21, 2006, 18:22 UTC

Message-ID

<7v1wrauex2.fsf@assigned-by-dhcp.cox.net>

In-Reply-To

<200608211739.32993.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> In first version of href() function we had
> (commit 06a9d86b49b826562e2b12b5c7e831e20b8f7dce)
>
> 	my $href = "$my_uri?";
> 	$href .= esc_param( join(";",
> 		map {
> 			"$mapping{$_}=$params{$_}"
> 		} keys %params
> 	) );
>
> First, there was a question what happend if someone would enter 
> parameter name incorrectly, and some key of %params is not found in 
> %mapping hash. The above code would generate warnings (which web admins 
> frown upon), and empty (because undef) parameters corresponding to e.g. 
> mistyped parameter name. 

@@ -204,7 +277,9 @@ sub href(%) {
 
 	my $href = "$my_uri?";
 	$href .= esc_param( join(";",
-		map { "$mapping{$_}=$params{$_}" } keys %params
+		map {
+			"$mapping{$_}=$params{$_}" if defined $params{$_}
+		} keys %params
 	) );
 
 	return $href;

So we perhaps would want to say:

	if (defined $params{$_} && exists $mapping{$_})

instead there?

> Second problem is that using href() function, although it consolidates 
> to generate URL for CGI, it changes the order of CGI parameters. It 
> used to be that 'p' (project) parameter was first, then 'a' (action) 
> parameter, then hashes ('h', 'hp', 'hb'), last 'f' (filename) or 
> 'p' (page) or 's' (searchtext). The simplest and fastest solution would 
> be to create array with all keys of %mapping in appropriate order and 
> do something like this:
>
> 	my @mapping_sorted = ('project', 'action', 'hash',
> 		'hash_parent', 'hash_base', 'file_name', 'searchtext');
>
> 	my $href = "$my_uri?";
> 	$href .= esc_param( join(";",
> 		map {
> 			"$mapping{$_}=$params{$_}"
> 		} grep { exists $params{$_}} @mapping_sorted;
> 	) );
>
> The problem is of course updating both %mappings and @mapping_sorted.
>
> Is this really a problem, should this (ordering of CGI parameters)
> addressed?

Keeping the generated URL stable would be a very desirable
feature.  Perhaps something like this?

sub href(%) {
        my @mapping = ( project => "p",
                        action => "a",
                        hash => "h",
                        hash_parent => "hp",
                        hash_base => "hb",
                        file_name => "f",
                        file_parent => "fp",
                        page => "pg",
                        searchtext => "s",
                        );
	my %mapping;                        
        for (my $i = 0; $i < @mapping; $i += 2) {
        	my ($k, $v) = ($mapping[$i], $mapping[$i+1]);
                $mapping{$k} = [$i, $v];
        }
	my %params = @_;
	$params{"project"} ||= $project;

	my $href = "$my_uri?";
	$href .= esc_param( join(";",
		map { $_->[1] }
		sort { $a->[0] <=> $b->[0] }
		map {
                	(defined $params{$_} && exists $mapping{$_})
			? [ $mapping{$_}[0], "$mapping{$_}[1]=$params{$_}" ]
                        : ();
		} keys %params
	) );

	return $href;
}