Re: [PATCHv5 2/3] gitweb: add patches view
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Dec 18, 2008, 18:15 UTC
- Message-ID
- <200812181915.48556.jnareb@gmail.com>
- In-Reply-To
- <7v4p12hv5q.fsf@gitster.siamese.dyndns.org>
On Thu, 18 Dec 2008, Junio C Hamano wrote:
Show 24 quoted lines
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> 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');Just a though, but does it really take much sense to have "patch" and "patches" views handled in git_commitdiff? I can understand in the first version, where it was more about better 'commitdiff_plain', but I wonder about now, where patch(es) view is something between git show and log_plain format... Wouldn't it be simpler to use separate subroutine, for example git_format_patch or something?
-- Jakub Narebski Poland