git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

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

When gathering the author and oneline subject for each commit, we hand-parse the commit headers to find the "author" line, and then continue past to the blank line at the end of the header.

We can replace this tricky hand-parsing by simply asking the pretty-printer for the relevant items. This also decouples the author and oneline parsing, opening up some new optimizations in further commits.

One reason to avoid the pretty-printer is that it might be less efficient than hand-parsing. However, I measured no slowdown at all running "git shortlog -ns HEAD" on linux.git.

As a bonus, we also fix a memory leak in the (uncommon) case that the author field is blank.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/shortlog.c | 62 +++++++++++++++++++++++-------------------------------
 1 file changed, 26 insertions(+), 36 deletions(-)
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 6c0a72e..1261ec4 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -113,45 +113,35 @@ static void read_from_stdin(struct shortlog *log)
 
 void shortlog_add_commit(struct shortlog *log, struct commit *commit)
 {
-	const char *author = NULL, *buffer;
-	struct strbuf buf = STRBUF_INIT;
-	struct strbuf ufbuf = STRBUF_INIT;
-
-	pp_commit_easy(CMIT_FMT_RAW, commit, &buf);
-	buffer = buf.buf;
-	while (*buffer && *buffer != '\n') {
-		const char *eol = strchr(buffer, '\n');
-
-		if (eol == NULL)
-			eol = buffer + strlen(buffer);
-		else
-			eol++;
-
-		if (starts_with(buffer, "author "))
-			author = buffer + 7;
-		buffer = eol;
-	}
-	if (!author) {
+	struct strbuf author = STRBUF_INIT;
+	struct strbuf oneline = STRBUF_INIT;
+	struct pretty_print_context ctx = {0};
+
+	ctx.fmt = CMIT_FMT_USERFORMAT;
+	ctx.abbrev = log->abbrev;
+	ctx.subject = "";
+	ctx.after_subject = "";
+	ctx.date_mode.type = DATE_NORMAL;
+	ctx.output_encoding = get_log_output_encoding();
+
+	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));
-		return;
-	}
-	if (log->user_format) {
-		struct pretty_print_context ctx = {0};
-		ctx.fmt = CMIT_FMT_USERFORMAT;
-		ctx.abbrev = log->abbrev;
-		ctx.subject = "";
-		ctx.after_subject = "";
-		ctx.date_mode.type = DATE_NORMAL;
-		ctx.output_encoding = get_log_output_encoding();
-		pretty_print_commit(&ctx, commit, &ufbuf);
-		buffer = ufbuf.buf;
-	} else if (*buffer) {
-		buffer++;
+		goto out;
 	}
-	insert_one_record(log, author, !*buffer ? "<none>" : buffer);
-	strbuf_release(&ufbuf);
-	strbuf_release(&buf);
+
+	if (log->user_format)
+		pretty_print_commit(&ctx, commit, &oneline);
+	else
+		format_commit_message(commit, "%s", &oneline, &ctx);
+
+	insert_one_record(log, author.buf, oneline.len ? oneline.buf : "<none>");
+
+out:
+	strbuf_release(&author);
+	strbuf_release(&oneline);
 }
 
 static void get_from_rev(struct rev_info *rev, struct shortlog *log)
-- 
2.7.0.248.g5eafd77
Previous: Jeff KingNext: Jeff King
Message 10 of 22 in “shortlog fixes and optimizations”
  1. 0/6 shortlog fixes and optimizationsJeff King, Jan 15, 2016
  2. 1/6 shortlog: match both "Author:" and "author" on stdinJeff King, Jan 15, 2016
  3. Eric SunshineJan 15, 2016
  4. Jeff KingJan 18, 2016
  5. Jeff KingJan 18, 2016
  6. Junio C HamanoJan 18, 2016
  7. 0/6 shortlog fixes and optimizationsJeff King, Jan 18, 2016
  8. 1/6 shortlog: match both "Author:" and "author" on stdinJeff King, Jan 18, 2016
  9. 2/6 shortlog: use strbufs to read from stdinJeff King, Jan 18, 2016
  10. 3/6 shortlog: replace hand-parsing of author with pretty-printerJeff King, Jan 18, 2016
  11. Jeff KingJan 18, 2016
  12. Jeff KingJan 18, 2016
  13. Junio C HamanoJan 19, 2016
  14. 4/6 shortlog: optimize "--summary" modeJeff King, Jan 18, 2016
  15. 5/6 shortlog: optimize out useless "<none>" normalizationJeff King, Jan 18, 2016
  16. 6/6 shortlog: optimize out useless string listJeff King, Jan 18, 2016
  17. 2/6 shortlog: use strbufs to read from stdinJeff King, Jan 15, 2016
  18. 3/6 shortlog: replace hand-parsing of author with pretty-printerJeff King, Jan 15, 2016
  19. 4/6 shortlog: optimize "--summary" modeJeff King, Jan 15, 2016
  20. 5/6 shortlog: optimize out useless "<none>" normalizationJeff King, Jan 15, 2016
  21. 6/6 shortlog: optimize out useless string listJeff King, Jan 15, 2016
  22. Junio C HamanoJan 15, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.