On Sat, Oct 4, 2008 at 3:31 AM, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> On Thu, 2 Oct 2008, Giuseppe Bilotta wrote:
>
>> This makes it possible to use an URL such as
>> $project/somebranch..otherbranch:/filename to get a diff between
>> different version of a file. Paths like
>> $project/$action/somebranch:/somefile..otherbranch:/otherfile are parsed
>> as well.
>>
>
> In short, it allows to have link to '*diff' views using path_info URL,
> or in general to pass $hash_[parent_]base and $file_parent using
> path_info.
Yes, that's probably a better form for the commit message.
>> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx>
>> ---
>> gitweb/gitweb.perl | 26 ++++++++++++++++++++++++--
>> 1 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 3e5b2b7..89e360f 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -534,7 +534,9 @@ if ($path_info && !defined $action) {
>>
>> # we can now parse ref and pathnames in PATH_INFO
>> if ($path_info) {
>> - my ($refname, $pathname) = split(/:/, $path_info, 2);
>> + $path_info =~ /^((.+?)(:(.+))?\.\.)?(.+?)(:(.+))?$/;
>> + my ($parentrefname, $parentpathname, $refname, $pathname) = (
>> + $2, $4, $5, $7);
>
> Style: I would use (but that is perhaps matter of taste)
>
> + my ($parentrefname, $parentpathname, $refname, $pathname) =
> + ($2, $4, $5, $7);
Right, I'm not sure why I put the ( on the previous line.
> Also it would be I think simpler to use instead non-catching grouping,
> i.e. (?: xxx ) extended pattern (see perlre(1)), and use
> ($1, $2, $3, $4), or even simpler 'list = (string =~ regexp)' form.
Good idea, I'll rework it in that sense.
> I also think that the situation is more complicated than that, if we
> want to be more correct.
>
> The following path_info layouts with '..' make sense:
>
> hpb:fp..hb:f
> hpb..hb:f == hpb:f..hb:f
> hp..h
And these are matched by the above regexp
> And the layout below can be though to make sense, but it is just
> plain weird.
>
> hpb:fp..f == hpb:fp..HEAD:f
I'm afraid I'm not going to support that, although it's probably easy
to support hpb:fp..:f (i.e. accept a missing refname but on condition
of having a : in front of the file spec).
>> + if (defined $input_params{'file_parent'}) {
>> + $input_params{'hash_parent'} ||= git_get_hash_by_path($input_params{'hash_parent_base'}, $input_params{'file_parent'});
>
> This line is bit long, and I think it should be wrapped..
By the way, on the first revision of the path_info patchset, you had me discard
$hash ||= git_get_hash_by_path($hash_base, $file_name);
in the simple case on the basis that it was an extra call to external git.
I actually forgot to remove it from this part of the patchset too at
the time, so this gets me wondering about this: should I put it back
in place in the simple case, or remove it from here too?
--
Giuseppe "Oblomov" Bilotta
--
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
