Re: [RFCv4 2/3] gitweb: add patches view

[Jakub Narebski <jnareb@gmail.com>]

Giuseppe Bilotta wrote:
> On Tue, Dec 16, 2008 at 4:14 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Sat, 6 Dec 2008 16:02, Giuseppe Bilotta wrote:
>>
>>> The only difference between patch and patches view is in the treatement
>>> of single commits: the former only displays a single patch, whereas the
>>> latter displays a patchset leading to the specified commit.
>>
>> I like that fact that we have "patches" action which intent is to
>> show series of patches, and "patch" action which intent is to show
>> single patch. I'm just not sure if "patch" view should not simply
>> ignore $hash_parent...
> 
> I had doubts on this myself. In the end I decided to make patch
> consider hash_parent if present because IMO it's what a user would
> expect in case e.g. of hand-crafted URLs.

Ah. I can understand that.

[...]
>>>  sub git_commitdiff {
>>>       my $format = shift || 'html';
>>> +     # for patch view: should we limit ourselves to a single patch
>>> +     # if only a single commit is passed?
>>> +     my $single_patch = shift && 1;
>>
>> What does this "shift && 1" does? Equivalent of "!!shift"?
>> Is it really needed?
>>
>> Perhaps it would be better to use %opts trick, like for some other
>> gitweb subroutines (-single=>1, or -single_patch=>1, or -nmax=>1)?
>> Or perhaps not...
> 
> It would be MUCH better, I'll do it this way. I'll pass the -single
> param in both cases, having value true/false, even though the false
> case is not needed since undef is false in perl anyway. (I like
> symmetry.)

-- 
Jakub Narebski
Poland