Re: [PATCH 2/3] gitweb: Filter out commit ID from @difftree in git_commit and git_commitdiff

[Jakub Narebski <jnareb@gmail.com>]

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Filter out commit ID output that git-diff-tree adds when called with
>> only one <tree-ish> (not only for --stdin) in git_commit and
>> git_commitdiff.
> 
> I initially wondered why this, or other existing such sripping,
> is necessary in the first place.
> 
> The collected result is given to git_difftree_body() and it
> feeds parse_difftree_raw_line() with it.  Interestingly enough,
> it _is_ prepared to handle the line with the commit object
> name.

Perhaps I was overeager in adding that. The only place it was used
was git_tree_blame in the proof-of-concept/RFC patch which added
"tree_blame" view, and even there it was discarded solution: using
git-diff-tree can give only commit which changed the file to current
version; it cannot find commit which changed tree/directory (latest
commit which changed any of its files).

This can be useful if at any time we need to parse git-diff-tree --stdin
output.
 
> However, the very initial part of git_difftree_body assumes that
> the array it gets does not have the commit object name (i.e. it
> counts the array members and says "N files changed").

The same is true for git_patchset_body. Moreover if I remember correctly
they count elements of @difftree array (or do equivalent of counting)
_before_ parsing.

> So I think your change is probably a good one, but I suspect you
> probably are better off to make parse_difftree_raw_line() to
> barf when it gets the commit object name to make sure that all
> callers strip it at the same time; 

I'd rather not do that, and leave parse_difftree_raw_line generic.

>                                    better yet, perhaps you can 
> have a single function that invokes git-diff-tree -r (with
> different parameters) and returns the result that is already
> only the difftree body lines?

That would be good idea... if not for the fact that git_commitdiff
uses --patch-with-raw format... but that would make it only slightly
more complicated (strip commit-id, or rather tree-ish from output,
and stop on end of raw part i.e. on first empty line).

I'll do that (unless someone else would do this first).
-- 
Jakub Narebski