shortlog: replace hand-parsing of author with pretty-printer

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jan 18, 2016 at 15:02 UTC 2db6b83d189bb82d1d45805fa6c85a9c8b507920
1 file changed +26 -36
builtin/shortlog.c
+26 -36
@@ -113,45 +113,35 @@ static void read_from_stdin(struct shortlog *log)
113
114 void shortlog_add_commit(struct shortlog *log, struct commit *commit)
115 {
116 - const char *author = NULL, *buffer;
117 - struct strbuf buf = STRBUF_INIT;
118 - struct strbuf ufbuf = STRBUF_INIT;
119 -
120 - pp_commit_easy(CMIT_FMT_RAW, commit, &buf);
121 - buffer = buf.buf;
122 - while (*buffer && *buffer != '\n') {
123 - const char *eol = strchr(buffer, '\n');
124 -
125 - if (eol == NULL)
126 - eol = buffer + strlen(buffer);
127 - else
128 - eol++;
129 -
130 - if (starts_with(buffer, "author "))
131 - author = buffer + 7;
132 - buffer = eol;
133 - }
134 - if (!author) {
116 + struct strbuf author = STRBUF_INIT;
117 + struct strbuf oneline = STRBUF_INIT;
118 + struct pretty_print_context ctx = {0};
119 +
120 + ctx.fmt = CMIT_FMT_USERFORMAT;
121 + ctx.abbrev = log->abbrev;
122 + ctx.subject = "";
123 + ctx.after_subject = "";
124 + ctx.date_mode.type = DATE_NORMAL;
125 + ctx.output_encoding = get_log_output_encoding();
126 +
127 + format_commit_message(commit, "%an <%ae>", &author, &ctx);
128 + /* we can detect a total failure only by seeing " <>" in the output */
129 + if (author.len <= 3) {
130 warning(_("Missing author: %s"),
131 oid_to_hex(&commit->object.oid));
137 - return;
138 - }
139 - if (log->user_format) {
140 - struct pretty_print_context ctx = {0};
141 - ctx.fmt = CMIT_FMT_USERFORMAT;
142 - ctx.abbrev = log->abbrev;
143 - ctx.subject = "";
144 - ctx.after_subject = "";
145 - ctx.date_mode.type = DATE_NORMAL;
146 - ctx.output_encoding = get_log_output_encoding();
147 - pretty_print_commit(&ctx, commit, &ufbuf);
148 - buffer = ufbuf.buf;
149 - } else if (*buffer) {
150 - buffer++;
132 + goto out;
133 }
152 - insert_one_record(log, author, !*buffer ? "<none>" : buffer);
153 - strbuf_release(&ufbuf);
154 - strbuf_release(&buf);
134 +
135 + if (log->user_format)
136 + pretty_print_commit(&ctx, commit, &oneline);
137 + else
138 + format_commit_message(commit, "%s", &oneline, &ctx);
139 +
140 + insert_one_record(log, author.buf, oneline.len ? oneline.buf : "<none>");
141 +
142 +out:
143 + strbuf_release(&author);
144 + strbuf_release(&oneline);
145 }
146
147 static void get_from_rev(struct rev_info *rev, struct shortlog *log)