On Tue, 30 Sep 2008, Giuseppe Bilotta wrote:
> On Tue, Sep 30, 2008 at 10:48 AM, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
>> I presume that you would want to replace for example $hash_base
>> everywhere by %input_params{'hash_base'}?
>
> No. %input_params{'hash_base'} would only be the _input_ hash base.
> $hash_base would be kept if it's supposed to indicate the value of
> hash base that is being manipulated.
Ah, sorry, I misunderstood. Then your idea is the same as one of mine
(except perhaps some details).
>> I can think of yet another solution, namely to abstract getting
>> parameters from CGI query string, from path_info, and possibly in the
>> future also from command line options, and use this mechanism in
>> the getting parameters and validation part.
>>
>> The %params hash would be filled from CGI parameters by using simply
>> "%params = $cgi->Vars;", then added to in evaluate_path_info instead
>> of directly modifying global parameters variables.
>
> So far I agree.
Using "%input_params = %cgi->Vars;" has consequence of using short
parameter names for keys (and also a bit strange syntax for multivalue
options, see CGI(3pm)).
>> The input validation
>> and dispatch part would be modified to use %params (taking care of
>> multivalued parameters as described in CGI(3pm)), like below:
This has the additional advantage of doing gitweb parameter validation
_once_, and not like it is now done for example first in the "input
validation and dispatch" section, and then in evaluate_path_info()
subroutine.
On the other hand $project is checked _already_ in evaluate_path_info(),
because it has to to find where project name ends, so this part would
get duplicated, unless something smart is done.
>>
>> our $action = $params{'a'} || $params{'action'};
>
> Not too sure about that. The path_info (or whatever)-derived params
> should be converted to use the same name as the CGI params. Or
> conversely, CGI params should be mapped to the corresponding
> full-form.
After thinking about it a little, I agree with above paragraph.
>> That is just for consideration: each approach has its advantages and
>> disadvantages. Your proposal, as I understand it, is similar to the
>> way described in "Storing options in a hash" subsection of
>> Getopt::Long(3pm) manpage.
>
> I'll read that, although it probably is.
Perhaps gitweb should have implement something like GetOptions?
>> Or we could just scrap and revert adding href(..., -replay=>1).
>> There is much trouble with getting it right and performing well,
>> and it is less useful than I thought (at least now).
>
> Dunno, the idea in itself is not bad. We just have to get it right ;)
>
> In a way, I actually think that -replay=>1 should be the default, I
> suspect it makes sense in most cases.
PITA but useful. Hmmm....
--
Jakub Narebski
Poland
--
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
