[PATCH] format-patch: teach --no-encode-headers

[PATCH] format-patch: teach --no-encode-headers

[Emma Brooks]

When commit subjects or authors have non-ASCII characters, git
format-patch Q-encodes them so they can be safely sent over email.
However, if the patch transfer method is something other than email (web
review tools, sneakernet), this only serves to make the patch metadata
harder to read without first applying it (unless you can decode RFC 2047
in your head). git am as well as some email software supports
non-Q-encoded mail as described in RFC 6531.

Add --[no-]encode-headers and format.encodeHeaders to let the user
control this behavior.

Signed-off-by: Emma Brooks <me@pluvano.com>
---
 Documentation/config/format.txt    |  4 +++
 Documentation/git-format-patch.txt |  7 ++++
 builtin/log.c                      |  7 ++++
 log-tree.c                         |  1 +
 pretty.c                           |  6 ++--
 pretty.h                           |  1 +
 revision.c                         |  4 +++
 revision.h                         |  3 +-
 t/t4014-format-patch.sh            | 53 ++++++++++++++++++++++++++++++
 9 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index 45c7bd5a8f..ee0eb4c5da 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -57,6 +57,10 @@ format.suffix::
 	`.patch`. Use this variable to change that suffix (make sure to
 	include the dot if you want it).
 
+format.encodeHeaders::
+	Encode email headers that have non-ASCII characters with
+	"Q-encoding" for email transmission. Defaults to true.
+
 format.pretty::
 	The default pretty format for log/show/whatchanged command,
 	See linkgit:git-log[1], linkgit:git-show[1],
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 0d4f8951bb..a1483a6a34 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -24,6 +24,7 @@ SYNOPSIS
 		   [(--reroll-count|-v) <n>]
 		   [--to=<email>] [--cc=<email>]
 		   [--[no-]cover-letter] [--quiet]
+		   [--[no-]encode-headers]
 		   [--no-notes | --notes[=<ref>]]
 		   [--interdiff=<previous>]
 		   [--range-diff=<previous> [--creation-factor=<percent>]]
@@ -253,6 +254,12 @@ feeding the result to `git send-email`.
 	containing the branch description, shortlog and the overall diffstat.  You can
 	fill in a description in the file before sending it out.
 
+--[no-]encode-headers::
+	Encode email headers that have non-ASCII characters with
+	"Q-encoding", instead of outputting the headers verbatim. The
+	default is `--encode-headers` unless the `format.encodeHeaders`
+	configuration variable is set.
+
 --interdiff=<previous>::
 	As a reviewer aid, insert an interdiff into the cover letter,
 	or as commentary of the lone patch of a 1-patch series, showing
diff --git a/builtin/log.c b/builtin/log.c
index 83a4a6188e..1a27049c88 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -46,6 +46,7 @@ static int default_abbrev_commit;
 static int default_show_root = 1;
 static int default_follow;
 static int default_show_signature;
+static int default_encode_headers = 1;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config = 1;
@@ -151,6 +152,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
 	rev->show_signature = default_show_signature;
+	rev->encode_headers = default_encode_headers;
 	rev->diffopt.flags.allow_textconv = 1;
 
 	if (default_date_mode)
@@ -438,6 +440,10 @@ static int git_log_config(const char *var, const char *value, void *cb)
 		return git_config_string(&fmt_pretty, var, value);
 	if (!strcmp(var, "format.subjectprefix"))
 		return git_config_string(&fmt_patch_subject_prefix, var, value);
