This output format prevents format-patch output from breaking
readers if somebody copy+pasted an mbox into a commit message.

Unlike the traditional "mboxo" format, "mboxrd" is designed to
be fully-reversible.  "mboxrd" also gracefully degrades to
showing extra ">" in existing "mboxo" readers.

This degradation is preferable to breaking message splitting
completely, a problem I've seen in "mboxcl" due to having
multiple, non-existent, or inaccurate Content-Length headers.

"mboxcl2" is a non-starter since it's inherits the problems
of "mboxcl" while being completely incompatible with existing
tooling based around mailsplit.

ref: 
http://homepage.ntlworld.com/jonathan.deboynepollard/FGA/mail-mbox-formats.html

Signed-off-by: Eric Wong <e...@80x24.org>
---
 builtin/log.c           |  2 +-
 commit.h                |  6 ++++++
 log-tree.c              |  4 ++--
 pretty.c                | 45 +++++++++++++++++++++++++++++++++++++--------
 t/t4014-format-patch.sh | 27 +++++++++++++++++++++++++++
 5 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 099f4f7..6d6f368 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -953,7 +953,7 @@ static void make_cover_letter(struct rev_info *rev, int 
use_stdout,
        struct pretty_print_context pp = {0};
        struct commit *head = list[0];
 
-       if (rev->commit_format != CMIT_FMT_EMAIL)
+       if (!cmit_fmt_is_mail(rev->commit_format))
                die(_("Cover letter needs email format"));
 
        committer = git_committer_info(0);
diff --git a/commit.h b/commit.h
index b06db4d..1e04d3a 100644
--- a/commit.h
+++ b/commit.h
@@ -131,11 +131,17 @@ enum cmit_fmt {
        CMIT_FMT_FULLER,
        CMIT_FMT_ONELINE,
        CMIT_FMT_EMAIL,
+       CMIT_FMT_MBOXRD,
        CMIT_FMT_USERFORMAT,
 
        CMIT_FMT_UNSPECIFIED
 };
 
+static inline int cmit_fmt_is_mail(enum cmit_fmt fmt)
+{
+       return (fmt == CMIT_FMT_EMAIL || fmt == CMIT_FMT_MBOXRD);
+}
+
 struct pretty_print_context {
        /*
         * Callers should tweak these to change the behavior of pp_* functions.
diff --git a/log-tree.c b/log-tree.c
index 78a5381..48daf84 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -603,7 +603,7 @@ void show_log(struct rev_info *opt)
         * Print header line of header..
         */
 
-       if (opt->commit_format == CMIT_FMT_EMAIL) {
+       if (cmit_fmt_is_mail(opt->commit_format)) {
                log_write_email_headers(opt, commit, &ctx.subject, 
&extra_headers,
                                        &ctx.need_8bit_cte);
        } else if (opt->commit_format != CMIT_FMT_USERFORMAT) {
@@ -694,7 +694,7 @@ void show_log(struct rev_info *opt)
 
        if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
            ctx.notes_message && *ctx.notes_message) {
-               if (ctx.fmt == CMIT_FMT_EMAIL) {
+               if (cmit_fmt_is_mail(ctx.fmt)) {
                        strbuf_addstr(&msgbuf, "---\n");
                        opt->shown_dashes = 1;
                }
diff --git a/pretty.c b/pretty.c
index 87c4497..a33e604 100644
--- a/pretty.c
+++ b/pretty.c
@@ -92,6 +92,7 @@ static void setup_commit_formats(void)
                { "medium",     CMIT_FMT_MEDIUM,        0,      8 },
                { "short",      CMIT_FMT_SHORT,         0,      0 },
                { "email",      CMIT_FMT_EMAIL,         0,      0 },
+               { "mboxrd",     CMIT_FMT_MBOXRD,        0,      0 },
                { "fuller",     CMIT_FMT_FULLER,        0,      8 },
                { "full",       CMIT_FMT_FULL,          0,      8 },
                { "oneline",    CMIT_FMT_ONELINE,       1,      0 }
@@ -444,7 +445,7 @@ void pp_user_info(struct pretty_print_context *pp,
        if (pp->mailmap)
                map_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
 
-       if (pp->fmt == CMIT_FMT_EMAIL) {
+       if (cmit_fmt_is_mail(pp->fmt)) {
                if (pp->from_ident && ident_cmp(pp->from_ident, &ident)) {
                        struct strbuf buf = STRBUF_INIT;
 
@@ -494,6 +495,7 @@ void pp_user_info(struct pretty_print_context *pp,
                            show_ident_date(&ident, &pp->date_mode));
                break;
        case CMIT_FMT_EMAIL:
+       case CMIT_FMT_MBOXRD:
                strbuf_addf(sb, "Date: %s\n",
                            show_ident_date(&ident, DATE_MODE(RFC2822)));
                break;
@@ -535,7 +537,7 @@ static void add_merge_info(const struct 
pretty_print_context *pp,
 {
        struct commit_list *parent = commit->parents;
 
-       if ((pp->fmt == CMIT_FMT_ONELINE) || (pp->fmt == CMIT_FMT_EMAIL) ||
+       if ((pp->fmt == CMIT_FMT_ONELINE) || (cmit_fmt_is_mail(pp->fmt)) ||
            !parent || !parent->next)
                return;
 
@@ -1614,7 +1616,7 @@ void pp_title_line(struct pretty_print_context *pp,
        if (pp->after_subject) {
                strbuf_addstr(sb, pp->after_subject);
        }
-       if (pp->fmt == CMIT_FMT_EMAIL) {
+       if (cmit_fmt_is_mail(pp->fmt)) {
                strbuf_addch(sb, '\n');
        }
 
@@ -1697,12 +1699,34 @@ static void pp_handle_indent(struct 
pretty_print_context *pp,
                strbuf_add(sb, line, linelen);
 }
 
+static regex_t *mboxrd_prepare(void)
+{
+       static regex_t preg;
+       const char re[] = "^>*From ";
+       int err = regcomp(&preg, re, REG_NOSUB | REG_EXTENDED);
+
+       if (err) {
+               char errbuf[1024];
+
+               regerror(err, &preg, errbuf, sizeof(errbuf));
+               regfree(&preg);
+               die("Cannot prepare regexp `%s': %s", re, errbuf);
+       }
+
+       return &preg;
+}
+
 void pp_remainder(struct pretty_print_context *pp,
                  const char **msg_p,
                  struct strbuf *sb,
                  int indent)
 {
        int first = 1;
+       static regex_t *mboxrd_from;
+
+       if (pp->fmt == CMIT_FMT_MBOXRD && !mboxrd_from)
+               mboxrd_from = mboxrd_prepare();
+
        for (;;) {
                const char *line = *msg_p;
                int linelen = get_one_line(line);
@@ -1725,8 +1749,13 @@ void pp_remainder(struct pretty_print_context *pp,
                else if (pp->expand_tabs_in_log)
                        strbuf_add_tabexpand(sb, pp->expand_tabs_in_log,
                                             line, linelen);
-               else
+               else {
+                       if (pp->fmt == CMIT_FMT_MBOXRD &&
+                                       !regexec(mboxrd_from, line, 0, 0, 0))
+                               strbuf_addch(sb, '>');
+
                        strbuf_add(sb, line, linelen);
+               }
                strbuf_addch(sb, '\n');
        }
 }
@@ -1750,14 +1779,14 @@ void pretty_print_commit(struct pretty_print_context 
*pp,
        encoding = get_log_output_encoding();
        msg = reencoded = logmsg_reencode(commit, NULL, encoding);
 
-       if (pp->fmt == CMIT_FMT_ONELINE || pp->fmt == CMIT_FMT_EMAIL)
+       if (pp->fmt == CMIT_FMT_ONELINE || cmit_fmt_is_mail(pp->fmt))
                indent = 0;
 
        /*
         * We need to check and emit Content-type: to mark it
         * as 8-bit if we haven't done so.
         */
-       if (pp->fmt == CMIT_FMT_EMAIL && need_8bit_cte == 0) {
+       if (cmit_fmt_is_mail(pp->fmt) && need_8bit_cte == 0) {
                int i, ch, in_body;
 
                for (in_body = i = 0; (ch = msg[i]); i++) {
@@ -1785,7 +1814,7 @@ void pretty_print_commit(struct pretty_print_context *pp,
        msg = skip_empty_lines(msg);
 
        /* These formats treat the title line specially. */
-       if (pp->fmt == CMIT_FMT_ONELINE || pp->fmt == CMIT_FMT_EMAIL)
+       if (pp->fmt == CMIT_FMT_ONELINE || cmit_fmt_is_mail(pp->fmt))
                pp_title_line(pp, &msg, sb, encoding, need_8bit_cte);
 
        beginning_of_body = sb->len;
@@ -1802,7 +1831,7 @@ void pretty_print_commit(struct pretty_print_context *pp,
         * format.  Make sure we did not strip the blank line
         * between the header and the body.
         */
-       if (pp->fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
+       if (cmit_fmt_is_mail(pp->fmt) && sb->len <= beginning_of_body)
                strbuf_addch(sb, '\n');
 
        unuse_commit_buffer(commit, reencoded);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 8049cad..0a2070e 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1565,4 +1565,31 @@ test_expect_success 'format-patch --base overrides 
format.useAutoBase' '
        test_cmp expected actual
 '
 
+test_expect_success 'format-patch --pretty=mboxrd' '
+       cat >msg <<-INPUT_END &&
+       mboxrd should escape the body
+
+       From could trip up a loose mbox parser
+       >From extra escape for reversibility
+       >>From extra escape for reversibility 2
+       from lower case not escaped
+       Fromm bad speling not escaped
+        From with leading space not escaped
+       INPUT_END
+
+       cat >expect <<-INPUT_END &&
+       >From could trip up a loose mbox parser
+       >>From extra escape for reversibility
+       >>>From extra escape for reversibility 2
+       from lower case not escaped
+       Fromm bad speling not escaped
+        From with leading space not escaped
+       INPUT_END
+
+       C=$(git commit-tree HEAD^^{tree} -p HEAD <msg) &&
+       git format-patch --pretty=mboxrd --stdout -1 $C~1..$C >patch &&
+       grep -A5 "^>From could trip up a loose mbox parser" patch >actual &&
+       test_cmp expect actual
+'
+
 test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to