Re: [PATCH v2 4/4] log: --author-date-order

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 10, 2013, 07:39 UTC
Message-ID: <7vobbel8ib.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130610055014.GF3621@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 6 quoted lines

> I'm not excited about introducing yet another place that parses commit
> objects (mostly not for correctness, but because we have had
> inconsistency in how malformed objects are treated). It is at least
> using split_ident_line which covers the hard bits. I wonder how much
> slower it would be to simply call format_commit_message to do the
> parsing.

The thought certainly crossed my mind, not exactly in that form but more about splitting the machinery used in pretty.c into a more reusable form.

The result of my attempt however did not become all that reusable (admittedly I didn't spend too much brain cycles on it), so I punted ;-).

> The record_author_date function assumes that commit->buffer is valid
> (i.e., not NULL).  We seem to assume that the commits are parsed already
> (for looking at parents, and at the committer date).

I thought that the latter is warranted, as the function worked on the output of limit_list(), and by the time limit_list() finishes, everything relevant must have been parsed already.

But you are right. The commit->buffer may no longer be there, and the --author-date-order option needs to read the object again in this codepath. That would be in line with what --pretty/format would do, I guess.

Or we could extend parse_commit() API to take an optional commit info slab to store not just author date but other non-essential stuff like people's names, and we arrange that extended API to be triggered when we know --author-date-order is in effect?