commit a1f6baa5 (wrap long header lines) breaks my habit

commit a1f6baa5 (wrap long header lines) breaks my habit

[Stefan-W. Hahn]

Hello,

for rebasing I'm using normally

   git format-patch -k --stdout a..b | git am -k -3

With commit

  commit a1f6baa5c97abc8b579fa7ac7c4dc21971bdc048
  format-patch: wrap long header lines 
  (since >v1.7.4)

this isn't possible anymore for those patches which have more then 78
characters on the first line.

The wrapping of the lines in the commit message will not be seen when
commiting but when rebasing via format-patch.

Was this the intention of this change or was my sort of workflow just
not in focus?

Any suggestion?


Stefan
-- 
Stefan-W. Hahn                          It is easy to make things.
                                        It is hard to make things simple.

Re: commit a1f6baa5 (wrap long header lines) breaks my habit

[Junio C Hamano]

"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:

>    git format-patch -k --stdout a..b | git am -k -3

Why -k to am?

Re: commit a1f6baa5 (wrap long header lines) breaks my habit

[Stefan-W. Hahn]

Mail von Junio C Hamano, Tue, 24 May 2011 at 09:27:46 -0700:

> >    git format-patch -k --stdout a..b | git am -k -3
> 
> Why -k to am?

Just first "-k", and "git am -3". Wrong in mind here at home before my
computer.

Stefan

-- 
Stefan-W. Hahn                          It is easy to make things.
                                        It is hard to make things simple.

Re: commit a1f6baa5 (wrap long header lines) breaks my habit

[Jeff King]

On Tue, May 24, 2011 at 06:46:16PM +0200, Stefan-W. Hahn wrote:

> Mail von Junio C Hamano, Tue, 24 May 2011 at 09:27:46 -0700:
> 
> > >    git format-patch -k --stdout a..b | git am -k -3
> > 
> > Why -k to am?
> 
> Just first "-k", and "git am -3". Wrong in mind here at home before my
> computer.

Then it should preserve your long subject line just fine, as mailsplit
(called by "am") will reassemble the folded line according to rfc822
header folding rules.

With "am -k", it does keep the fold. This is an artifact of the original
behavior, where the folds were literally included from a multi-line
subject. We should probably stop doing that now that we fold on length
(and should probably embed newlines via rfc2047 encoding in
format-patch, at least with "-k", so that you can losslessly move
multi-line subjects between the two if you always use "-k").

-Peff

Re: commit a1f6baa5 (wrap long header lines) breaks my habit

[Stefan-W. Hahn]

Mail von Jeff King, Tue, 24 May 2011 at 16:07:16 -0400:

Hello,

> > > >    git format-patch -k --stdout a..b | git am -k -3
> > > 
> > > Why -k to am?
> > 
> > Just first "-k", and "git am -3". Wrong in mind here at home before my
> > computer.

Wrong. I really typed the second "-k". (Local intelligence in
fingers.)

> Then it should preserve your long subject line just fine, as mailsplit
> (called by "am") will reassemble the folded line according to rfc822
> header folding rules.
> 
> With "am -k", it does keep the fold. This is an artifact of the original
> behavior, where the folds were literally included from a multi-line

Correct, I checked this, so with

   git format-patch -k --stdout a..b | git am -3
(no second -k)

all is as before. 

Thanks for your clarification, sorry for the noise.

Stefan

Perhaps a little clarification like this:

commit e8069848dffe72579aa7f2c542e39fde9eab84b1
Author: Stefan-W. Hahn <stefan.hahn@s-hahn.de>
Date:   Wed May 25 17:33:03 2011 +0200

    format-patch: Clarify the behaviour of '-k'.
    
    Added clarification in documentation of of 'git format-patch'.
    
    When using 'git format-patch' together with 'git am' for rebasing,
    'git am' should be called without '-k' if long subject lines should be
    reassembled. This is neccesary, because a wrapping of long header lines
    was introduced with commit:
    
    commit a1f6baa5c97abc8b579fa7ac7c4dc21971bdc048
    format-patch: wrap long header lines
    
    Signed-off-by: Stefan-W. Hahn <stefan.hahn@s-hahn.de>

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index d13c9b2..4e62248 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -468,6 +468,16 @@ the current branch using 'git am' to cherry-pick them:
 ------------
 $ git format-patch -k --stdout R1..R2 | git am -3 -k
 ------------
++
+In this example the subject lines of the commits will be folded after
+78 characters and 'git am' will keep this folding.
++
+To preserve long subject lines, 'git am' will reassemble the folded
+lines according to rfc822 if called without '-k' like this:
++
+------------
+$ git format-patch -k --stdout R1..R2 | git am -3
+------------
 
 * Extract all commits which are in the current branch but not in the
 origin branch:

-- 
Stefan-W. Hahn                          It is easy to make things.
                                        It is hard to make things simple.

Re: commit a1f6baa5 (wrap long header lines) breaks my habit

[Jeff King]

On Wed, May 25, 2011 at 05:40:46PM +0200, Stefan-W. Hahn wrote:

> > With "am -k", it does keep the fold. This is an artifact of the original
> > behavior, where the folds were literally included from a multi-line
> 
> Correct, I checked this, so with
> 
>    git format-patch -k --stdout a..b | git am -3
> (no second -k)
> 
> all is as before. 
> 
> Thanks for your clarification, sorry for the noise.

Actually, I don't think it's noise. Look at the documentation patch you
suggest:

> index d13c9b2..4e62248 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -468,6 +468,16 @@ the current branch using 'git am' to cherry-pick them:
>  ------------
>  $ git format-patch -k --stdout R1..R2 | git am -3 -k
>  ------------
> ++
> +In this example the subject lines of the commits will be folded after
> +78 characters and 'git am' will keep this folding.

You are using "format-patch -k | am -k". Surely that should preserve
your subject, no matter the length, and the fact that we need to
document it is a sign that the behavior is simply wrong.

So I think we should do the following series instead.

  [1/3]: t: test subject handling in format-patch / am pipeline
  [2/3]: mailinfo: always clean up rfc822 header folding
  [3/3]: format-patch: preserve subject newlines with -k

-Peff

[PATCH 1/3] t: test subject handling in format-patch / am pipeline

[Jeff King]

