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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

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