Re: [PATCH 0/2] gitweb: patch view

From: Jakub Narebski <jnareb@gmail.com>
Date: Nov 30, 2008, 01:06 UTC
Message-ID: <200811300206.23240.jnareb@gmail.com>
In-Reply-To: <1227966071-11104-1-git-send-email-giuseppe.bilotta@gmail.com>

On Sat, 29 Nov 2008, Giuseppe Bilotta wrote:

> I recently discovered that the commitdiff_plain view is not exactly
> something that can be used by git am directly (for example, the subject
> line gets duplicated in the commit message body after using git am).

That's because gitweb generates email-like format "by hand", instead
of using '--format=email' or git-format-patch like in your series. On
the other hand that allows us to add extra headers, namely X-Git-Tag:
(which hasn't best implementation, anyway) and X-Git-Url: with URL
for given output.

> Since I'm not sure if it was the case to fix the plain view because I
> don't know what its intended usage was, I prepared a new view,
> uncreatively called 'patch', that exposes git format-patch output
> directly.

Perhaps 'format_patch' would be better... hmmm... ?

Actually IMHO both 'commitdiff' and 'commitdiff_plain' try to do two things at once. First to show diff _for_ a commit, i.e. equivalent of "git show" or "git show --pretty=email", perhaps choosing one of parents for a merge commit. Then showing commit message for $hash has sense. The fact that 'commit' view doesn't show patchset, while 'commitdiff' does might be result of historical situation.

Second, to show diff _between_ commits, i.e. equivalent of "git diff branch master". Then there doesn't make much sense to show full commit message _only_ for one side of diff. IMHO that should be main purpose of 'commitdiff' and 'commitdiff_plain' views, or simply 'diff' / 'diff_plain' future views.

What 'patch' view does, what might be not obvious from this description and from first patch in series, is to show diffs for _series_ of commits. It means equivalent of "git log -p" or "git whatchanged". It might make more sense to have plain git-format-patch output, but it could be useful to have some kind of 'git log -p' HTML output.

So even if 'commitdiff' / 'commitdiff_plain' is fixed, 'patch' whould still have its place.

By the way, we still might want to add somehow X-Git-Url and X-Git-Tag headers later to 'patch' ('patchset') output format.

> 
> The second patch exposes it from commitdiff view (obviosly), but also
> from shortlog view, when less than 16 patches are begin shown.

Why this nonconfigurable limit?

Show 7 quoted lines

> 
> Giuseppe Bilotta (2):
>   gitweb: add patch view
>   gitweb: links to patch action in commitdiff and shortlog view
> 
>  gitweb/gitweb.perl |   35 +++++++++++++++++++++++++++++++++--
>  1 files changed, 33 insertions(+), 2 deletions(-)

Thank you for your work on gitweb

-- 
Jakub Narebski
Poland