Commit a1f6baa (format-patch: wrap long header lines,
2011-02-23) changed format-patch's behavior with respect to
long header lines, but made no accompanying changes to the
receiving side. It was thought that "git am" would handle
these folded subjects fine, but there is a regression when
using "am -k".

Let's add a test documenting this. While we're at it, let's
give more complete test coverage to document what should be
happening in each case. We test three types of subjects:
a short one, one long enough to require wrapping, and a
multiline subject. For each, we test these three
combinations:

  format-patch | am
  format-patch -k | am
  format-patch -k | am -k

We don't bother testing "format-patch | am -k", which is
nonsense (you will be adding in [PATCH] cruft to each
subject).

This reveals the regression above (long subjects have
linebreaks introduced via "format-patch -k | am -k"),
as well as an existing non-optimal behavior (multiline
subjects are not preserved using "-k").

Signed-off-by: Jeff King <peff@peff.net>
---
This can go straight on top of the jk/format-patch-multiline-header
which went into v1.7.5 (the tip was c22e7de).

 t/t4152-am-subjects.sh |   77 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 77 insertions(+), 0 deletions(-)
 create mode 100755 t/t4152-am-subjects.sh

diff --git a/t/t4152-am-subjects.sh b/t/t4152-am-subjects.sh
new file mode 100755
index 0000000..7222c06
--- /dev/null
+++ b/t/t4152-am-subjects.sh
@@ -0,0 +1,77 @@
+#!/bin/sh
+
+test_description='test subject preservation with format-patch | am'
+. ./test-lib.sh
+
+make_patches() {
+	type=$1
+	subject=$2
+	test_expect_success "create patches with $type subject" '
+		git reset --hard baseline &&
+		echo $type >file &&
+		git commit -a -m "$subject" &&
+		git format-patch -1 --stdout >$type.patch &&
+		git format-patch -1 --stdout -k >$type-k.patch
+	'
+}
+
+check_subject() {
+	git reset --hard baseline &&
+	git am $2 $1.patch &&
+	git log -1 --pretty=format:%B >actual &&
+	test_cmp expect actual
+}
+
+test_expect_success 'setup baseline commit' '
+	test_commit baseline file
+'
+
+SHORT_SUBJECT='short subject'
+make_patches short "$SHORT_SUBJECT"
+
+LONG_SUBJECT1='this is a long subject that is virtually guaranteed'
+LONG_SUBJECT2='to require wrapping via format-patch if it is all'
+LONG_SUBJECT3='going to appear on a single line'
+LONG_SUBJECT="$LONG_SUBJECT1 $LONG_SUBJECT2 $LONG_SUBJECT3"
+make_patches long "$LONG_SUBJECT"
+
+MULTILINE_SUBJECT="$LONG_SUBJECT1
+$LONG_SUBJECT2
+$LONG_SUBJECT3"
+make_patches multiline "$MULTILINE_SUBJECT"
+
+echo "$SHORT_SUBJECT" >expect
+test_expect_success 'short subject preserved (format-patch | am)' '
+	check_subject short
+'
+test_expect_success 'short subject preserved (format-patch -k | am)' '
+	check_subject short-k
+'
+test_expect_success 'short subject preserved (format-patch -k | am -k)' '
+	check_subject short-k -k
+'
+
+echo "$LONG_SUBJECT" >expect
+test_expect_success 'long subject preserved (format-patch | am)' '
+	check_subject long
+'
+test_expect_success 'long subject preserved (format-patch -k | am)' '
+	check_subject long-k
+'
+test_expect_failure 'long subject preserved (format-patch -k | am -k)' '
+	check_subject long-k -k
+'
+
+echo "$LONG_SUBJECT" >expect
+test_expect_success 'multiline subject unwrapped (format-patch | am)' '
+	check_subject multiline
+'
+test_expect_success 'multiline subject unwrapped (format-patch -k | am)' '
+	check_subject multiline-k
+'
+echo "$MULTILINE_SUBJECT" >expect
+test_expect_failure 'multiline subject preserved (format-patch -k | am -k)' '
+	check_subject multiline-k -k
+'
+
+test_done
-- 
1.7.4.5.26.g0c6a2

[PATCH 2/3] mailinfo: always clean up rfc822 header folding

[Jeff King]

Without the "-k" option, mailinfo will convert a folded
subject header like:

  Subject: this is a
    subject that doesn't
    fit on one line

into a single line. With "-k", however, we assumed that
these newlines were significant and represented something
that the sending side would want us to preserve.

For messages created by format-patch, this assumption was
broken by a1f6baa (format-patch: wrap long header lines,
2011-02-23).  For messages sent by arbitrary MUAs, this was
probably never a good assumption to make, as they may have
been folding subjects in accordance with rfc822's line
length recommendations all along.

This patch now joins folded lines with a single whitespace
character. This treats header folding purely as a syntactic
feature of the transport mechanism, not as something that
format-patch is trying to tell us about the original
subject.

Signed-off-by: Jeff King <peff@peff.net>
---
The astute reader will notice that even with this patch, there is still
a regression when using new versions of format-patch (with a1f6baa) with
an older version of "git am". But you only see it when using "am -k", so
interoperability is probably not a huge deal:

  1. Before this patch, "am -k" was arguably broken anyway for applying
     random patches via email, since MUAs may have been doing arbitrary
     header folding. So we can probably discount people running "am -k"
     on random input as insane.

  2. People doing "git format-patch -k | git am -k" will presumably use
     the same version for both, and are OK.

  3. People doing "git format-patch -k >file", followed by upgrading
     git, and t hen "git am file" are still OK, since the newer version
     of "am" handles the output of both old and new format-patch.

  4. The problematic case is "git format-patch -k >file" with v1.7.5 or
     newer, then _downgrading_ git, then using "git am -k" to apply. Or
     more likely, using a newer version to create an mbox, shipping the
     mbox to another machine, and then using an older "git am" to apply.

So I don't see it as all that likely a problem in practice. If we do
care, we can't fix it with a simple patch. We would have to revert the
header-folding from format-patch, fix am, wait N time units until all of
the old "am" no longer exists, and then re-apply.

 builtin/mailinfo.c     |    2 +-
 t/t4152-am-subjects.sh |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 71e6262..bfb32b7 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -400,7 +400,7 @@ static int read_one_header_line(struct strbuf *line, FILE *in)
 			break;
 		if (strbuf_getline(&continuation, in, '\n'))
 			break;
