Jakub Narebski <jnareb@xxxxxxxxx> writes:
> It would be better (less error prone) and easier to use '-replay'
> option to href(), i.e. write
>
>> + print $cgi->a({-href => href(-replay=>1, -anchor=>"patch$patchno")},
>> + "patch") .
>
> or even make it so 'href(-anchor=>"ANCHOR")' implies '-replay => 1'.
I don't see why "or even" is an improvement, given the following
implementation.
> @@ -1310,6 +1310,7 @@ sub href {
>
> $params{'project'} = $project unless exists $params{'project'};
>
> - if ($params{-replay}) {
> + if ($params{-replay} ||
> + ($params{-anchor} && keys %params == 1)) {
> while (my ($name, $symbol) = each %cgi_param_mapping) {
> if (!exists $params{$name}) {
> $params{$name} = $input_params{$name};
I don't share your intuition that "anchor" is so special that this part
will not grow into a long chain of "(I want an implicit replay too) ||".
Implicitly enabling it in certain obvious cases is perfectly fine, but the
logic to do so should be in a separate place. Wouldn't it better to have
a separate code that sets 'replay' under this and that condition so that
other people can later add to it at the very beginning of "sub href"?
Unless we do so, if there are other places that need to change the
behaviour based on 'replay', they need to duplicate the "implicit" logic.
--
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