Subject: Re: [PATCHv5 2/3] gitweb: add patches view

From: Junio C Hamano <[email-masked]>
Date: Thu, 18 Dec 2008 11:15:37 -0800

Jakub Narebski <[email-masked]> writes:

> On Thu, 18 Dec 2008, Junio C Hamano wrote:
>> Giuseppe Bilotta <[email-masked]> writes:
>> 
>> >  sub git_commitdiff {
>> >  	my $format = shift || 'html';
>> > +	my %params = @_;
>> > ...  
>> > +			if ($params{-single}) {
>> > +				push @commit_spec, '-1';
>> > +			} else {
>> > +				if ($patch_max > 0) {
>> > ...
>> > +			}
>> > @@ -5625,6 +5635,10 @@ sub git_commitdiff_plain {
>> >  
>> >  # format-patch-style patches
>> >  sub git_patch {
>> > +	git_commitdiff('patch', -single=> 1);
>> > +}
>> 
>> Hmm, if you are changing the interface this way, wouldn't it make more
>> sense to also do this?
>> 
>> 	git_commitdiff(--format => 'patch', --single => 1);
>> 	git_commitdiff(--format => 'html');
>
> The first argument (format) is _required_, second is _optional_;

My reading of the first line of the sub seems to contradict with your
statement.  "If the first argument is empty, we use 'html' instead" sounds
pretty optional to me.