-		continuation.buf[0] = '\n';
+		continuation.buf[0] = ' ';
 		strbuf_rtrim(&continuation);
 		strbuf_addbuf(line, &continuation);
 	}
diff --git a/t/t4152-am-subjects.sh b/t/t4152-am-subjects.sh
index 7222c06..37e5c03 100755
--- a/t/t4152-am-subjects.sh
+++ b/t/t4152-am-subjects.sh
@@ -58,7 +58,7 @@ test_expect_success 'long subject preserved (format-patch | am)' '
 test_expect_success 'long subject preserved (format-patch -k | am)' '
 	check_subject long-k
 '
-test_expect_failure 'long subject preserved (format-patch -k | am -k)' '
+test_expect_success 'long subject preserved (format-patch -k | am -k)' '
 	check_subject long-k -k
 '
 
-- 
1.7.4.5.26.g0c6a2

[PATCH 3/3] format-patch: preserve subject newlines with -k

[Jeff King]

In older versions of git, we used rfc822 header folding to
indicate that the original subject line had multiple lines
in it.  But since a1f6baa (format-patch: wrap long header
lines, 2011-02-23), we now use header folding whenever there
is a long line.

This means that "git am" cannot trust header folding as a
sign from format-patch that newlines should be preserved.
Instead, format-patch needs to signal more explicitly that
the newlines are significant.  This patch does so by
rfc2047-encoding the newlines in the subject line. No
changes are needed on the "git am" end; it already decodes
the newlines properly.

Signed-off-by: Jeff King <peff@peff.net>
---
We have always treated multi-line subjects as second-class citizens, so
this is not a must-have patch. But I think it makes sense to do,
considering how simple it is, and the fact that it makes "format-patch
-k | am -k" always a no-op, even with multi-line subjects.

 builtin/log.c          |    3 ++-
 commit.h               |    4 +++-
 log-tree.c             |    1 +
 pretty.c               |    8 +++++---
 revision.h             |    3 ++-
 t/t4152-am-subjects.sh |    2 +-
 6 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index d8c6c28..3fdf488 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -768,7 +768,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,
 		     encoding);
 	pp_title_line(CMIT_FMT_EMAIL, &msg, &sb, subject_start, extra_headers,
-		      encoding, need_8bit_cte);
+		      encoding, need_8bit_cte, 0);
 	pp_remainder(CMIT_FMT_EMAIL, &msg, &sb, 0);
 	printf("%s\n", sb.buf);
 
@@ -1130,6 +1130,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		die ("-n and -k are mutually exclusive.");
 	if (keep_subject && subject_prefix)
 		die ("--subject-prefix and -k are mutually exclusive.");
+	rev.preserve_subject = keep_subject;
 
 	argc = setup_revisions(argc, argv, &rev, &s_r_opt);
 	if (argc > 1)
diff --git a/commit.h b/commit.h
index eb6c5af..30cb7bc 100644
--- a/commit.h
+++ b/commit.h
@@ -73,6 +73,7 @@ struct pretty_print_context
 	int abbrev;
 	const char *subject;
 	const char *after_subject;
+	int preserve_subject;
 	enum date_mode date_mode;
 	int need_8bit_cte;
 	int show_notes;
@@ -107,7 +108,8 @@ void pp_title_line(enum cmit_fmt fmt,
 		   const char *subject,
 		   const char *after_subject,
 		   const char *encoding,
-		   int need_8bit_cte);
+		   int need_8bit_cte,
+		   int preserve_lines);
 void pp_remainder(enum cmit_fmt fmt,
 		  const char **msg_p,
 		  struct strbuf *sb,
diff --git a/log-tree.c b/log-tree.c
index b46ed3b..9b9aaf2 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -504,6 +504,7 @@ void show_log(struct rev_info *opt)
 	ctx.date_mode = opt->date_mode;
 	ctx.abbrev = opt->diffopt.abbrev;
 	ctx.after_subject = extra_headers;
+	ctx.preserve_subject = opt->preserve_subject;
 	ctx.reflog_info = opt->reflog_info;
 	pretty_print_commit(opt->commit_format, commit, &msgbuf, &ctx);
 
diff --git a/pretty.c b/pretty.c
index 65d20a7..315f1d2 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1121,12 +1121,13 @@ void pp_title_line(enum cmit_fmt fmt,
 		   const char *subject,
 		   const char *after_subject,
 		   const char *encoding,
-		   int need_8bit_cte)
+		   int need_8bit_cte,
+		   int preserve_lines)
 {
 	struct strbuf title;
 
 	strbuf_init(&title, 80);
-	*msg_p = format_subject(&title, *msg_p, " ");
+	*msg_p = format_subject(&title, *msg_p, preserve_lines ? "\n" : " ");
 
 	strbuf_grow(sb, title.len + 1024);
 	if (subject) {
@@ -1254,7 +1255,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	/* These formats treat the title line specially. */
 	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
 		pp_title_line(fmt, &msg, sb, context->subject,
-			      context->after_subject, encoding, need_8bit_cte);
+			      context->after_subject, encoding, need_8bit_cte,
+			      context->preserve_subject);
 
 	beginning_of_body = sb->len;
 	if (fmt != CMIT_FMT_ONELINE)
diff --git a/revision.h b/revision.h
index 05659c6..f8ddd83 100644
--- a/revision.h
+++ b/revision.h
@@ -90,7 +90,8 @@ struct rev_info {
 			abbrev_commit:1,
 			use_terminator:1,
 			missing_newline:1,
-			date_mode_explicit:1;
+			date_mode_explicit:1,
+			preserve_subject:1;
 	unsigned int	disable_stdin:1;
 
 	enum date_mode date_mode;
diff --git a/t/t4152-am-subjects.sh b/t/t4152-am-subjects.sh
index 37e5c03..4c68245 100755
--- a/t/t4152-am-subjects.sh
+++ b/t/t4152-am-subjects.sh
@@ -70,7 +70,7 @@ test_expect_success 'multiline subject unwrapped (format-patch -k | am)' '
 	check_subject multiline-k
 '
 echo "$MULTILINE_SUBJECT" >expect
-test_expect_failure 'multiline subject preserved (format-patch -k | am -k)' '
+test_expect_success 'multiline subject preserved (format-patch -k | am -k)' '
 	check_subject multiline-k -k
 '
 
-- 
1.7.4.5.26.g0c6a2

Re: [PATCH 3/3] format-patch: preserve subject newlines with -k

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> We have always treated multi-line subjects as second-class citizens, so
> this is not a must-have patch. But I think it makes sense to do,
> considering how simple it is, and the fact that it makes "format-patch
> -k | am -k" always a no-op, even with multi-line subjects.

I think this is a good thing to have, as the reason why we treated
multi-line subjects as second-class citizens is exactly because we didn't
try to stuff multiple lines on "Subject:" like you did with this patch.

> diff --git a/builtin/log.c b/builtin/log.c
> index d8c6c28..3fdf488 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -768,7 +768,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
>  	pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,
>  		     encoding);
>  	pp_title_line(CMIT_FMT_EMAIL, &msg, &sb, subject_start, extra_headers,
> -		      encoding, need_8bit_cte);
> +		      encoding, need_8bit_cte, 0);

