Giuseppe Bilotta wrote:
> 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.
I have thought about this rather as supplement (addition) to the
current commit message (which states explicitly new form of supported
path_info URL), than replacing it.
>> 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).
No, not supporting this form is just fine.
>>> + 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?
I think you should remove it here too. IMHO if needed, it should be
dealt with (and I think is dealt with) in appropriate action subroutine.
--
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
