Jakub Narebski <jnareb@xxxxxxxxx> writes:
> So perhaps
>
> my %mapping = @mapping;
> my @params_keys_sorted;
> for (my $i = 0; $i < @mapping; $i += 2) {
> if (exists $params{$mapping[$i]}) {
> push @params_keys_sorted, $mapping[$i];
> }
> }
>
> This way we have only one loop over all valid parameter names,
> and we wouldn't need grep nor conditional expression in map,
> and map would loop over all (valid) params keys only.
At that point I suspect that loop can do everything your map {}
did, and would look simpler (albeit perhaps a little bit less
Perlish) like this:
sub href(%) {
my %params = @_;
$params{"project"} ||= $project;
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 @result = ();
for (my $i = 0; $i < @mapping; $i += 2) {
my ($name, $symbol) = ($mapping[$i], $mapping[$i+1]);
if (defined $params{$name}) {
push @result, "$symbol=$params{$name}";
}
}
return "$my_uri?" . esc_param(join(';', @result));
}
By the way, is it really what we want to do to call esc_param()
on joined result?
Maybe I am not reading this code correctly, but this feels quite
counter-intuitive. Actually, it feels downright wrong.
Semicolons and equals are used as separators between key-value
pairs (i.e. syntactic elements) so if we have a value
$params{$name} that happens to contain a ';' or '=' character I
suspect we would want to quote that but not the one we use
before the value or between tuples. Otherwise, how is a search
text that is "a = b;" encoded in the resulting href?
So the last part of the above should perhaps read:
my @result = ();
for (my $i = 0; $i < @mapping; $i += 2) {
my ($name, $symbol) = ($mapping[$i], $mapping[$i+1]);
if (defined $params{$name}) {
push @result, join('=', esc_param($symbol),
esc_param($params{$name}));
}
}
return "$my_uri?" . join(';', @result);
We would also need to fix esc_param to quote ';' and '=' as
well, which does not seem to quote them.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html