I do not appreciate a single-bit tweak as separate parameter to a
function.  Back when pp_title_line() had only "do we need 8-bit cte", it
was Ok, but now that you are adding another bit, could we make it an
"unsigned flag"?

Re: [PATCH 3/3] format-patch: preserve subject newlines with -k

[Jeff King]

On Thu, May 26, 2011 at 02:18:33PM -0700, Junio C Hamano wrote:

> > diff --git a/builtin/log.c b/builtin/log.c
> > index d8c6c28..3fdf488 100644
> > --- a/builtin/log.c
> > +++ b/builtin/log.c
> > @@ -768,7 +768,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
> >  	pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,
> >  		     encoding);
> >  	pp_title_line(CMIT_FMT_EMAIL, &msg, &sb, subject_start, extra_headers,
> > -		      encoding, need_8bit_cte);
> > +		      encoding, need_8bit_cte, 0);
> 
> I do not appreciate a single-bit tweak as separate parameter to a
> function.  Back when pp_title_line() had only "do we need 8-bit cte", it
> was Ok, but now that you are adding another bit, could we make it an
> "unsigned flag"?

Actually, I wonder if we can refactor to just pass the pretty_context to
pp_title_line. Let me see what I can do.

-Peff

Re: [PATCH 3/3] format-patch: preserve subject newlines with -k

[Jeff King]

On Thu, May 26, 2011 at 05:19:52PM -0400, Jeff King wrote:

> > >  	pp_title_line(CMIT_FMT_EMAIL, &msg, &sb, subject_start, extra_headers,
> > > -		      encoding, need_8bit_cte);
> > > +		      encoding, need_8bit_cte, 0);
> > 
> > I do not appreciate a single-bit tweak as separate parameter to a
> > function.  Back when pp_title_line() had only "do we need 8-bit cte", it
> > was Ok, but now that you are adding another bit, could we make it an
> > "unsigned flag"?
> 
> Actually, I wonder if we can refactor to just pass the pretty_context to
> pp_title_line. Let me see what I can do.

It ends up being a lot of lines changed, but I think the result is more
readable. Replace my 3/3 with (1/5 and 2/5 are the same as before):

  [3/5]: pretty: add pp_commit_easy function for simple callers
  [4/5]: clean up calling conventions for pretty.c functions
  [5/5]: format-patch: preserve subject newlines with -k

-Peff

[PATCH 3/5] pretty: add pp_commit_easy function for simple callers

[Jeff King]

Many callers don't actually care about the pretty print
context at all; let's just give them a simple way of
pretty-printing a commit without having to create a context
struct.

Signed-off-by: Jeff King <peff@peff.net>
---
This trades off per-call lines for some infrastructure lines. It's a
slight lose in line count now, but as more callers are added, it may be
a win. But I think even now it's overall easier to read.

 builtin/branch.c      |    4 +---
 builtin/checkout.c    |    3 +--
 builtin/log.c         |    4 +---
 builtin/shortlog.c    |    3 +--
 builtin/show-branch.c |    3 +--
 commit.h              |    2 ++
 pretty.c              |    7 +++++++
 7 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 9e546e4..d8f1522 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -436,9 +436,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
 
 		commit = item->commit;
 		if (commit && !parse_commit(commit)) {
-			struct pretty_print_context ctx = {0};
-			pretty_print_commit(CMIT_FMT_ONELINE, commit,
-					    &subject, &ctx);
+			pp_commit_easy(CMIT_FMT_ONELINE, commit, &subject);
 			sub = subject.buf;
 		}
 
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 757f9a0..c1759dc 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -300,9 +300,8 @@ static void show_local_changes(struct object *head, struct diff_options *opts)
 static void describe_detached_head(char *msg, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
-	struct pretty_print_context ctx = {0};
 	parse_commit(commit);
-	pretty_print_commit(CMIT_FMT_ONELINE, commit, &sb, &ctx);
+	pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
 	fprintf(stderr, "%s %s... %s\n", msg,
 		find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV), sb.buf);
 	strbuf_release(&sb);
