Re: [PATCH 1/6] gitweb: action in path with use_pathinfo
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Sep 30, 2008, 23:24 UTC
- Message-ID
- <200810010124.41455.jnareb@gmail.com>
- In-Reply-To
- <cb7bb73a0809300340t79a497fey4ededd960223fcdd@mail.gmail.com>
On Tue, 30 Sep 2008, Giuseppe Bilotta wrote:
> On Tue, Sep 30, 2008 at 10:48 AM, Jakub Narebski <jnareb@gmail.com> wrote:
Show 6 quoted lines
>> 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).
Show 10 quoted lines
>> 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.
Show 7 quoted lines
>>
>> 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.
Show 6 quoted lines
>> 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?
Show 8 quoted lines
>> 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