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

From: Jakub Narebski <jnareb@gmail.com>
Date: Oct 25, 2006, 08:03 UTC
Message-ID: <200610251003.22526.jnareb@gmail.com>
In-Reply-To: <7v1woxf1v9.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

Show 13 quoted lines

> 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