diff --git a/builtin/log.c b/builtin/log.c
index d8c6c28..cedfdb6 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1439,9 +1439,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 
 		if (verbose) {
 			struct strbuf buf = STRBUF_INIT;
-			struct pretty_print_context ctx = {0};
-			pretty_print_commit(CMIT_FMT_ONELINE, commit,
-					    &buf, &ctx);
+			pp_commit_easy(CMIT_FMT_ONELINE, commit, &buf);
 			printf("%c %s %s\n", sign,
 			       find_unique_abbrev(commit->object.sha1, abbrev),
 			       buf.buf);
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 1a21e4b..90877b5 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -141,9 +141,8 @@ 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;
-	struct pretty_print_context ctx = {0};
 
-	pretty_print_commit(CMIT_FMT_RAW, commit, &buf, &ctx);
+	pp_commit_easy(CMIT_FMT_RAW, commit, &buf);
 	buffer = buf.buf;
 	while (*buffer && *buffer != '\n') {
 		const char *eol = strchr(buffer, '\n');
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index da69581..a5fc2aa 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -293,8 +293,7 @@ static void show_one_commit(struct commit *commit, int no_name)
 	struct commit_name *name = commit->util;
 
 	if (commit->object.parsed) {
-		struct pretty_print_context ctx = {0};
-		pretty_print_commit(CMIT_FMT_ONELINE, commit, &pretty, &ctx);
+		pp_commit_easy(CMIT_FMT_ONELINE, commit, &pretty);
 		pretty_str = pretty.buf;
 	}
 	if (!prefixcmp(pretty_str, "[PATCH] "))
diff --git a/commit.h b/commit.h
index eb6c5af..3e733be 100644
--- a/commit.h
+++ b/commit.h
@@ -98,6 +98,8 @@ extern void format_commit_message(const struct commit *commit,
 extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 				struct strbuf *sb,
 				const struct pretty_print_context *context);
+extern void pp_commit_easy(enum cmit_fmt fmt, const struct commit *commit,
+			   struct strbuf *sb);
 void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 		   const char *line, enum date_mode dmode,
 		   const char *encoding);
diff --git a/pretty.c b/pretty.c
index 65d20a7..38cd398 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1279,3 +1279,10 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 
 	free(reencoded);
 }
+
+void pp_commit_easy(enum cmit_fmt fmt, const struct commit *commit,
+		    struct strbuf *sb)
+{
+	struct pretty_print_context pp = {0};
+	pretty_print_commit(fmt, commit, sb, &pp);
+}
-- 
1.7.4.5.26.g0c6a2

Re: [PATCH 3/5] pretty: add pp_commit_easy function for simple callers

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Many callers don't actually care about the pretty print context at all;
> let's just give them a simple way of pretty-printing a commit without
> having to create a context struct.

Very nice ;-).

[PATCH 4/5] clean up calling conventions for pretty.c functions

[Jeff King]

We have a pretty_print_context representing the parameters
for a pretty-print session, but we did not use it uniformly.
As a result, functions kept growing more and more arguments.

Let's clean this up in a few ways:

  1. All pretty-print pp_* functions now take a context.
     This lets us reduce the number of arguments to these
     functions, since we were just passing around the
     context values separately.

  2. The context argument now has a cmit_fmt field, which
     was passed around separately. That's one less argument
     per function.

  3. The context argument always comes first, which makes
     calling a little more uniform.

This drops lines from some callers, and adds lines in a few
places (because we need an extra line to set the context's
fmt field). Overall, we don't save many lines, but the lines
that are there are a lot simpler and more readable.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/log.c      |   21 ++++++-----
 builtin/merge.c    |    3 +-
 builtin/rev-list.c |    3 +-
 builtin/shortlog.c |    3 +-
 commit.h           |   19 +++++-----
 log-tree.c         |    3 +-
 pretty.c           |   99 ++++++++++++++++++++++++---------------------------
 7 files changed, 75 insertions(+), 76 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index cedfdb6..8d842cb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -327,9 +327,11 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
 static void show_tagger(char *buf, int len, struct rev_info *rev)
 {
 	struct strbuf out = STRBUF_INIT;
+	struct pretty_print_context pp = {0};
 
-	pp_user_info("Tagger", rev->commit_format, &out, buf, rev->date_mode,
-		get_log_output_encoding());
+	pp.fmt = rev->commit_format;
+	pp.date_mode = rev->date_mode;
+	pp_user_info(&pp, "Tagger", &out, buf, get_log_output_encoding());
 	printf("%s", out.buf);
 	strbuf_release(&out);
 }
@@ -715,10 +717,8 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			      int nr, struct commit **list, struct commit *head)
 {
 	const char *committer;
-	const char *subject_start = NULL;
 	const char *body = "*** SUBJECT HERE ***\n\n*** BLURB HERE ***\n";
 	const char *msg;
-	const char *extra_headers = rev->extra_headers;
 	struct shortlog log;
 	struct strbuf sb = STRBUF_INIT;
 	int i;
@@ -726,6 +726,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	struct diff_options opts;
 	int need_8bit_cte = 0;
 	struct commit *commit = NULL;
+	struct pretty_print_context pp = {0};
 
 	if (rev->commit_format != CMIT_FMT_EMAIL)
 		die("Cover letter needs email format");
@@ -757,7 +758,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 		free(commit);
 	}
 
-	log_write_email_headers(rev, head, &subject_start, &extra_headers,
+	log_write_email_headers(rev, head, &pp.subject, &pp.after_subject,
 				&need_8bit_cte);
 
 	for (i = 0; !need_8bit_cte && i < nr; i++)
@@ -765,11 +766,11 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			need_8bit_cte = 1;
 
 	msg = body;
-	pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,
-		     encoding);
-	pp_title_line(CMIT_FMT_EMAIL, &msg, &sb, subject_start, extra_headers,
-		      encoding, need_8bit_cte);
-	pp_remainder(CMIT_FMT_EMAIL, &msg, &sb, 0);
+	pp.fmt = CMIT_FMT_EMAIL;
+	pp.date_mode = DATE_RFC2822;
+	pp_user_info(&pp, NULL, &sb, committer, encoding);
+	pp_title_line(&pp, &msg, &sb, encoding, need_8bit_cte);
+	pp_remainder(&pp, &msg, &sb, 0);
 	printf("%s\n", sb.buf);
 
 	strbuf_release(&sb);
diff --git a/builtin/merge.c b/builtin/merge.c
index 42fff38..c902e81 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -333,13 +333,14 @@ static void squash_message(void)
 
 	ctx.abbrev = rev.abbrev;
 	ctx.date_mode = rev.date_mode;
+	ctx.fmt = rev.commit_format;
 
 	strbuf_addstr(&out, "Squashed commit of the following:\n");
 	while ((commit = get_revision(&rev)) != NULL) {
 		strbuf_addch(&out, '\n');
 		strbuf_addf(&out, "commit %s\n",
 			sha1_to_hex(commit->object.sha1));
-		pretty_print_commit(rev.commit_format, commit, &out, &ctx);
+		pretty_print_commit(&ctx, commit, &out);
 	}
 	if (write(fd, out.buf, out.len) < 0)
 		die_errno("Writing SQUASH_MSG");
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index ba27d39..0ec42fc 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -108,7 +108,8 @@ static void show_commit(struct commit *commit, void *data)
 		struct pretty_print_context ctx = {0};
 		ctx.abbrev = revs->abbrev;
 		ctx.date_mode = revs->date_mode;
