Re: [PATCH 3/6] shortlog: replace hand-parsing of author with pretty-printer

From: Jeff King <peff@peff.net>
Date: Jan 18, 2016, 20:13 UTC
Message-ID: <20160118201337.GA15943@sigill.intra.peff.net>
In-Reply-To: <20160118200248.GC15836@sigill.intra.peff.net>

On Mon, Jan 18, 2016 at 03:02:48PM -0500, Jeff King wrote:

Show 8 quoted lines

> +	format_commit_message(commit, "%an <%ae>", &author, &ctx);
> +	/* we can detect a total failure only by seeing " <>" in the output */
> +	if (author.len <= 3) {
>  		warning(_("Missing author: %s"),
>  		    oid_to_hex(&commit->object.oid));
> [...]
> +		goto out;
>  	}

One note on this. In the linux.git tree, this warning actually triggers, because there is a commit with a bogus empty author:

  $ git log -1 --format=raw af25e94d4dc | grep ^author
  author  <> 1120285620 -0700

Whereas in the original code, you really do get a line with a blank name.

I think what the new code does is quite reasonable, but I'm not sure if:

  1. People really want a syntactically valid empty name to be
     represented.

and

  2. Regardless of the output, if kernel folks will be annoyed by the
     warning whenever they run a full-repo shortlog.

-Peff