Re: [PATCH] pretty: add extra headers and MIME boundary directly
From: Jeff King <hidden>
Date: 2017-03-25 16:23:54
On Sat, Mar 25, 2017 at 01:16:42PM +0100, René Scharfe wrote:
Use the after_subject member of struct pretty_print_context to pass the extra_headers unchanged, and construct and add the MIME boundary headers directly in pretty.c::pp_title_line() instead of writing both to a static buffer in log-tree.c::log_write_email_headers() first. That's easier, quicker and gets rid of said static buffer.
I'm definitely pleased with the direction. A few comments:
quoted hunk ↗ jump to hunk
@@ -374,26 +372,9 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit, graph_show_oneline(opt->graph); } if (opt->mime_boundary) { - static char subject_buffer[1024]; static char buffer[1024];
We still have this other buffer, which ends up in stat_sep. It should probably get the same treatment, though I think the module boundaries make it a little more awkward. We look at it in diff_flush(), which otherwise doesn't need to know much about the pretty-printing. Perhaps stat_sep should be a callback?
quoted hunk ↗ jump to hunk
diff --git a/pretty.c b/pretty.c index d0f86f5d85..56e668781a 100644 --- a/pretty.c +++ b/pretty.c@@ -1641,6 +1641,21 @@ void pp_title_line(struct pretty_print_context *pp, if (pp->after_subject) { strbuf_addstr(sb, pp->after_subject); } + if (pp->print_email_subject && pp->rev && pp->rev->mime_boundary) { + strbuf_addf(sb, + "MIME-Version: 1.0\n"
In the original, this would have been in "after_subject". Which means we would print it even if print_email_subject is not true. Why do we need to check it in the new conditional? Not that I expect the behavior to be wrong either way; why would we have a mime boundary without setting print_email_subject? But I would think that "do we have a mime boundary" would be the right conditional to trigger printing it. -Peff