-		pretty_print_commit(revs->commit_format, commit, &buf, &ctx);
+		ctx.fmt = revs->commit_format;
+		pretty_print_commit(&ctx, commit, &buf);
 		if (revs->graph) {
 			if (buf.len) {
 				if (revs->commit_format != CMIT_FMT_ONELINE)
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 90877b5..074fd26 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -161,11 +161,12 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
 		    sha1_to_hex(commit->object.sha1));
 	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 = DATE_NORMAL;
-		pretty_print_commit(CMIT_FMT_USERFORMAT, commit, &ufbuf, &ctx);
+		pretty_print_commit(&ctx, commit, &ufbuf);
 		buffer = ufbuf.buf;
 	} else if (*buffer) {
 		buffer++;
diff --git a/commit.h b/commit.h
index 3e733be..2935740 100644
--- a/commit.h
+++ b/commit.h
@@ -70,6 +70,7 @@ enum cmit_fmt {
 
 struct pretty_print_context
 {
+	enum cmit_fmt fmt;
 	int abbrev;
 	const char *subject;
 	const char *after_subject;
@@ -95,22 +96,20 @@ extern void userformat_find_requirements(const char *fmt, struct userformat_want
 extern void format_commit_message(const struct commit *commit,
 				  const char *format, struct strbuf *sb,
 				  const struct pretty_print_context *context);
-extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
-				struct strbuf *sb,
-				const struct pretty_print_context *context);
+extern void pretty_print_commit(const struct pretty_print_context *pp,
+				const struct commit *commit,
+				struct strbuf *sb);
 extern void pp_commit_easy(enum cmit_fmt fmt, const struct commit *commit,
 			   struct strbuf *sb);
-void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
-		   const char *line, enum date_mode dmode,
-		   const char *encoding);
-void pp_title_line(enum cmit_fmt fmt,
+void pp_user_info(const struct pretty_print_context *pp,
+		  const char *what, struct strbuf *sb,
+		  const char *line, const char *encoding);
+void pp_title_line(const struct pretty_print_context *pp,
 		   const char **msg_p,
 		   struct strbuf *sb,
-		   const char *subject,
-		   const char *after_subject,
 		   const char *encoding,
 		   int need_8bit_cte);
-void pp_remainder(enum cmit_fmt fmt,
+void pp_remainder(const struct pretty_print_context *pp,
 		  const char **msg_p,
 		  struct strbuf *sb,
 		  int indent);
diff --git a/log-tree.c b/log-tree.c
index b46ed3b..0d8cc7a 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -505,7 +505,8 @@ void show_log(struct rev_info *opt)
 	ctx.abbrev = opt->diffopt.abbrev;
 	ctx.after_subject = extra_headers;
 	ctx.reflog_info = opt->reflog_info;
-	pretty_print_commit(opt->commit_format, commit, &msgbuf, &ctx);
+	ctx.fmt = opt->commit_format;
+	pretty_print_commit(&ctx, commit, &msgbuf);
 
 	if (opt->add_signoff)
 		append_signoff(&msgbuf, opt->add_signoff);
diff --git a/pretty.c b/pretty.c
index 38cd398..f920205 100644
--- a/pretty.c
+++ b/pretty.c
@@ -266,16 +266,16 @@ needquote:
 	strbuf_addstr(sb, "?=");
 }
 
-void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
-		  const char *line, enum date_mode dmode,
-		  const char *encoding)
+void pp_user_info(const struct pretty_print_context *pp,
+		  const char *what, struct strbuf *sb,
+		  const char *line, const char *encoding)
 {
 	char *date;
 	int namelen;
 	unsigned long time;
 	int tz;
 
-	if (fmt == CMIT_FMT_ONELINE)
+	if (pp->fmt == CMIT_FMT_ONELINE)
 		return;
 	date = strchr(line, '>');
 	if (!date)
@@ -284,7 +284,7 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 	time = strtoul(date, &date, 10);
 	tz = strtol(date, NULL, 10);
 
-	if (fmt == CMIT_FMT_EMAIL) {
+	if (pp->fmt == CMIT_FMT_EMAIL) {
 		char *name_tail = strchr(line, '<');
 		int display_name_length;
 		if (!name_tail)
@@ -298,18 +298,18 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 		strbuf_addch(sb, '\n');
 	} else {
 		strbuf_addf(sb, "%s: %.*s%.*s\n", what,
-			      (fmt == CMIT_FMT_FULLER) ? 4 : 0,
+			      (pp->fmt == CMIT_FMT_FULLER) ? 4 : 0,
 			      "    ", namelen, line);
 	}
-	switch (fmt) {
+	switch (pp->fmt) {
 	case CMIT_FMT_MEDIUM:
-		strbuf_addf(sb, "Date:   %s\n", show_date(time, tz, dmode));
+		strbuf_addf(sb, "Date:   %s\n", show_date(time, tz, pp->date_mode));
 		break;
 	case CMIT_FMT_EMAIL:
 		strbuf_addf(sb, "Date: %s\n", show_date(time, tz, DATE_RFC2822));
 		break;
 	case CMIT_FMT_FULLER:
-		strbuf_addf(sb, "%sDate: %s\n", what, show_date(time, tz, dmode));
+		strbuf_addf(sb, "%sDate: %s\n", what, show_date(time, tz, pp->date_mode));
 		break;
 	default:
 		/* notin' */
@@ -340,12 +340,12 @@ static const char *skip_empty_lines(const char *msg)
 	return msg;
 }
 
-static void add_merge_info(enum cmit_fmt fmt, struct strbuf *sb,
-			const struct commit *commit, int abbrev)
+static void add_merge_info(const struct pretty_print_context *pp,
+			   struct strbuf *sb, const struct commit *commit)
 {
 	struct commit_list *parent = commit->parents;
 
-	if ((fmt == CMIT_FMT_ONELINE) || (fmt == CMIT_FMT_EMAIL) ||
+	if ((pp->fmt == CMIT_FMT_ONELINE) || (pp->fmt == CMIT_FMT_EMAIL) ||
 	    !parent || !parent->next)
 		return;
 
@@ -354,8 +354,8 @@ static void add_merge_info(enum cmit_fmt fmt, struct strbuf *sb,
 	while (parent) {
 		struct commit *p = parent->item;
 		const char *hex = NULL;
-		if (abbrev)
-			hex = find_unique_abbrev(p->object.sha1, abbrev);
+		if (pp->abbrev)
+			hex = find_unique_abbrev(p->object.sha1, pp->abbrev);
 		if (!hex)
 			hex = sha1_to_hex(p->object.sha1);
 		parent = parent->next;
@@ -1052,9 +1052,7 @@ void format_commit_message(const struct commit *commit,
 		free(context.message);
 }
 
-static void pp_header(enum cmit_fmt fmt,
-		      int abbrev,
-		      enum date_mode dmode,
+static void pp_header(const struct pretty_print_context *pp,
 		      const char *encoding,
 		      const struct commit *commit,
 		      const char **msg_p,
@@ -1074,7 +1072,7 @@ static void pp_header(enum cmit_fmt fmt,
 			/* End of header */
 			return;
 
-		if (fmt == CMIT_FMT_RAW) {
+		if (pp->fmt == CMIT_FMT_RAW) {
 			strbuf_add(sb, line, linelen);
 			continue;
 		}
@@ -1094,7 +1092,7 @@ static void pp_header(enum cmit_fmt fmt,
 				;
 			/* with enough slop */
 			strbuf_grow(sb, num * 50 + 20);
-			add_merge_info(fmt, sb, commit, abbrev);
+			add_merge_info(pp, sb, commit);
 			parents_shown = 1;
 		}
 
@@ -1105,21 +1103,19 @@ static void pp_header(enum cmit_fmt fmt,
 		 */
 		if (!memcmp(line, "author ", 7)) {
 			strbuf_grow(sb, linelen + 80);
-			pp_user_info("Author", fmt, sb, line + 7, dmode, encoding);
+			pp_user_info(pp, "Author", sb, line + 7, encoding);
 		}
 		if (!memcmp(line, "committer ", 10) &&
-		    (fmt == CMIT_FMT_FULL || fmt == CMIT_FMT_FULLER)) {
+		    (pp->fmt == CMIT_FMT_FULL || pp->fmt == CMIT_FMT_FULLER)) {
 			strbuf_grow(sb, linelen + 80);
-			pp_user_info("Commit", fmt, sb, line + 10, dmode, encoding);
+			pp_user_info(pp, "Commit", sb, line + 10, encoding);
 		}
 	}
 }
 
-void pp_title_line(enum cmit_fmt fmt,
+void pp_title_line(const struct pretty_print_context *pp,
 		   const char **msg_p,
 		   struct strbuf *sb,
-		   const char *subject,
-		   const char *after_subject,
 		   const char *encoding,
 		   int need_8bit_cte)
 {
@@ -1129,8 +1125,8 @@ void pp_title_line(enum cmit_fmt fmt,
 	*msg_p = format_subject(&title, *msg_p, " ");
 
 	strbuf_grow(sb, title.len + 1024);
-	if (subject) {
-		strbuf_addstr(sb, subject);
+	if (pp->subject) {
+		strbuf_addstr(sb, pp->subject);
 		add_rfc2047(sb, title.buf, title.len, encoding);
 	} else {
 		strbuf_addbuf(sb, &title);
@@ -1144,16 +1140,16 @@ void pp_title_line(enum cmit_fmt fmt,
 			"Content-Transfer-Encoding: 8bit\n";
 		strbuf_addf(sb, header_fmt, encoding);
 	}
-	if (after_subject) {
-		strbuf_addstr(sb, after_subject);
+	if (pp->after_subject) {
+		strbuf_addstr(sb, pp->after_subject);
 	}
-	if (fmt == CMIT_FMT_EMAIL) {
+	if (pp->fmt == CMIT_FMT_EMAIL) {
 		strbuf_addch(sb, '\n');
 	}
 	strbuf_release(&title);
 }
 
-void pp_remainder(enum cmit_fmt fmt,
+void pp_remainder(const struct pretty_print_context *pp,
 		  const char **msg_p,
 		  struct strbuf *sb,
 		  int indent)
@@ -1170,7 +1166,7 @@ void pp_remainder(enum cmit_fmt fmt,
 		if (is_empty_line(line, &linelen)) {
 			if (first)
 				continue;
-			if (fmt == CMIT_FMT_SHORT)
+			if (pp->fmt == CMIT_FMT_SHORT)
 				break;
 		}
 		first = 0;
@@ -1195,19 +1191,19 @@ char *reencode_commit_message(const struct commit *commit, const char **encoding
 	return logmsg_reencode(commit, encoding);
 }
 
-void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
-			 struct strbuf *sb,
-			 const struct pretty_print_context *context)
+void pretty_print_commit(const struct pretty_print_context *pp,
+			 const struct commit *commit,
+			 struct strbuf *sb)
 {
 	unsigned long beginning_of_body;
 	int indent = 4;
 	const char *msg = commit->buffer;
 	char *reencoded;
 	const char *encoding;
-	int need_8bit_cte = context->need_8bit_cte;
+	int need_8bit_cte = pp->need_8bit_cte;
 
-	if (fmt == CMIT_FMT_USERFORMAT) {
-		format_commit_message(commit, user_format, sb, context);
+	if (pp->fmt == CMIT_FMT_USERFORMAT) {
+		format_commit_message(commit, user_format, sb, pp);
 		return;
 	}
 
@@ -1216,14 +1212,14 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 		msg = reencoded;
 	}
 
-	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
+	if (pp->fmt == CMIT_FMT_ONELINE || pp->fmt == CMIT_FMT_EMAIL)
 		indent = 0;
 
 	/*
 	 * We need to check and emit Content-type: to mark it
 	 * as 8-bit if we haven't done so.
 	 */
-	if (fmt == CMIT_FMT_EMAIL && need_8bit_cte == 0) {
+	if (pp->fmt == CMIT_FMT_EMAIL && need_8bit_cte == 0) {
 		int i, ch, in_body;
 
 		for (in_body = i = 0; (ch = msg[i]); i++) {
@@ -1242,9 +1238,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 		}
 	}
 
-	pp_header(fmt, context->abbrev, context->date_mode, encoding,
-		  commit, &msg, sb);
-	if (fmt != CMIT_FMT_ONELINE && !context->subject) {
+	pp_header(pp, encoding, commit, &msg, sb);
+	if (pp->fmt != CMIT_FMT_ONELINE && !pp->subject) {
 		strbuf_addch(sb, '\n');
 	}
 
@@ -1252,17 +1247,16 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	msg = skip_empty_lines(msg);
 
 	/* These formats treat the title line specially. */
-	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
-		pp_title_line(fmt, &msg, sb, context->subject,
-			      context->after_subject, encoding, need_8bit_cte);
+	if (pp->fmt == CMIT_FMT_ONELINE || pp->fmt == CMIT_FMT_EMAIL)
+		pp_title_line(pp, &msg, sb, encoding, need_8bit_cte);
 
 	beginning_of_body = sb->len;
-	if (fmt != CMIT_FMT_ONELINE)
-		pp_remainder(fmt, &msg, sb, indent);
+	if (pp->fmt != CMIT_FMT_ONELINE)
+		pp_remainder(pp, &msg, sb, indent);
 	strbuf_rtrim(sb);
 
 	/* Make sure there is an EOLN for the non-oneline case */
-	if (fmt != CMIT_FMT_ONELINE)
+	if (pp->fmt != CMIT_FMT_ONELINE)
 		strbuf_addch(sb, '\n');
 
 	/*
@@ -1270,10 +1264,10 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	 * format.  Make sure we did not strip the blank line
 	 * between the header and the body.
 	 */
-	if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
+	if (pp->fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
 		strbuf_addch(sb, '\n');
 
-	if (context->show_notes)
+	if (pp->show_notes)
 		format_display_notes(commit->object.sha1, sb, encoding,
 				     NOTES_SHOW_HEADER | NOTES_INDENT);
 
@@ -1284,5 +1278,6 @@ void pp_commit_easy(enum cmit_fmt fmt, const struct commit *commit,
 		    struct strbuf *sb)
 {
 	struct pretty_print_context pp = {0};
-	pretty_print_commit(fmt, commit, sb, &pp);
+	pp.fmt = fmt;
+	pretty_print_commit(&pp, commit, sb);
 }
-- 
1.7.4.5.26.g0c6a2

[PATCH 5/5] format-patch: preserve subject newlines with -k

[Jeff King]

In older versions of git, we used rfc822 header folding to
indicate that the original subject line had multiple lines
in it.  But since a1f6baa (format-patch: wrap long header
lines, 2011-02-23), we now use header folding whenever there
is a long line.

This means that "git am" cannot trust header folding as a
sign from format-patch that newlines should be preserved.
Instead, format-patch needs to signal more explicitly that
the newlines are significant.  This patch does so by
rfc2047-encoding the newlines in the subject line. No
changes are needed on the "git am" end; it already decodes
the newlines properly.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/log.c          |    1 +
 commit.h               |    1 +
 log-tree.c             |    1 +
 pretty.c               |    3 ++-
 revision.h             |    3 ++-
 t/t4152-am-subjects.sh |    2 +-
 6 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 8d842cb..0e46e5a 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1131,6 +1131,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		die ("-n and -k are mutually exclusive.");
 	if (keep_subject && subject_prefix)
 		die ("--subject-prefix and -k are mutually exclusive.");
+	rev.preserve_subject = keep_subject;
 
 	argc = setup_revisions(argc, argv, &rev, &s_r_opt);
 	if (argc > 1)
diff --git a/commit.h b/commit.h
index 2935740..e985dcc 100644
--- a/commit.h
+++ b/commit.h
@@ -74,6 +74,7 @@ struct pretty_print_context
 	int abbrev;
 	const char *subject;
 	const char *after_subject;
+	int preserve_subject;
 	enum date_mode date_mode;
 	int need_8bit_cte;
 	int show_notes;
diff --git a/log-tree.c b/log-tree.c
index 0d8cc7a..0c41789 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -504,6 +504,7 @@ void show_log(struct rev_info *opt)
 	ctx.date_mode = opt->date_mode;
 	ctx.abbrev = opt->diffopt.abbrev;
 	ctx.after_subject = extra_headers;
+	ctx.preserve_subject = opt->preserve_subject;
 	ctx.reflog_info = opt->reflog_info;
 	ctx.fmt = opt->commit_format;
 	pretty_print_commit(&ctx, commit, &msgbuf);
diff --git a/pretty.c b/pretty.c
index f920205..905a082 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1122,7 +1122,8 @@ void pp_title_line(const struct pretty_print_context *pp,
 	struct strbuf title;
 
 	strbuf_init(&title, 80);
-	*msg_p = format_subject(&title, *msg_p, " ");
+	*msg_p = format_subject(&title, *msg_p,
+				pp->preserve_subject ? "\n" : " ");
 
 	strbuf_grow(sb, title.len + 1024);
 	if (pp->subject) {
diff --git a/revision.h b/revision.h
index 05659c6..f8ddd83 100644
--- a/revision.h
+++ b/revision.h
@@ -90,7 +90,8 @@ struct rev_info {
 			abbrev_commit:1,
 			use_terminator:1,
 			missing_newline:1,
-			date_mode_explicit:1;
+			date_mode_explicit:1,
+			preserve_subject:1;
 	unsigned int	disable_stdin:1;
 
 	enum date_mode date_mode;
diff --git a/t/t4152-am-subjects.sh b/t/t4152-am-subjects.sh
index 37e5c03..4c68245 100755
--- a/t/t4152-am-subjects.sh
+++ b/t/t4152-am-subjects.sh
@@ -70,7 +70,7 @@ test_expect_success 'multiline subject unwrapped (format-patch -k | am)' '
 	check_subject multiline-k
 '
 echo "$MULTILINE_SUBJECT" >expect
-test_expect_failure 'multiline subject preserved (format-patch -k | am -k)' '
+test_expect_success 'multiline subject preserved (format-patch -k | am -k)' '
 	check_subject multiline-k -k
 '
 
-- 
1.7.4.5.26.g0c6a2