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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> 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?


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]