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

[Jakub Narebski <jnareb@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?

> 
> 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