+	if (!strcmp(var, "format.encodeheaders")) {
+		default_encode_headers = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "log.abbrevcommit")) {
 		default_abbrev_commit = git_config_bool(var, value);
 		return 0;
@@ -1719,6 +1725,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.show_notes = show_notes;
 	memcpy(&rev.notes_opt, &notes_opt, sizeof(notes_opt));
 	rev.commit_format = CMIT_FMT_EMAIL;
+	rev.encode_headers = default_encode_headers;
 	rev.expand_tabs_in_log_default = 0;
 	rev.verbose_header = 1;
 	rev.diff = 1;
diff --git a/log-tree.c b/log-tree.c
index 897a90233e..eaec299762 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -693,6 +693,7 @@ void show_log(struct rev_info *opt)
 	ctx.abbrev = opt->diffopt.abbrev;
 	ctx.after_subject = extra_headers;
 	ctx.preserve_subject = opt->preserve_subject;
+	ctx.encode_headers = opt->encode_headers;
 	ctx.reflog_info = opt->reflog_info;
 	ctx.fmt = opt->commit_format;
 	ctx.mailmap = opt->mailmap;
diff --git a/pretty.c b/pretty.c
index 28afc701b6..12959cca4d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -474,7 +474,8 @@ void pp_user_info(struct pretty_print_context *pp,
 		}
 
 		strbuf_addstr(sb, "From: ");
-		if (needs_rfc2047_encoding(namebuf, namelen)) {
+		if (pp->encode_headers &&
+				needs_rfc2047_encoding(namebuf, namelen)) {
 			add_rfc2047(sb, namebuf, namelen,
 				    encoding, RFC2047_ADDRESS);
 			max_length = 76; /* per rfc2047 */
@@ -1767,7 +1768,8 @@ void pp_title_line(struct pretty_print_context *pp,
 	if (pp->print_email_subject) {
 		if (pp->rev)
 			fmt_output_email_subject(sb, pp->rev);
-		if (needs_rfc2047_encoding(title.buf, title.len))
+		if (pp->encode_headers &&
+				needs_rfc2047_encoding(title.buf, title.len))
 			add_rfc2047(sb, title.buf, title.len,
 						encoding, RFC2047_SUBJECT);
 		else
diff --git a/pretty.h b/pretty.h
index 4ad1fc31ff..4840f7e559 100644
--- a/pretty.h
+++ b/pretty.h
@@ -43,6 +43,7 @@ struct pretty_print_context {
 	struct string_list *mailmap;
 	int color;
 	struct ident_split *from_ident;
+	unsigned encode_headers:1;
 
 	/*
 	 * Fields below here are manipulated internally by pp_* functions and
diff --git a/revision.c b/revision.c
index 8136929e23..961a901985 100644
--- a/revision.c
+++ b/revision.c
@@ -2241,6 +2241,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->topo_order = 1;
 		revs->rewrite_parents = 1;
 		revs->graph = graph_init(revs);
+	} else if (!strcmp(arg, "--encode-headers")) {
+		revs->encode_headers = 1;
+	} else if (!strcmp(arg, "--no-encode-headers")) {
+		revs->encode_headers = 0;
 	} else if (!strcmp(arg, "--root")) {
 		revs->show_root_diff = 1;
 	} else if (!strcmp(arg, "--no-commit-id")) {
diff --git a/revision.h b/revision.h
index 475f048fb6..e4dff23d62 100644
--- a/revision.h
+++ b/revision.h
@@ -203,7 +203,8 @@ struct rev_info {
 			use_terminator:1,
 			missing_newline:1,
 			date_mode_explicit:1,
-			preserve_subject:1;
+			preserve_subject:1,
+			encode_headers:1;
 	unsigned int	disable_stdin:1;
 	/* --show-linear-break */
 	unsigned int	track_linear:1,
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index b653dd7d44..d9c0fe7a45 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1160,6 +1160,59 @@ test_expect_success 'format-patch wraps extremely long from-header (rfc2047)' '
 	check_author "Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar"
 '
 
+cat >expect <<'EOF'
+From: Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
+ Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo
+ Bar Foo Bar Foo Bar Foo Bar <author@example.com>
+EOF
+test_expect_success 'format-patch wraps extremely long from-header (non-ASCII without Q-encoding)' '
+	echo content >>file &&
+	git add file &&
+	GIT_AUTHOR_NAME="Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar" \
+	git commit -m author-check &&
+	git format-patch --no-encode-headers --stdout -1 >patch &&
+	sed -n "/^From: /p; /^ /p; /^$/q" patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [PATCH] Foö
+EOF
+test_expect_success 'subject lines are unencoded with --no-encode-headers' '
+	echo content >>file &&
+	git add file &&
+	git commit -m "Foö" &&
+	git format-patch --no-encode-headers -1 --stdout >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [PATCH] Foö
+EOF
+test_expect_success 'subject lines are unencoded with format.encodeHeaders=false' '
+	echo content >>file &&
+	git add file &&
+	git commit -m "Foö" &&
+	git config format.encodeHeaders false &&
+	git format-patch -1 --stdout >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [PATCH] =?UTF-8?q?Fo=C3=B6?=
+EOF
+test_expect_success '--encode-headers overrides format.encodeHeaders' '
+	echo content >>file &&
+	git add file &&
+	git commit -m "Foö" &&
+	git config format.encodeHeaders false &&
+	git format-patch --encode-headers -1 --stdout >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
 cat >expect <<'EOF'
 Subject: header with . in it
 EOF

Re: [PATCH] format-patch: teach --no-encode-headers

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]

On 2020-04-05 at 23:11:09, Emma Brooks wrote:
> When commit subjects or authors have non-ASCII characters, git
> format-patch Q-encodes them so they can be safely sent over email.
> However, if the patch transfer method is something other than email (web
> review tools, sneakernet), this only serves to make the patch metadata
> harder to read without first applying it (unless you can decode RFC 2047
> in your head). git am as well as some email software supports
> non-Q-encoded mail as described in RFC 6531.

Do we always output UTF-8 in this case, or do we sometimes output other
encodings if the user has specified one for the commit message?  Do we
know how git send-email handles such a message if it receives one?

I know it isn't your intention to work with git send-email in this
patch, but it would be nice to know whether there's additional value in
someone sending a followup patch to make git send-email use SMTPUTF8 if
that's necessary.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH] format-patch: teach --no-encode-headers

[Jeff King]

On Mon, Apr 06, 2020 at 03:04:44AM +0000, brian m. carlson wrote:

> On 2020-04-05 at 23:11:09, Emma Brooks wrote:
> > When commit subjects or authors have non-ASCII characters, git
> > format-patch Q-encodes them so they can be safely sent over email.
> > However, if the patch transfer method is something other than email (web
> > review tools, sneakernet), this only serves to make the patch metadata
> > harder to read without first applying it (unless you can decode RFC 2047
> > in your head). git am as well as some email software supports
> > non-Q-encoded mail as described in RFC 6531.
> 
> Do we always output UTF-8 in this case, or do we sometimes output other
> encodings if the user has specified one for the commit message?

That was my first question, too. But I think even without this option,
we always respect i18n.logOutputEncoding before we even hit the email
pretty-printing code. So by default it would always be utf8 (and
otherwise whatever the user has asked us to output).

That would obviously be disastrous for an output encoding that isn't an
ASCII superset, but that's already true for any of our output formats.

> Do we know how git send-email handles such a message if it receives
> one?
> 
> I know it isn't your intention to work with git send-email in this
> patch, but it would be nice to know whether there's additional value in
> someone sending a followup patch to make git send-email use SMTPUTF8 if
> that's necessary.

I suspect this is mostly orthogonal, as that deals only with the
SMTP-level addresses, which include only the actual email part (not the
name) and aren't RFC2047-encoded anyway. It looks like we already leave
characters in addresses untouched (I'm not even 100% sure that RFC2047
allows modifying within the local part of an addr):

  $echo foo >file
  $ git add file
  $ git -c user.email=péff@peff.net commit -m foo
  $ git format-patch -1 --stdout | grep From:
  From: Jeff King <péff@peff.net>

I did wonder if there are any standards around 8bit headers. Certainly
the de facto standard for local tools (e.g., mutt reading a message
you've edited in vim) is that they can be treated like a stream of
ASCII-compatible bytes, and that works pretty well in practice. But if
there's an IETF-endorsed method for 8bit headers, it would be nice to
use it. For 8bit bodies, we're able to give a content-transfer-encoding
and a content-type with the charset. But I don't know of an equivalent
for headers.

-Peff

Re: [PATCH] format-patch: teach --no-encode-headers

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1840 bytes --]

On 2020-04-06 at 13:30:40, Jeff King wrote:
> I suspect this is mostly orthogonal, as that deals only with the
> SMTP-level addresses, which include only the actual email part (not the
> name) and aren't RFC2047-encoded anyway. It looks like we already leave
> characters in addresses untouched (I'm not even 100% sure that RFC2047
> allows modifying within the local part of an addr):
> 
>   $echo foo >file
>   $ git add file
>   $ git -c user.email=péff@peff.net commit -m foo
>   $ git format-patch -1 --stdout | grep From:
>   From: Jeff King <péff@peff.net>
> 
> I did wonder if there are any standards around 8bit headers. Certainly
> the de facto standard for local tools (e.g., mutt reading a message
> you've edited in vim) is that they can be treated like a stream of
> ASCII-compatible bytes, and that works pretty well in practice. But if
> there's an IETF-endorsed method for 8bit headers, it would be nice to
> use it. For 8bit bodies, we're able to give a content-transfer-encoding
> and a content-type with the charset. But I don't know of an equivalent
> for headers.

That's RFC 6532, Internationalized Email Headers, the companion document
to RFC 6531.  (The RFC editor has cleverly kept the last digits in sync
between the RFC 532x and 653x series).

The basic summary is that header field names are not internationalized,
but the field values do allow UTF-8 if they contain unstructured text
(e.g., Subject), anything using atoms (e.g., Message-ID), quoted strings
(e.g., local-parts of an email address), domains, and a few other
constructs.  RFC 2047 (MIME encoded words) is allowed "only in a subset
of the places allowed by" RFC 6532, so just not encoding should be safe
here, as long as it's UTF-8.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH] format-patch: teach --no-encode-headers

[Jeff King]

On Mon, Apr 06, 2020 at 03:17:34PM +0000, brian m. carlson wrote:

> > I did wonder if there are any standards around 8bit headers. Certainly
> > the de facto standard for local tools (e.g., mutt reading a message
> > you've edited in vim) is that they can be treated like a stream of
> > ASCII-compatible bytes, and that works pretty well in practice. But if
> > there's an IETF-endorsed method for 8bit headers, it would be nice to
> > use it. For 8bit bodies, we're able to give a content-transfer-encoding
> > and a content-type with the charset. But I don't know of an equivalent
> > for headers.
> 
> That's RFC 6532, Internationalized Email Headers, the companion document
> to RFC 6531.  (The RFC editor has cleverly kept the last digits in sync
> between the RFC 532x and 653x series).

Ah, thanks, that's exactly what I was looking for.

> The basic summary is that header field names are not internationalized,
> but the field values do allow UTF-8 if they contain unstructured text
> (e.g., Subject), anything using atoms (e.g., Message-ID), quoted strings
> (e.g., local-parts of an email address), domains, and a few other
> constructs.  RFC 2047 (MIME encoded words) is allowed "only in a subset
> of the places allowed by" RFC 6532, so just not encoding should be safe
> here, as long as it's UTF-8.

That makes sense. It looks like such messages are technically
message/global rather than message/rfc822. But since there's no
content-type given for the outermost message of an mbox, I guess that
just becomes implied.

The utf8 thing means that doing:

  git format-patch --encoding=iso8859-1 --no-encode-headers

violates the standard. But I think that's OK. If you really prefer that
charset for your local use, it does what you want. And if you try to
send it over SMTP and somebody complains, I think that falls under "if
it hurts, don't do that".

-Peff

Re: [PATCH] format-patch: teach --no-encode-headers

[Junio C Hamano]

Emma Brooks <me@pluvano.com> writes:

> When commit subjects or authors have non-ASCII characters, git
> format-patch Q-encodes them so they can be safely sent over email.
> However, if the patch transfer method is something other than email (web
> review tools, sneakernet), this only serves to make the patch metadata
> harder to read without first applying it (unless you can decode RFC 2047
> in your head). git am as well as some email software supports
> non-Q-encoded mail as described in RFC 6531.
>
> Add --[no-]encode-headers and format.encodeHeaders to let the user
> control this behavior.

This would be immensely useful.  I often find the in-body headers
that are Q-encoded too ugly to live.

Is the RFC2047 the only thing we do to message headers?  What I am
trying is to figure out if "encode-headers - yes/no?" would be a
stable (iow, would we be gaining other kinds of encoding over time?)
and well-defined (iow, is there a case where one kind of 'encoding'
is still desirable while disabling other kinds of 'encoding' is
wanted?) question.  If there is any doubt in your answers to the
above question, we may have to make sure the name of the option
makes it clear to users what kind of encoding we're talking about.

> +format.encodeHeaders::
> +	Encode email headers that have non-ASCII characters with
> +	"Q-encoding" for email transmission. Defaults to true.

OK.

>  
> +--[no-]encode-headers::

I think we'd want to standadize on writing these out, i.e.

        --encode-headers::
        --no-encode-headers::

so let's follow that when adding a new option.

> +	Encode email headers that have non-ASCII characters with
> +	"Q-encoding", instead of outputting the headers verbatim. The

I wonder if calling RFC2047 out helps readers here, when they wonder
what Q is and how they can decipher it.

> +	default is `--encode-headers` unless the `format.encodeHeaders`
> +	configuration variable is set.

I am wondering if we can go even shorter, e.g.

	The default is set to the value of `format.encodeHeaders`
	configuration variable.

> -		if (needs_rfc2047_encoding(namebuf, namelen)) {
> +		if (pp->encode_headers &&
> +				needs_rfc2047_encoding(namebuf, namelen)) {

Don't overly indent the second line like this.  The same comment
applies to the next hunk (not quoted).

Thanks.

Re: [PATCH] format-patch: teach --no-encode-headers

[Emma Brooks]

On 2020-04-05 20:29:57-0700, Junio C Hamano wrote:
> Is the RFC2047 the only thing we do to message headers?  What I am
> trying is to figure out if "encode-headers - yes/no?" would be a
> stable (iow, would we be gaining other kinds of encoding over time?)
> and well-defined (iow, is there a case where one kind of 'encoding'
> is still desirable while disabling other kinds of 'encoding' is
> wanted?) question.  If there is any doubt in your answers to the
> above question, we may have to make sure the name of the option
> makes it clear to users what kind of encoding we're talking about.

It's also too vague and it's not entirely clear from the option itself
what sort of encoding it refers to. I will change it to
--[no-]q-encode-headers and format.qEncodeHeaders in v2 unless there are
other suggestions.

> > +--[no-]encode-headers::
> 
> I think we'd want to standadize on writing these out, i.e.
> 
>         --encode-headers::
>         --no-encode-headers::
> 
> so let's follow that when adding a new option.

OK.

> > +	Encode email headers that have non-ASCII characters with
> > +	"Q-encoding", instead of outputting the headers verbatim. The
> 
> I wonder if calling RFC2047 out helps readers here, when they wonder
> what Q is and how they can decipher it.

I'll reference the RFC directly in v2.

> > +	default is `--encode-headers` unless the `format.encodeHeaders`
> > +	configuration variable is set.
> 
> I am wondering if we can go even shorter, e.g.
> 
> 	The default is set to the value of `format.encodeHeaders`
> 	configuration variable.

OK, I'll go with that.

> > -		if (needs_rfc2047_encoding(namebuf, namelen)) {
> > +		if (pp->encode_headers &&
> > +				needs_rfc2047_encoding(namebuf, namelen)) {
> 
> Don't overly indent the second line like this.  The same comment
> applies to the next hunk (not quoted).
> 
> Thanks.

OK.

Re: [PATCH] format-patch: teach --no-encode-headers

[Junio C Hamano]

Emma Brooks <me@pluvano.com> writes:

> It's also too vague and it's not entirely clear from the option itself
> what sort of encoding it refers to. I will change it to
> --[no-]q-encode-headers and format.qEncodeHeaders in v2 unless there are
> other suggestions.

I actually did not mean to push you into that direction.  We can,
and do want to, keep the most generic "--[no-]encode-headers" if we
do not anticipate us wanting to special case the Q encoding.  A
sample question to ask is "would it make sense to disable q-encoding
but still perform other parts of 'encode headers'?"  I haven't
thought deeply about such questions, but as a proposer of this
topic, you would certainly have, and I was hoping that you'd say
things like "Q-encoding is the only thing that we do to munge
headers, so there aren't any 'other parts of encoding headers' we
need to worry about", "there are things like X, Y and Z that we do
to the headers when we enable Q-encoding, but they all are what we
do not want when we do not want the Q-encoding", which would be a
very good sign that assures us that "--[no-]encode-headers" is a
good name.

Thanks.

Re: [PATCH] format-patch: teach --no-encode-headers

[Jeff King]

On Tue, Apr 07, 2020 at 12:37:31PM -0700, Junio C Hamano wrote:

> Emma Brooks <me@pluvano.com> writes:
> 
> > It's also too vague and it's not entirely clear from the option itself
> > what sort of encoding it refers to. I will change it to
> > --[no-]q-encode-headers and format.qEncodeHeaders in v2 unless there are
> > other suggestions.
> 
> I actually did not mean to push you into that direction.  We can,
> and do want to, keep the most generic "--[no-]encode-headers" if we
> do not anticipate us wanting to special case the Q encoding.  A
> sample question to ask is "would it make sense to disable q-encoding
> but still perform other parts of 'encode headers'?"  I haven't
> thought deeply about such questions, but as a proposer of this
> topic, you would certainly have, and I was hoping that you'd say
> things like "Q-encoding is the only thing that we do to munge
> headers, so there aren't any 'other parts of encoding headers' we
> need to worry about", "there are things like X, Y and Z that we do
> to the headers when we enable Q-encoding, but they all are what we
> do not want when we do not want the Q-encoding", which would be a
> very good sign that assures us that "--[no-]encode-headers" is a
> good name.

I thought we might b-encode some headers, but couldn't find any code to
do so (after about 5 minutes of looking).

However, this new option isn't just for format-patch. It is available
for all revision walkers (as it should be; I can say "log
--format=email" and I might want to use it there). And there "headers"
is less clear that we are talking about email headers, and not other
object headers (e.g., that you might see with --format=raw).

Saying "--no-rfc2047-encoding" would be more descriptive to _me_, but I
wonder if people not so familiar with the standards would find it a bit
obscure. Another option is to invert it to "--8bit-email-headers" or
something.

-Peff

Re: [PATCH] format-patch: teach --no-encode-headers

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Saying "--no-rfc2047-encoding" would be more descriptive to _me_, but I
> wonder if people not so familiar with the standards would find it a bit
> obscure. Another option is to invert it to "--8bit-email-headers" or
> something.

Yup, having "rfc2047" in the name of the option was one of the
things I considered suggesting, but I didn't for the same reason.  I
am OK with "--[no-]8bit-email-headers" (when --8bit, rfc2047 is
skipped).  Or "--[no-]email-header-encoding".

Thanks.

Re: [PATCH] format-patch: teach --no-encode-headers

[Emma Brooks]

On 2020-04-07 12:37:31-0700, Junio C Hamano wrote:
> Emma Brooks <me@pluvano.com> writes:
> 
> > It's also too vague and it's not entirely clear from the option itself
> > what sort of encoding it refers to. I will change it to
> > --[no-]q-encode-headers and format.qEncodeHeaders in v2 unless there are
> > other suggestions.
> 
> I actually did not mean to push you into that direction.  We can,
> and do want to, keep the most generic "--[no-]encode-headers" if we
> do not anticipate us wanting to special case the Q encoding.  A
> sample question to ask is "would it make sense to disable q-encoding
> but still perform other parts of 'encode headers'?"  I haven't
> thought deeply about such questions, but as a proposer of this
> topic, you would certainly have, and I was hoping that you'd say
> things like "Q-encoding is the only thing that we do to munge
> headers, so there aren't any 'other parts of encoding headers' we
> need to worry about", "there are things like X, Y and Z that we do
> to the headers when we enable Q-encoding, but they all are what we
> do not want when we do not want the Q-encoding", which would be a
> very good sign that assures us that "--[no-]encode-headers" is a
> good name.

Ah. I don't think there are any cases where we do other sorts of
encoding, or want to enable one "part" of encoding and disable another.
I do think the name need to be more obviously about *email* headers as
Jeff pointed out, though.

[PATCH v2] format-patch: teach --no-q-encode-headers

[Emma Brooks]

When commit subjects or authors have non-ASCII characters, git
format-patch Q-encodes them so they can be safely sent over email.
However, if the patch transfer method is something other than email (web
review tools, sneakernet), this only serves to make the patch metadata
harder to read without first applying it (unless you can decode RFC 2047
in your head). git am as well as some email software supports
non-Q-encoded mail as described in RFC 6531.

Add --[no-]q-encode-headers and format.qEncodeHeaders to let the user
control this behavior.

Signed-off-by: Emma Brooks <me@pluvano.com>
---

Changes since v1:

- Rename --encode-headers to --q-encode-headers
- Rename format.encodeHeaders to format.qEncodeHeaders
- Simplify language in docs
- Reel in over-indented lines

 Documentation/config/format.txt    |  4 +++
 Documentation/git-format-patch.txt |  8 +++++
 builtin/log.c                      |  7 ++++
 log-tree.c                         |  1 +
 pretty.c                           |  6 ++--
 pretty.h                           |  1 +
 revision.c                         |  4 +++
 revision.h                         |  3 +-
 t/t4014-format-patch.sh            | 53 ++++++++++++++++++++++++++++++
 9 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index 45c7bd5a8f..9b0d665d27 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -57,6 +57,10 @@ format.suffix::
 	`.patch`. Use this variable to change that suffix (make sure to
 	include the dot if you want it).
 
+format.qEncodeHeaders::
+	Encode email headers that have non-ASCII characters with
+	"Q-encoding" for email transmission. Defaults to true.
+
 format.pretty::
 	The default pretty format for log/show/whatchanged command,
 	See linkgit:git-log[1], linkgit:git-show[1],
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 0d4f8951bb..13de39a27c 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -24,6 +24,7 @@ SYNOPSIS
 		   [(--reroll-count|-v) <n>]
 		   [--to=<email>] [--cc=<email>]
 		   [--[no-]cover-letter] [--quiet]
+		   [--[no-]q-encode-headers]
 		   [--no-notes | --notes[=<ref>]]
 		   [--interdiff=<previous>]
 		   [--range-diff=<previous> [--creation-factor=<percent>]]
@@ -253,6 +254,13 @@ feeding the result to `git send-email`.
 	containing the branch description, shortlog and the overall diffstat.  You can
 	fill in a description in the file before sending it out.
 
+--q-encode-headers::
+--no-q-encode-headers::
+	Encode email headers that have non-ASCII characters with
+	"Q-encoding" (described in RFC 2047), instead of outputting the
+	headers verbatim. The default is set to the value of the
+	`format.qEncodeHeaders` configuration variable.
+
 --interdiff=<previous>::
 	As a reviewer aid, insert an interdiff into the cover letter,
 	or as commentary of the lone patch of a 1-patch series, showing
diff --git a/builtin/log.c b/builtin/log.c
index 83a4a6188e..ea63902b82 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -46,6 +46,7 @@ static int default_abbrev_commit;
 static int default_show_root = 1;
 static int default_follow;
 static int default_show_signature;
+static int default_q_encode_headers = 1;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config = 1;
@@ -151,6 +152,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
 	rev->show_signature = default_show_signature;
+	rev->q_encode_headers = default_q_encode_headers;
 	rev->diffopt.flags.allow_textconv = 1;
 
 	if (default_date_mode)
@@ -438,6 +440,10 @@ static int git_log_config(const char *var, const char *value, void *cb)
 		return git_config_string(&fmt_pretty, var, value);
 	if (!strcmp(var, "format.subjectprefix"))
 		return git_config_string(&fmt_patch_subject_prefix, var, value);
+	if (!strcmp(var, "format.qencodeheaders")) {
+		default_q_encode_headers = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "log.abbrevcommit")) {
 		default_abbrev_commit = git_config_bool(var, value);
 		return 0;
@@ -1719,6 +1725,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.show_notes = show_notes;
 	memcpy(&rev.notes_opt, &notes_opt, sizeof(notes_opt));
 	rev.commit_format = CMIT_FMT_EMAIL;
+	rev.q_encode_headers = default_q_encode_headers;
 	rev.expand_tabs_in_log_default = 0;
 	rev.verbose_header = 1;
 	rev.diff = 1;
diff --git a/log-tree.c b/log-tree.c
index 897a90233e..fb37149854 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -693,6 +693,7 @@ void show_log(struct rev_info *opt)
 	ctx.abbrev = opt->diffopt.abbrev;
 	ctx.after_subject = extra_headers;
 	ctx.preserve_subject = opt->preserve_subject;
+	ctx.q_encode_headers = opt->q_encode_headers;
 	ctx.reflog_info = opt->reflog_info;
 	ctx.fmt = opt->commit_format;
 	ctx.mailmap = opt->mailmap;
diff --git a/pretty.c b/pretty.c
index 28afc701b6..3ae52cc9db 100644
--- a/pretty.c
+++ b/pretty.c
@@ -474,7 +474,8 @@ void pp_user_info(struct pretty_print_context *pp,
 		}
 
 		strbuf_addstr(sb, "From: ");
-		if (needs_rfc2047_encoding(namebuf, namelen)) {
+		if (pp->q_encode_headers &&
+		    needs_rfc2047_encoding(namebuf, namelen)) {
 			add_rfc2047(sb, namebuf, namelen,
 				    encoding, RFC2047_ADDRESS);
 			max_length = 76; /* per rfc2047 */
@@ -1767,7 +1768,8 @@ void pp_title_line(struct pretty_print_context *pp,
 	if (pp->print_email_subject) {
 		if (pp->rev)
 			fmt_output_email_subject(sb, pp->rev);
-		if (needs_rfc2047_encoding(title.buf, title.len))
+		if (pp->q_encode_headers &&
+		    needs_rfc2047_encoding(title.buf, title.len))
 			add_rfc2047(sb, title.buf, title.len,
 						encoding, RFC2047_SUBJECT);
 		else
diff --git a/pretty.h b/pretty.h
index 4ad1fc31ff..e40b6d3e83 100644
--- a/pretty.h
+++ b/pretty.h
@@ -43,6 +43,7 @@ struct pretty_print_context {
 	struct string_list *mailmap;
 	int color;
 	struct ident_split *from_ident;
+	unsigned q_encode_headers:1;
 
 	/*
 	 * Fields below here are manipulated internally by pp_* functions and
diff --git a/revision.c b/revision.c
index 8136929e23..51d3e8a558 100644
--- a/revision.c
+++ b/revision.c
@@ -2241,6 +2241,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->topo_order = 1;
 		revs->rewrite_parents = 1;
 		revs->graph = graph_init(revs);
+	} else if (!strcmp(arg, "--q-encode-headers")) {
+		revs->q_encode_headers = 1;
+	} else if (!strcmp(arg, "--no-q-encode-headers")) {
+		revs->q_encode_headers = 0;
 	} else if (!strcmp(arg, "--root")) {
 		revs->show_root_diff = 1;
 	} else if (!strcmp(arg, "--no-commit-id")) {
diff --git a/revision.h b/revision.h
index 475f048fb6..4794aa4742 100644
--- a/revision.h
+++ b/revision.h
@@ -203,7 +203,8 @@ struct rev_info {
 			use_terminator:1,
 			missing_newline:1,
 			date_mode_explicit:1,
-			preserve_subject:1;
+			preserve_subject:1,
+			q_encode_headers:1;
 	unsigned int	disable_stdin:1;
 	/* --show-linear-break */
 	unsigned int	track_linear:1,
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index b653dd7d44..9f780b8fe8 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1160,6 +1160,59 @@ test_expect_success 'format-patch wraps extremely long from-header (rfc2047)' '
 	check_author "Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar"
 '
 
+cat >expect <<'EOF'
+From: Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
+ Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo
+ Bar Foo Bar Foo Bar Foo Bar <author@example.com>
+EOF
+test_expect_success 'format-patch wraps extremely long from-header (non-ASCII without Q-encoding)' '
+	echo content >>file &&
+	git add file &&
+	GIT_AUTHOR_NAME="Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar" \
+	git commit -m author-check &&
+	git format-patch --no-q-encode-headers --stdout -1 >patch &&
+	sed -n "/^From: /p; /^ /p; /^$/q" patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [PATCH] Foö
+EOF
+test_expect_success 'subject lines are unencoded with --no-q-encode-headers' '
+	echo content >>file &&
+	git add file &&
+	git commit -m "Foö" &&
+	git format-patch --no-q-encode-headers -1 --stdout >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [PATCH] Foö
+EOF
+test_expect_success 'subject lines are unencoded with format.qEncodeHeaders=false' '
+	echo content >>file &&
+	git add file &&
+	git commit -m "Foö" &&
+	git config format.qEncodeHeaders false &&
+	git format-patch -1 --stdout >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [PATCH] =?UTF-8?q?Fo=C3=B6?=
+EOF
+test_expect_success '--q-encode-headers overrides format.qEncodeHeaders' '
+	echo content >>file &&
+	git add file &&
+	git commit -m "Foö" &&
+	git config format.qEncodeHeaders false &&
+	git format-patch --q-encode-headers -1 --stdout >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
 cat >expect <<'EOF'
 Subject: header with . in it
 EOF

Re: [PATCH v2] format-patch: teach --no-q-encode-headers

[Danh Doan]

On 2020-04-07 05:17:45+0000, Emma Brooks <me@pluvano.com> wrote:
> When commit subjects or authors have non-ASCII characters, git
> format-patch Q-encodes them so they can be safely sent over email.
> However, if the patch transfer method is something other than email (web
> review tools, sneakernet), this only serves to make the patch metadata
> harder to read without first applying it (unless you can decode RFC 2047
> in your head). git am as well as some email software supports
> non-Q-encoded mail as described in RFC 6531.
> 
> Add --[no-]q-encode-headers and format.qEncodeHeaders to let the user
> control this behavior.
> 
> Signed-off-by: Emma Brooks <me@pluvano.com>
> ---
> 
> Changes since v1:
> 
> - Rename --encode-headers to --q-encode-headers
> - Rename format.encodeHeaders to format.qEncodeHeaders
> - Simplify language in docs
> - Reel in over-indented lines
> 
>  Documentation/config/format.txt    |  4 +++
>  Documentation/git-format-patch.txt |  8 +++++
>  builtin/log.c                      |  7 ++++
>  log-tree.c                         |  1 +
>  pretty.c                           |  6 ++--
>  pretty.h                           |  1 +
>  revision.c                         |  4 +++
>  revision.h                         |  3 +-
>  t/t4014-format-patch.sh            | 53 ++++++++++++++++++++++++++++++
>  9 files changed, 84 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
> index 45c7bd5a8f..9b0d665d27 100644
> --- a/Documentation/config/format.txt
> +++ b/Documentation/config/format.txt
> @@ -57,6 +57,10 @@ format.suffix::
>  	`.patch`. Use this variable to change that suffix (make sure to
>  	include the dot if you want it).
>  
> +format.qEncodeHeaders::
> +	Encode email headers that have non-ASCII characters with
> +	"Q-encoding" for email transmission. Defaults to true.

I think have (described in RFC 2047) as same as
--[no]-q-encode-headers belows would be better.

> +
>  format.pretty::
>  	The default pretty format for log/show/whatchanged command,
>  	See linkgit:git-log[1], linkgit:git-show[1],
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 0d4f8951bb..13de39a27c 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -24,6 +24,7 @@ SYNOPSIS
>  		   [(--reroll-count|-v) <n>]
>  		   [--to=<email>] [--cc=<email>]
>  		   [--[no-]cover-letter] [--quiet]
> +		   [--[no-]q-encode-headers]
>  		   [--no-notes | --notes[=<ref>]]
>  		   [--interdiff=<previous>]
>  		   [--range-diff=<previous> [--creation-factor=<percent>]]
> @@ -253,6 +254,13 @@ feeding the result to `git send-email`.
>  	containing the branch description, shortlog and the overall diffstat.  You can
>  	fill in a description in the file before sending it out.
>  
> +--q-encode-headers::
> +--no-q-encode-headers::
> +	Encode email headers that have non-ASCII characters with
> +	"Q-encoding" (described in RFC 2047), instead of outputting the
> +	headers verbatim. The default is set to the value of the
> +	`format.qEncodeHeaders` configuration variable.

I find this paragraph hard to understand.
I think:

	If this option is omited, set to value of of ...

is easier to understand (to me, at least).

> +
>  --interdiff=<previous>::
>  	As a reviewer aid, insert an interdiff into the cover letter,
>  	or as commentary of the lone patch of a 1-patch series, showing
> diff --git a/builtin/log.c b/builtin/log.c
> index 83a4a6188e..ea63902b82 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -46,6 +46,7 @@ static int default_abbrev_commit;
>  static int default_show_root = 1;
>  static int default_follow;
>  static int default_show_signature;
> +static int default_q_encode_headers = 1;
>  static int decoration_style;
>  static int decoration_given;
>  static int use_mailmap_config = 1;
> @@ -151,6 +152,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
>  	rev->show_root_diff = default_show_root;
>  	rev->subject_prefix = fmt_patch_subject_prefix;
>  	rev->show_signature = default_show_signature;
> +	rev->q_encode_headers = default_q_encode_headers;
>  	rev->diffopt.flags.allow_textconv = 1;
>  
>  	if (default_date_mode)
> @@ -438,6 +440,10 @@ static int git_log_config(const char *var, const char *value, void *cb)
>  		return git_config_string(&fmt_pretty, var, value);
>  	if (!strcmp(var, "format.subjectprefix"))
>  		return git_config_string(&fmt_patch_subject_prefix, var, value);
> +	if (!strcmp(var, "format.qencodeheaders")) {
> +		default_q_encode_headers = git_config_bool(var, value);
> +		return 0;
> +	}
>  	if (!strcmp(var, "log.abbrevcommit")) {
>  		default_abbrev_commit = git_config_bool(var, value);
>  		return 0;
> @@ -1719,6 +1725,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  	rev.show_notes = show_notes;
>  	memcpy(&rev.notes_opt, &notes_opt, sizeof(notes_opt));
>  	rev.commit_format = CMIT_FMT_EMAIL;
> +	rev.q_encode_headers = default_q_encode_headers;
>  	rev.expand_tabs_in_log_default = 0;
>  	rev.verbose_header = 1;
>  	rev.diff = 1;
> diff --git a/log-tree.c b/log-tree.c
> index 897a90233e..fb37149854 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -693,6 +693,7 @@ void show_log(struct rev_info *opt)
>  	ctx.abbrev = opt->diffopt.abbrev;
>  	ctx.after_subject = extra_headers;
>  	ctx.preserve_subject = opt->preserve_subject;
> +	ctx.q_encode_headers = opt->q_encode_headers;
>  	ctx.reflog_info = opt->reflog_info;
>  	ctx.fmt = opt->commit_format;
>  	ctx.mailmap = opt->mailmap;
> diff --git a/pretty.c b/pretty.c
> index 28afc701b6..3ae52cc9db 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -474,7 +474,8 @@ void pp_user_info(struct pretty_print_context *pp,
>  		}
>  
>  		strbuf_addstr(sb, "From: ");
> -		if (needs_rfc2047_encoding(namebuf, namelen)) {
> +		if (pp->q_encode_headers &&
> +		    needs_rfc2047_encoding(namebuf, namelen)) {
>  			add_rfc2047(sb, namebuf, namelen,
>  				    encoding, RFC2047_ADDRESS);
>  			max_length = 76; /* per rfc2047 */
> @@ -1767,7 +1768,8 @@ void pp_title_line(struct pretty_print_context *pp,
>  	if (pp->print_email_subject) {
>  		if (pp->rev)
>  			fmt_output_email_subject(sb, pp->rev);
> -		if (needs_rfc2047_encoding(title.buf, title.len))
> +		if (pp->q_encode_headers &&
> +		    needs_rfc2047_encoding(title.buf, title.len))
>  			add_rfc2047(sb, title.buf, title.len,
>  						encoding, RFC2047_SUBJECT);
>  		else
> diff --git a/pretty.h b/pretty.h
> index 4ad1fc31ff..e40b6d3e83 100644
> --- a/pretty.h
> +++ b/pretty.h
> @@ -43,6 +43,7 @@ struct pretty_print_context {
>  	struct string_list *mailmap;
>  	int color;
>  	struct ident_split *from_ident;
> +	unsigned q_encode_headers:1;
>  
>  	/*
>  	 * Fields below here are manipulated internally by pp_* functions and
> diff --git a/revision.c b/revision.c
> index 8136929e23..51d3e8a558 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2241,6 +2241,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>  		revs->topo_order = 1;
>  		revs->rewrite_parents = 1;
>  		revs->graph = graph_init(revs);
> +	} else if (!strcmp(arg, "--q-encode-headers")) {
> +		revs->q_encode_headers = 1;
> +	} else if (!strcmp(arg, "--no-q-encode-headers")) {
> +		revs->q_encode_headers = 0;
>  	} else if (!strcmp(arg, "--root")) {
>  		revs->show_root_diff = 1;
>  	} else if (!strcmp(arg, "--no-commit-id")) {
> diff --git a/revision.h b/revision.h
> index 475f048fb6..4794aa4742 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -203,7 +203,8 @@ struct rev_info {
>  			use_terminator:1,
>  			missing_newline:1,
>  			date_mode_explicit:1,
> -			preserve_subject:1;
> +			preserve_subject:1,
> +			q_encode_headers:1;
>  	unsigned int	disable_stdin:1;
>  	/* --show-linear-break */
>  	unsigned int	track_linear:1,
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index b653dd7d44..9f780b8fe8 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1160,6 +1160,59 @@ test_expect_success 'format-patch wraps extremely long from-header (rfc2047)' '
>  	check_author "Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar"
>  '
>  
> +cat >expect <<'EOF'
> +From: Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
> + Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo
> + Bar Foo Bar Foo Bar Foo Bar <author@example.com>
> +EOF
> +test_expect_success 'format-patch wraps extremely long from-header (non-ASCII without Q-encoding)' '
> +	echo content >>file &&
> +	git add file &&
> +	GIT_AUTHOR_NAME="Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar" \
> +	git commit -m author-check &&
> +	git format-patch --no-q-encode-headers --stdout -1 >patch &&
> +	sed -n "/^From: /p; /^ /p; /^$/q" patch >actual &&
> +	test_cmp expect actual
> +'
> +
> +cat >expect <<'EOF'
> +Subject: [PATCH] Foö
> +EOF

In most of tests, we use this here doc inside the test instead of
global scope.

Only in this file and a minority others, this style is used.

I'm not sure which one is prefered,
maybe Junio can decide.

> +test_expect_success 'subject lines are unencoded with --no-q-encode-headers' '
> +	echo content >>file &&
> +	git add file &&
> +	git commit -m "Foö" &&

IIUC, test_commit is prefered to those above 3 lines.

> +	git format-patch --no-q-encode-headers -1 --stdout >patch &&
> +	grep ^Subject: patch >actual &&
> +	test_cmp expect actual
> +'
> +
> +cat >expect <<'EOF'
> +Subject: [PATCH] Foö
> +EOF
> +test_expect_success 'subject lines are unencoded with format.qEncodeHeaders=false' '
> +	echo content >>file &&
> +	git add file &&
> +	git commit -m "Foö" &&
> +	git config format.qEncodeHeaders false &&
> +	git format-patch -1 --stdout >patch &&
> +	grep ^Subject: patch >actual &&
> +	test_cmp expect actual
> +'
> +
> +cat >expect <<'EOF'
> +Subject: [PATCH] =?UTF-8?q?Fo=C3=B6?=
> +EOF
> +test_expect_success '--q-encode-headers overrides format.qEncodeHeaders' '
> +	echo content >>file &&
> +	git add file &&
> +	git commit -m "Foö" &&
> +	git config format.qEncodeHeaders false &&
> +	git format-patch --q-encode-headers -1 --stdout >patch &&
> +	grep ^Subject: patch >actual &&
> +	test_cmp expect actual
> +'
> +
>  cat >expect <<'EOF'
>  Subject: header with . in it
>  EOF

-- 
Danh

Re: [PATCH v2] format-patch: teach --no-q-encode-headers

[Emma Brooks]

On 2020-04-07 14:40:31+0700, Danh Doan wrote:
> > +format.qEncodeHeaders::
> > +	Encode email headers that have non-ASCII characters with
> > +	"Q-encoding" for email transmission. Defaults to true.
> 
> I think have (described in RFC 2047) as same as
> --[no]-q-encode-headers belows would be better.

Agreed. I will reference it there as well.

> > +--q-encode-headers::
> > +--no-q-encode-headers::
> > +	Encode email headers that have non-ASCII characters with
> > +	"Q-encoding" (described in RFC 2047), instead of outputting the
> > +	headers verbatim. The default is set to the value of the
> > +	`format.qEncodeHeaders` configuration variable.
> 
> I find this paragraph hard to understand.
> I think:
> 
> 	If this option is omited, set to value of of ...
> 
> is easier to understand (to me, at least).

OK, I will simplify the language in v3.

[PATCH v3] format-patch: teach --no-encode-email-headers

[Emma Brooks]

When commit subjects or authors have non-ASCII characters, git
format-patch Q-encodes them so they can be safely sent over email.
However, if the patch transfer method is something other than email (web
review tools, sneakernet), this only serves to make the patch metadata
harder to read without first applying it (unless you can decode RFC 2047
in your head). git am as well as some email software supports
non-Q-encoded mail as described in RFC 6531.

Add --[no-]encode-email-headers and format.encodeEmailHeaders to let the
user control this behavior.

Signed-off-by: Emma Brooks <me@pluvano.com>
---

Changes since v2:

- Rename options to to --[no-]encode-email-headers /
  format.encodeEmailHeaders so it's clear they refer to email headers.
- Reference RFC 2047 in config docs too
- Simplify language in docs more

 Documentation/config/format.txt    |  5 +++
 Documentation/git-format-patch.txt |  8 +++++
 builtin/log.c                      |  7 ++++
 log-tree.c                         |  1 +
 pretty.c                           |  6 ++--
 pretty.h                           |  1 +
 revision.c                         |  4 +++
 revision.h                         |  3 +-
 t/t4014-format-patch.sh            | 53 ++++++++++++++++++++++++++++++
 9 files changed, 85 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index 45c7bd5a8f..564e8091ba 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -57,6 +57,11 @@ format.suffix::
 	`.patch`. Use this variable to change that suffix (make sure to
 	include the dot if you want it).
 
+format.encodeEmailHeaders::
+	Encode email headers that have non-ASCII characters with
+	"Q-encoding" (described in RFC 2047) for email transmission.
+	Defaults to true.
+
 format.pretty::
 	The default pretty format for log/show/whatchanged command,
 	See linkgit:git-log[1], linkgit:git-show[1],
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 0d4f8951bb..0f81d0437b 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -24,6 +24,7 @@ SYNOPSIS
 		   [(--reroll-count|-v) <n>]
 		   [--to=<email>] [--cc=<email>]
 		   [--[no-]cover-letter] [--quiet]
+		   [--[no-]encode-email-headers]
 		   [--no-notes | --notes[=<ref>]]
 		   [--interdiff=<previous>]
 		   [--range-diff=<previous> [--creation-factor=<percent>]]
@@ -253,6 +254,13 @@ feeding the result to `git send-email`.
 	containing the branch description, shortlog and the overall diffstat.  You can
 	fill in a description in the file before sending it out.
 
+--encode-email-headers::
+--no-encode-email-headers::
+	Encode email headers that have non-ASCII characters with
+	"Q-encoding" (described in RFC 2047), instead of outputting the
+	headers verbatim. Defaults to the value of the
+	`format.encodeEmailHeaders` configuration variable.
+
 --interdiff=<previous>::
 	As a reviewer aid, insert an interdiff into the cover letter,
 	or as commentary of the lone patch of a 1-patch series, showing
diff --git a/builtin/log.c b/builtin/log.c
index 83a4a6188e..a5c3ace9a0 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -46,6 +46,7 @@ static int default_abbrev_commit;
 static int default_show_root = 1;
 static int default_follow;
 static int default_show_signature;
+static int default_encode_email_headers = 1;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config = 1;
@@ -151,6 +152,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
 	rev->show_signature = default_show_signature;
+	rev->encode_email_headers = default_encode_email_headers;
 	rev->diffopt.flags.allow_textconv = 1;
 
 	if (default_date_mode)
@@ -438,6 +440,10 @@ static int git_log_config(const char *var, const char *value, void *cb)
 		return git_config_string(&fmt_pretty, var, value);
 	if (!strcmp(var, "format.subjectprefix"))
 		return git_config_string(&fmt_patch_subject_prefix, var, value);
+	if (!strcmp(var, "format.encodeemailheaders")) {
+		default_encode_email_headers = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "log.abbrevcommit")) {
 		default_abbrev_commit = git_config_bool(var, value);
 		return 0;
@@ -1719,6 +1725,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.show_notes = show_notes;
 	memcpy(&rev.notes_opt, &notes_opt, sizeof(notes_opt));
 	rev.commit_format = CMIT_FMT_EMAIL;
+	rev.encode_email_headers = default_encode_email_headers;
 	rev.expand_tabs_in_log_default = 0;
 	rev.verbose_header = 1;
 	rev.diff = 1;
diff --git a/log-tree.c b/log-tree.c
index 897a90233e..0064788b25 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -693,6 +693,7 @@ void show_log(struct rev_info *opt)
 	ctx.abbrev = opt->diffopt.abbrev;
 	ctx.after_subject = extra_headers;
 	ctx.preserve_subject = opt->preserve_subject;
+	ctx.encode_email_headers = opt->encode_email_headers;
 	ctx.reflog_info = opt->reflog_info;
 	ctx.fmt = opt->commit_format;
 	ctx.mailmap = opt->mailmap;
diff --git a/pretty.c b/pretty.c
index 28afc701b6..2a3d46bf42 100644
--- a/pretty.c
+++ b/pretty.c
@@ -474,7 +474,8 @@ void pp_user_info(struct pretty_print_context *pp,
 		}
 
 		strbuf_addstr(sb, "From: ");
-		if (needs_rfc2047_encoding(namebuf, namelen)) {
+		if (pp->encode_email_headers &&
+		    needs_rfc2047_encoding(namebuf, namelen)) {
 			add_rfc2047(sb, namebuf, namelen,
 				    encoding, RFC2047_ADDRESS);
 			max_length = 76; /* per rfc2047 */
@@ -1767,7 +1768,8 @@ void pp_title_line(struct pretty_print_context *pp,
 	if (pp->print_email_subject) {
 		if (pp->rev)
 			fmt_output_email_subject(sb, pp->rev);
-		if (needs_rfc2047_encoding(title.buf, title.len))
+		if (pp->encode_email_headers &&
+		    needs_rfc2047_encoding(title.buf, title.len))
 			add_rfc2047(sb, title.buf, title.len,
 						encoding, RFC2047_SUBJECT);
 		else
diff --git a/pretty.h b/pretty.h
index 4ad1fc31ff..071f2fb8e4 100644
--- a/pretty.h
+++ b/pretty.h
@@ -43,6 +43,7 @@ struct pretty_print_context {
 	struct string_list *mailmap;
 	int color;
 	struct ident_split *from_ident;
+	unsigned encode_email_headers:1;
 
 	/*
 	 * Fields below here are manipulated internally by pp_* functions and
diff --git a/revision.c b/revision.c
index 8136929e23..58f5826df0 100644
--- a/revision.c
+++ b/revision.c
@@ -2241,6 +2241,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->topo_order = 1;
 		revs->rewrite_parents = 1;
 		revs->graph = graph_init(revs);
+	} else if (!strcmp(arg, "--encode-email-headers")) {
+		revs->encode_email_headers = 1;
+	} else if (!strcmp(arg, "--no-encode-email-headers")) {
+		revs->encode_email_headers = 0;
 	} else if (!strcmp(arg, "--root")) {
 		revs->show_root_diff = 1;
 	} else if (!strcmp(arg, "--no-commit-id")) {
diff --git a/revision.h b/revision.h
index 475f048fb6..0f962e4e17 100644
--- a/revision.h
+++ b/revision.h
@@ -203,7 +203,8 @@ struct rev_info {
 			use_terminator:1,
 			missing_newline:1,
 			date_mode_explicit:1,
-			preserve_subject:1;
+			preserve_subject:1,
+			encode_email_headers:1;
 	unsigned int	disable_stdin:1;
 	/* --show-linear-break */
 	unsigned int	track_linear:1,
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index b653dd7d44..db7e733af9 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1160,6 +1160,59 @@ test_expect_success 'format-patch wraps extremely long from-header (rfc2047)' '
 	check_author "Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar"
 '
 
+cat >expect <<'EOF'
+From: Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
+ Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo
+ Bar Foo Bar Foo Bar Foo Bar <author@example.com>
+EOF
+test_expect_success 'format-patch wraps extremely long from-header (non-ASCII without Q-encoding)' '
+	echo content >>file &&
+	git add file &&
+	GIT_AUTHOR_NAME="Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar" \
+	git commit -m author-check &&
+	git format-patch --no-encode-email-headers --stdout -1 >patch &&
+	sed -n "/^From: /p; /^ /p; /^$/q" patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [PATCH] Foö
+EOF
+test_expect_success 'subject lines are unencoded with --no-encode-email-headers' '
+	echo content >>file &&
+	git add file &&
+	git commit -m "Foö" &&
+	git format-patch --no-encode-email-headers -1 --stdout >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [PATCH] Foö
+EOF
+test_expect_success 'subject lines are unencoded with format.encodeEmailHeaders=false' '
+	echo content >>file &&
+	git add file &&
+	git commit -m "Foö" &&
+	git config format.encodeEmailHeaders false &&
+	git format-patch -1 --stdout >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [PATCH] =?UTF-8?q?Fo=C3=B6?=
+EOF
+test_expect_success '--encode-email-headers overrides format.encodeEmailHeaders' '
+	echo content >>file &&
+	git add file &&
+	git commit -m "Foö" &&
+	git config format.encodeEmailHeaders false &&
+	git format-patch --encode-email-headers -1 --stdout >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
 cat >expect <<'EOF'
 Subject: header with . in it
 EOF