[PATCH] Add MIME information to outgoing email

[PATCH] Add MIME information to outgoing email

[Samuel Tardieu]

Add MIME-Version/Content-Type/Content-Transfer-Encoding headers in
messages generated with git-format-patch. Without it, messages generated
without using --attach or --inline didn't have any content type information.

I got hit with this problem yesterday when sending a patch to linux-kernel
with a commit message containing the name "Pádraig" in it. Moreover,
the mailing-list software added an incorrect ISO-8859-1 encoding information
which mangled Pádraig's name.

Signed-off-by: Samuel Tardieu <sam@rfc1149.net>
---
 log-tree.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 608f697..0dacf63 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -198,7 +198,16 @@ void log_write_email_headers(struct rev_info *opt, const char *name,
 			 opt->no_inline ? "attachment" : "inline",
 			 name);
 		opt->diffopt.stat_sep = buffer;
-	}
+	} else {
+		static char buffer[1024];
+		snprintf(buffer, sizeof(buffer) - 1,
+			 "%s"
+			 "MIME-Version: 1.0\n"
+			 "Content-Type: text/plain; charset=UTF-8; format=fixed\n"
+			 "Content-Transfer-Encoding: 8bit\n",
+			 extra_headers ? extra_headers : "");
+		extra_headers = buffer;
+	};
 	*subject_p = subject;
 	*extra_headers_p = extra_headers;
 }
-- 
1.5.4.4.653.g7cf1e.dirty

Re: [PATCH] Add MIME information to outgoing email

[Jeff King]

On Thu, Mar 13, 2008 at 05:40:19PM +0100, Samuel Tardieu wrote:

> Add MIME-Version/Content-Type/Content-Transfer-Encoding headers in
> messages generated with git-format-patch. Without it, messages generated
> without using --attach or --inline didn't have any content type information.
> 
> I got hit with this problem yesterday when sending a patch to linux-kernel
> with a commit message containing the name "Pádraig" in it. Moreover,
> the mailing-list software added an incorrect ISO-8859-1 encoding information
> which mangled Pádraig's name.

It's supposed to handle this automatically if the commit message
contains non-ascii characters. What version of git were you using?

-Peff

Re: [PATCH] Add MIME information to outgoing email

[Samuel Tardieu]

On 13/03, Jeff King wrote:

| It's supposed to handle this automatically if the commit message
| contains non-ascii characters. What version of git were you using?

A version from "next" from March 8:
ea6cde37d54121c5a1e1da51e1cd5cf27bfa3904 (+ 1 unrelated patch)

E.g, does "git format-patch a1eebf~1..a1eebf" add MIME headers for you
without my patch?

Re: [PATCH] Add MIME information to outgoing email

[Jeff King]

On Thu, Mar 13, 2008 at 06:14:36PM +0100, Samuel Tardieu wrote:

> | It's supposed to handle this automatically if the commit message
> | contains non-ascii characters. What version of git were you using?
> 
> A version from "next" from March 8:
> ea6cde37d54121c5a1e1da51e1cd5cf27bfa3904 (+ 1 unrelated patch)

Hmm, that is certainly recent enough.

> E.g, does "git format-patch a1eebf~1..a1eebf" add MIME headers for you
> without my patch?

Sorry, I don't have that commit. What repo is it in?

It does work for me with this simple test:

  mkdir repo && cd repo && git init
  echo content >file && git add file && git commit -m one
  echo more >>file && git commit -a -m 'two

  utf8 body: ñ'
  git format-patch HEAD^
  cat 0001-two.patch

I get:

-- >8 --
From a9e3222c0dca0a2b1e1a53ab9b7a7526ed359b79 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Fri, 14 Mar 2008 09:27:30 -0400
Subject: [PATCH] two
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

utf8 body: ñ
---
 file |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/file b/file
index d95f3ad..94b334d 100644
--- a/file
+++ b/file
@@ -1 +1,2 @@
 content
+more
-- 
1.5.4.4.553.g83e84.dirty
-- 8< --

So I assume there is some bug in git that is being triggered by the
commit you mention.

-Peff

Re: [PATCH] Add MIME information to outgoing email

[Samuel Tardieu]

On 14/03, Jeff King wrote:

| Sorry, I don't have that commit. What repo is it in?

Junio's "next" branch.

| So I assume there is some bug in git that is being triggered by the
| commit you mention.

Maybe, that's why I'd be interested by the behaviour you get with it.

Re: [PATCH] Add MIME information to outgoing email

[Jeff King]

On Fri, Mar 14, 2008 at 02:40:22PM +0100, Samuel Tardieu wrote:

> | Sorry, I don't have that commit. What repo is it in?
> 
> Junio's "next" branch.

Ah, sorry, for some reason I couldn't find it before. I must have
typo'd it.

Yes, it works fine for me:

$ git-format-patch a1eebf~1..a1eebf
0001-git.el-find-the-git-status-buffer-whatever-its-name.patch
$ head 0001*
From a1eebfb3a90b6c240afd1a32cfebe6ee5dbd72c5 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?R=C3=A9mi=20Vanicat?= <vanicat@debian.org>
Date: Fri, 29 Feb 2008 19:28:19 +0100
Subject: [PATCH] git.el: find the git-status buffer whatever its name is
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

git-status used the buffer name to find git-status buffers, and that
can fail if the buffer has another name, for example when multiple


-Peff

Re: [PATCH] Add MIME information to outgoing email

[Samuel Tardieu]

On 14/03, Jeff King wrote:

| Yes, it works fine for me:

Ahhhhh, found it. It looks like you have no format.headers configuration
variable, do you?

Re: [PATCH] Add MIME information to outgoing email

[Jeff King]

On Fri, Mar 14, 2008 at 02:50:48PM +0100, Samuel Tardieu wrote:

> | Yes, it works fine for me:
> 
> Ahhhhh, found it. It looks like you have no format.headers configuration
> variable, do you?

No, I don't. Having peeked a few days ago at the pretty-printing code,
that is almost certainly the problem (I think the extra_headers
parameter is overloaded to handle both of these conditions). Can you
work up a patch?

-Peff

Re: [PATCH] Add MIME information to outgoing email

[Samuel Tardieu]

On 14/03, Jeff King wrote:

| No, I don't. Having peeked a few days ago at the pretty-printing code,
| that is almost certainly the problem (I think the extra_headers
| parameter is overloaded to handle both of these conditions). Can you
| work up a patch?

Sure, but not before some time, I am quite busy with urgent work right
now. If you want to beat me at it, be my guest :)

Re: [PATCH] Add MIME information to outgoing email

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Mar 13, 2008 at 05:40:19PM +0100, Samuel Tardieu wrote:
>
>> Add MIME-Version/Content-Type/Content-Transfer-Encoding headers in
>> messages generated with git-format-patch. Without it, messages generated
>> without using --attach or --inline didn't have any content type information.
>> 
>> I got hit with this problem yesterday when sending a patch to linux-kernel
>> with a commit message containing the name "Pádraig" in it. Moreover,
>> the mailing-list software added an incorrect ISO-8859-1 encoding information
>> which mangled Pádraig's name.
>
> It's supposed to handle this automatically if the commit message
> contains non-ascii characters. What version of git were you using?

You are right.  The call-chain looks like this:

    log_tree_diff_flush()
     show_log()
      log_write_email_headers()     writes mbox From
      pretty_print_commit()
                                    check commit log if it is pure ascii
       pp_header()
        pp_user_info()              writes RFC2822 From:
       pp_title_line()              writes RFC2822 Subject:
                                    writes MIME-Version: and friends if needed
       pp_remainder()               writes the remainder of the log message
      append_signoff()
     printf("---\n")
     diff_flush()                   writes the patch

At the beginning of pretty_print_commit() we look at the log and if it is
not ascii we pass that information down to pp_title_line() which is
responsible for writing MIME header at the appropriate place.

If your patch itself has some non-ASCII material, and if your commit log
message is pure ASCII, the above would end up not writing MIME at all.  If
your commit log message is non ASCII, then we will mark it as if the
entire message is in the encoding of the log in pp_title_line().  This
might look like a problem, but it is not something non multipart output of
format-patch should even try to cater to.  The payload (i.e. the patch)
out of git has always been uninterpreted sequence of bytes (and it is not
going to change).

A patch to i18n po/ files for example could contain patches to different
files encoded in KOI-8, BIG5, EUC-JP and UTF-8 at the same time.  There is
no way to say "text/plain; charset=X" for such a payload (because there is
no single charset used in such a patch), and git simply does not know nor
care about what encoding each file is in.  The output from git marks only
the part git knows the encoding about (i.e. the commit log message).

Having said all that, I notice that addition of format.headers variable
(which I think is a later invention) was done not quite correctly.  In the
callchain above, pretty_print_commit() function checks the commit log but
it is meant to do so only when we haven't emitted MIME Content-Type:
(because the user told us to do multipart), and "after_subject" parameter
was getting passed around for it (and its callees) to detect exactly that.
But format.headers misused that variable to carry its contents along ---
there needs a way to pass "have we said MIME-Version crap already"
separately.

Re* [PATCH] Add MIME information to outgoing email

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Having said all that, I notice that addition of format.headers variable
> (which I think is a later invention) was done not quite correctly.  In the
> callchain above, pretty_print_commit() function checks the commit log but
> it is meant to do so only when we haven't emitted MIME Content-Type:
> (because the user told us to do multipart), and "after_subject" parameter
> was getting passed around for it (and its callees) to detect exactly that.
> But format.headers misused that variable to carry its contents along ---
> there needs a way to pass "have we said MIME-Version crap already"
> separately.

I think the real culprit was the way the "after_subject" was added to the
callchain (it had loaded semantics -- "here is what we want to say after
emitting Subject: line" and "have we done any MIME yet?"), not the poor
guy who did format.headers.

In any case, this patch would hopefully separate the two.  The old
"plain_non_ascii" parameter is now need_8bit_ct_header and now can have
one of three values:

 -1 : we've already done MIME crap so never add extra header to say this
      is 8bit;

 0  : we haven't done MIME and we have not seen anything that is 8bit yet.

 1  : we haven't done MIME and we have seen something that is 8bit.
      pp_title_line() needs to add MIME header.

(As with any other patches I send during my lunchtime, this is untested).

---

 builtin-log.c |    6 ++++--
 commit.h      |    4 ++--
 log-tree.c    |   15 ++++++++++++---
 log-tree.h    |    4 +++-
 pretty.c      |   24 +++++++++++-------------
 5 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index d983cbc..4b1b34f 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -662,6 +662,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	int i;
 	const char *encoding = "utf-8";
 	struct diff_options opts;
+	int need_8bit_ct_header = 0;
 
 	if (rev->commit_format != CMIT_FMT_EMAIL)
 		die("Cover letter needs email format");
@@ -672,7 +673,8 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 
 	head_sha1 = sha1_to_hex(head->object.sha1);
 
-	log_write_email_headers(rev, head_sha1, &subject_start, &extra_headers);
+	log_write_email_headers(rev, head_sha1, &subject_start, &extra_headers,
+				&need_8bit_ct_header);
 
 	committer = git_committer_info(0);
 
@@ -681,7 +683,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, 0);
+		      encoding, need_8bit_ct_header);
 	pp_remainder(CMIT_FMT_EMAIL, &msg, &sb, 0);
 	printf("%s\n", sb.buf);
 
diff --git a/commit.h b/commit.h
index a1e9591..0907a78 100644
--- a/commit.h
+++ b/commit.h
@@ -70,7 +70,7 @@ extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit*,
                                 struct strbuf *,
                                 int abbrev, const char *subject,
                                 const char *after_subject, enum date_mode,
-				int non_ascii_present);
+				int need_8bit_ct_header);
 void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 		   const char *line, enum date_mode dmode,
 		   const char *encoding);
@@ -80,7 +80,7 @@ void pp_title_line(enum cmit_fmt fmt,
 		   const char *subject,
 		   const char *after_subject,
 		   const char *encoding,
-		   int plain_non_ascii);
+		   int need_8bit_ct_header);
 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 608f697..f2ce32a 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -138,10 +138,14 @@ static int has_non_ascii(const char *s)
 }
 
 void log_write_email_headers(struct rev_info *opt, const char *name,
-			     const char **subject_p, const char **extra_headers_p)
+			     const char **subject_p,
+			     const char **extra_headers_p,
+			     int *need_8bit_ct_header_p)
 {
 	const char *subject = NULL;
 	const char *extra_headers = opt->extra_headers;
+
+	*need_8bit_ct_header_p = 0; /* unknown */
 	if (opt->total > 0) {
 		static char buffer[64];
 		snprintf(buffer, sizeof(buffer),
@@ -169,6 +173,7 @@ void log_write_email_headers(struct rev_info *opt, const char *name,
 	if (opt->mime_boundary) {
 		static char subject_buffer[1024];
 		static char buffer[1024];
+		*need_8bit_ct_header_p = -1; /* NEVER */
 		snprintf(subject_buffer, sizeof(subject_buffer) - 1,
 			 "%s"
 			 "MIME-Version: 1.0\n"
@@ -212,6 +217,7 @@ void show_log(struct rev_info *opt, const char *sep)
 	int abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;
 	const char *extra;
 	const char *subject = NULL, *extra_headers = opt->extra_headers;
+	int need_8bit_ct_header = 0;
 
 	opt->loginfo = NULL;
 	if (!opt->verbose_header) {
@@ -255,7 +261,8 @@ void show_log(struct rev_info *opt, const char *sep)
 
 	if (opt->commit_format == CMIT_FMT_EMAIL) {
 		log_write_email_headers(opt, sha1_to_hex(commit->object.sha1),
-					&subject, &extra_headers);
+					&subject, &extra_headers,
+					&need_8bit_ct_header);
 	} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {
 		fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), stdout);
 		if (opt->commit_format != CMIT_FMT_ONELINE)
@@ -299,9 +306,11 @@ void show_log(struct rev_info *opt, const char *sep)
 	 * And then the pretty-printed message itself
 	 */
 	strbuf_init(&msgbuf, 0);
+	if (need_8bit_ct_header >= 0)
+		need_8bit_ct_header = has_non_ascii(opt->add_signoff);
 	pretty_print_commit(opt->commit_format, commit, &msgbuf,
 			    abbrev, subject, extra_headers, opt->date_mode,
-			    has_non_ascii(opt->add_signoff));
+			    need_8bit_ct_header);
 
 	if (opt->add_signoff)
 		append_signoff(&msgbuf, opt->add_signoff);
diff --git a/log-tree.h b/log-tree.h
index 0cc9344..69c1c4b 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -14,6 +14,8 @@ int log_tree_opt_parse(struct rev_info *, const char **, int);
 void show_log(struct rev_info *opt, const char *sep);
 void show_decorations(struct commit *commit);
 void log_write_email_headers(struct rev_info *opt, const char *name,
-			     const char **subject_p, const char **extra_headers_p);
+			     const char **subject_p,
+			     const char **extra_headers_p,
+			     int *need_8bit_ct_header_p);
 
 #endif
diff --git a/pretty.c b/pretty.c
index 703f521..7a5b115 100644
--- a/pretty.c
+++ b/pretty.c
@@ -636,7 +636,7 @@ void pp_title_line(enum cmit_fmt fmt,
 		   const char *subject,
 		   const char *after_subject,
 		   const char *encoding,
-		   int plain_non_ascii)
+		   int need_8bit_ct_header)
 {
 	struct strbuf title;
 
@@ -669,7 +669,7 @@ void pp_title_line(enum cmit_fmt fmt,
 	}
 	strbuf_addch(sb, '\n');
 
-	if (plain_non_ascii) {
+	if (need_8bit_ct_header > 0) {
 		const char *header_fmt =
 			"MIME-Version: 1.0\n"
 			"Content-Type: text/plain; charset=%s\n"
@@ -718,9 +718,9 @@ void pp_remainder(enum cmit_fmt fmt,
 }
 
 void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
-				  struct strbuf *sb, int abbrev,
-				  const char *subject, const char *after_subject,
-				  enum date_mode dmode, int plain_non_ascii)
+			 struct strbuf *sb, int abbrev,
+			 const char *subject, const char *after_subject,
+			 enum date_mode dmode, int need_8bit_ct_header)
 {
 	unsigned long beginning_of_body;
 	int indent = 4;
@@ -746,13 +746,11 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
 		indent = 0;
 
-	/* After-subject is used to pass in Content-Type: multipart
-	 * MIME header; in that case we do not have to do the
-	 * plaintext content type even if the commit message has
-	 * non 7-bit ASCII character.  Otherwise, check if we need
-	 * to say this is not a 7-bit ASCII.
+	/*
+	 * 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 && !after_subject) {
+	if (fmt == CMIT_FMT_EMAIL && need_8bit_ct_header == 0) {
 		int i, ch, in_body;
 
 		for (in_body = i = 0; (ch = msg[i]); i++) {
@@ -765,7 +763,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 					in_body = 1;
 			}
 			else if (non_ascii(ch)) {
-				plain_non_ascii = 1;
+				need_8bit_ct_header = 1;
 				break;
 			}
 		}
@@ -790,7 +788,7 @@ 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, subject,
-			      after_subject, encoding, plain_non_ascii);
+			      after_subject, encoding, need_8bit_ct_header);
 
 	beginning_of_body = sb->len;
 	if (fmt != CMIT_FMT_ONELINE)

Re: Re* [PATCH] Add MIME information to outgoing email

[Jeff King]

On Fri, Mar 14, 2008 at 01:21:27PM -0700, Junio C Hamano wrote:

> I think the real culprit was the way the "after_subject" was added to the
> callchain (it had loaded semantics -- "here is what we want to say after
> emitting Subject: line" and "have we done any MIME yet?"), not the poor
> guy who did format.headers.
> 
> In any case, this patch would hopefully separate the two.  The old
> "plain_non_ascii" parameter is now need_8bit_ct_header and now can have
> one of three values:

I was just about to submit a patch splitting after_subject into
"mime_headers" and "extra_headers".

I noticed another bug while doing mine: we are sometimes not strict
_enough_ in squelching headers. A patch made with "-s --attach" when the
signoff has non-ascii characters would end up with duplicated MIME
headers. Your patch handles this fine.

I think your approach is a little nicer. Here is the test case I wrote
for my patch. It covers the original problem and the one I mentioned
above; both fail with current master but pass with your patch.

---
diff --git a/t/t4021-format-patch-signer-mime.sh b/t/t4021-format-patch-signer-mime.sh
index 67a70fa..9bc47a5 100755
--- a/t/t4021-format-patch-signer-mime.sh
+++ b/t/t4021-format-patch-signer-mime.sh
@@ -38,5 +38,13 @@ test_expect_success 'format with non ASCII signer name' '
 
 '
 
+test_expect_success 'attach and signoff do not duplicate mime headers' '
+
+	GIT_COMMITTER_NAME="^[$B$O$^$N^[(B ^[$B$U$K$*$&^[(B" \
+	git format-patch -s --stdout -1 --attach >output &&
+	test `grep -ci ^MIME-Version: output` = 1
+
+'
+
 test_done
 
diff --git a/t/t4028-format-patch-mime-headers.sh b/t/t4028-format-patch-mime-headers.sh
new file mode 100755
index 0000000..204ba67
--- /dev/null
+++ b/t/t4028-format-patch-mime-headers.sh
@@ -0,0 +1,30 @@
+#!/bin/sh
+
+test_description='format-patch mime headers and extra headers do not conflict'
+. ./test-lib.sh
+
+test_expect_success 'create commit with utf-8 body' '
+	echo content >file &&
+	git add file &&
+	git commit -m one &&
+	echo more >>file &&
+	git commit -a -m "two
+
+	utf-8 body: ñ"
+'
+
+test_expect_success 'patch has mime headers' '
+	rm -f 0001-two.patch &&
+	git format-patch HEAD^ &&
+	grep -i "content-type: text/plain; charset=utf-8" 0001-two.patch
+'
+
+test_expect_success 'patch has mime and extra headers' '
+	rm -f 0001-two.patch &&
+	git config format.headers "x-foo: bar" &&
+	git format-patch HEAD^ &&
+	grep -i "x-foo: bar" 0001-two.patch &&
+	grep -i "content-type: text/plain; charset=utf-8" 0001-two.patch
+'
+
+test_done

Re: [PATCH] Add MIME information to outgoing email

[Junio C Hamano]

Samuel Tardieu <sam@rfc1149.net> writes:

> Add MIME-Version/Content-Type/Content-Transfer-Encoding headers in
> messages generated with git-format-patch. Without it, messages generated
> without using --attach or --inline didn't have any content type information.

Isn't that job for send-email (or user's MUA)?  I really do not think we
want to clutter format-patch output any more than necessary.

Re: [PATCH] Add MIME information to outgoing email

[Samuel Tardieu]

On 13/03, Junio C Hamano wrote:

| Samuel Tardieu <sam@rfc1149.net> writes:
| 
| > Add MIME-Version/Content-Type/Content-Transfer-Encoding headers in
| > messages generated with git-format-patch. Without it, messages generated
| > without using --attach or --inline didn't have any content type information.
| 
| Isn't that job for send-email (or user's MUA)?  I really do not think we
| want to clutter format-patch output any more than necessary.

Only format-patch knows what encoding has been used by itself to
generate the message. Doing it at any later stage would have to guess
what the correct charset is.

Re: [PATCH] Add MIME information to outgoing email

[Brian Swetland]

[Samuel Tardieu <sam@rfc1149.net>]
> On 13/03, Junio C Hamano wrote:
> 
> | Samuel Tardieu <sam@rfc1149.net> writes:
> | 
> | > Add MIME-Version/Content-Type/Content-Transfer-Encoding headers in
> | > messages generated with git-format-patch. Without it, messages generated
> | > without using --attach or --inline didn't have any content type information.
> | 
> | Isn't that job for send-email (or user's MUA)?  I really do not think we
> | want to clutter format-patch output any more than necessary.
> 
> Only format-patch knows what encoding has been used by itself to
> generate the message. Doing it at any later stage would have to guess
> what the correct charset is.

When the encoded string is entirely ascii except for one or two characters
(such as occurs in a lot of patches I handle from people with names
not represented in plain ascii) guessing later on seems to run pretty
high risk of guessing wrong.

I've taken to manually adding UTF-8 content-type/transfer-encoding
headers to avoid the routine mangling of my coworkers' names and
would welcome a change to do this automatically.

Considering that UTF-8 is the expected default encoding (right?) for
git metadata, it seems to be the sane thing to indicate if the default
is unchanged.

Brian

> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add MIME information to outgoing email

[Samuel Tardieu]

On 14/03, Brian Swetland wrote:

| > Only format-patch knows what encoding has been used by itself to
| > generate the message. Doing it at any later stage would have to guess
| > what the correct charset is.
| 
| When the encoded string is entirely ascii except for one or two characters
| (such as occurs in a lot of patches I handle from people with names
| not represented in plain ascii) guessing later on seems to run pretty
| high risk of guessing wrong.
| [...]
| Considering that UTF-8 is the expected default encoding (right?) for
| git metadata, it seems to be the sane thing to indicate if the default
| is unchanged.

Yup.

MIME headers in introductory message (git send-email --compose)

[Teemu Likonen]

Junio C Hamano kirjoitti:

> Samuel Tardieu <sam@rfc1149.net> writes:
> > Add MIME-Version/Content-Type/Content-Transfer-Encoding headers in
> > messages generated with git-format-patch. Without it, messages
> > generated without using --attach or --inline didn't have any
> > content type information.
>
> Isn't that job for send-email (or user's MUA)?  I really do not think
> we want to clutter format-patch output any more than necessary.

By the way, 'git send-email --compose' does not add MIME headers to 
introductory message. All non-Ascii chars will output something 
undefined in receivers' end.

I guess the right way would be to detect user's charset (locale) and add 
appropriate MIME headers. Also, the Subject field should be encoded if 
it contains non-Ascii characters.

Re: MIME headers in introductory message (git send-email --compose)

[Jay Soffian]

On Tue, Mar 25, 2008 at 2:31 PM, Teemu Likonen <tlikonen@iki.fi> wrote:
> Junio C Hamano kirjoitti:
>
>  > Samuel Tardieu <sam@rfc1149.net> writes:
>  > > Add MIME-Version/Content-Type/Content-Transfer-Encoding headers in
>  > > messages generated with git-format-patch. Without it, messages
>  > > generated without using --attach or --inline didn't have any
>  > > content type information.
>  >
>  > Isn't that job for send-email (or user's MUA)?  I really do not think
>  > we want to clutter format-patch output any more than necessary.
>
>  By the way, 'git send-email --compose' does not add MIME headers to
>  introductory message. All non-Ascii chars will output something
>  undefined in receivers' end.
>
>  I guess the right way would be to detect user's charset (locale) and add
>  appropriate MIME headers. Also, the Subject field should be encoded if
>  it contains non-Ascii characters.

I stuck this in my config and it works-for-me:

[format]
	headers = \
"MIME-Version: 1.0\n\
Content-Type: text/plain; charset=UTF-8\n\
Content-Transfer-Encoding: 8bit\n"

Shrug. (Never tried putting non-ascii in the subject tho.)

j.

Re: MIME headers in introductory message (git send-email --compose)

[Junio C Hamano]

"Jay Soffian" <jaysoffian@gmail.com> writes:

> I stuck this in my config and it works-for-me:
>
> [format]
> 	headers = \
> "MIME-Version: 1.0\n\
> Content-Type: text/plain; charset=UTF-8\n\
> Content-Transfer-Encoding: 8bit\n"

I suspect that you shouldn't do this.  This would badly interfere both
with existing format-patch behaviour that adds these MIME-Version and
Content-Type headers by looking at the contents, and with recent
format-patch fix 6bf4f1b (format-patch: generate MIME header as needed
even when there is format.header, 2008-03-14) to make the detection based
on contents (and presense of format.headers).

Re: MIME headers in introductory message (git send-email --compose)

[Jay Soffian]

On Tue, Mar 25, 2008 at 4:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "Jay Soffian" <jaysoffian@gmail.com> writes:
>
>  > I stuck this in my config and it works-for-me:
>  >
>  > [format]
>  >       headers = \
>  > "MIME-Version: 1.0\n\
>  > Content-Type: text/plain; charset=UTF-8\n\
>  > Content-Transfer-Encoding: 8bit\n"
>
>  I suspect that you shouldn't do this.  This would badly interfere both
>  with existing format-patch behaviour that adds these MIME-Version and
>  Content-Type headers by looking at the contents, and with recent
>  format-patch fix 6bf4f1b (format-patch: generate MIME header as needed
>  even when there is format.header, 2008-03-14) to make the detection based
>  on contents (and presense of format.headers).

Fair enough. But I never send out a patch w/o looking at it in an editor
first so I would've caught that. Thanks for the heads-up though.

j.

Re: MIME headers in introductory message (git send-email --compose)

[Jeff King]

On Tue, Mar 25, 2008 at 01:47:05PM -0700, Junio C Hamano wrote:

> > I stuck this in my config and it works-for-me:
> >
> > [format]
> > 	headers = \
> > "MIME-Version: 1.0\n\
> > Content-Type: text/plain; charset=UTF-8\n\
> > Content-Transfer-Encoding: 8bit\n"
> 
> I suspect that you shouldn't do this.  This would badly interfere both
> with existing format-patch behaviour that adds these MIME-Version and
> Content-Type headers by looking at the contents, and with recent
> format-patch fix 6bf4f1b (format-patch: generate MIME header as needed
> even when there is format.header, 2008-03-14) to make the detection based
> on contents (and presense of format.headers).

Yes, I can confirm that that is problematic without even testing.  The
whole point of 6bf4f1b was that we _should_ add MIME headers even if the
user has set format.headers.

We could be more clever about parsing format.headers and mark the "we
have already added MIME" flag (I think we already have to do such
parsing because of to/cc magic). But I have to wonder what the real goal
is here. There has sometimes been a call for "please add MIME headers
unconditionally"; maybe that is an option that people would like.

-Peff

Re: MIME headers in introductory message (git send-email --compose)

[Jeff King]

On Tue, Mar 25, 2008 at 05:56:14PM -0400, Jeff King wrote:

> We could be more clever about parsing format.headers and mark the "we
> have already added MIME" flag (I think we already have to do such
> parsing because of to/cc magic). But I have to wonder what the real goal

I started on this out of curiosity, and it _is_ really simple, but it's
also wrong.  It can't be right to set your MIME headers statically
because some options (like --attach) might cause us to have to put in
_different_ MIME headers. So we are left with either conflicting
headers, disallowing --attach, ignoring some of your format.headers, or
possibly picking out those headers and making them part of the header of
that patch part of the multipart. All of which seem a bit ugly to me.

If this is something people really want, I think just adding an "always
add mime headers" option makes the most sense.

-Peff

Re: MIME headers in introductory message (git send-email --compose)

[Jeff King]

On Tue, Mar 25, 2008 at 08:31:16PM +0200, Teemu Likonen wrote:

> By the way, 'git send-email --compose' does not add MIME headers to 
> introductory message. All non-Ascii chars will output something 
> undefined in receivers' end.
> 
> I guess the right way would be to detect user's charset (locale) and add 
> appropriate MIME headers. Also, the Subject field should be encoded if 
> it contains non-Ascii characters.

I just posted some patches to fix this; however, they always encode as
utf-8. I'm not sure what is the best way to find the user's encoding.
AIUI, locale environment variables are not enough, since, e.g., "en_US"
could come in iso8859-1 and utf-8 flavors. Is there a portable way to
figure this out? Should we be pulling it from .git/config? Neither the
commitEncoding nor the logOutputEncoding really makes sense to reuse.

-Peff

Re: MIME headers in introductory message (git send-email --compose)

[Jay Soffian]

On Tue, Mar 25, 2008 at 7:06 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Mar 25, 2008 at 08:31:16PM +0200, Teemu Likonen wrote:
>
>  > By the way, 'git send-email --compose' does not add MIME headers to
>  > introductory message. All non-Ascii chars will output something
>  > undefined in receivers' end.
>  >
>  > I guess the right way would be to detect user's charset (locale) and add
>  > appropriate MIME headers. Also, the Subject field should be encoded if
>  > it contains non-Ascii characters.
>
>  I just posted some patches to fix this; however, they always encode as
>  utf-8. I'm not sure what is the best way to find the user's encoding.
>  AIUI, locale environment variables are not enough, since, e.g., "en_US"
>  could come in iso8859-1 and utf-8 flavors. Is there a portable way to
>  figure this out? Should we be pulling it from .git/config?

I think so. There's no reason the message encoding necessarily matches the
locale anyway. There are ways to guess, but I think .git/config is sanest
with UTF-8 as the default.

j.

Re: MIME headers in introductory message (git send-email --compose)

[Jan Hudec]

On Tue, Mar 25, 2008 at 19:06:49 -0400, Jeff King wrote:
> On Tue, Mar 25, 2008 at 08:31:16PM +0200, Teemu Likonen wrote:
> 
> > By the way, 'git send-email --compose' does not add MIME headers to 
> > introductory message. All non-Ascii chars will output something 
> > undefined in receivers' end.
> > 
> > I guess the right way would be to detect user's charset (locale) and add 
> > appropriate MIME headers. Also, the Subject field should be encoded if 
> > it contains non-Ascii characters.
> 
> I just posted some patches to fix this; however, they always encode as
> utf-8. I'm not sure what is the best way to find the user's encoding.
> AIUI, locale environment variables are not enough, since, e.g., "en_US"
> could come in iso8859-1 and utf-8 flavors. Is there a portable way to
> figure this out? Should we be pulling it from .git/config? Neither the
> commitEncoding nor the logOutputEncoding really makes sense to reuse.

The portable way is to use the locale environment variables, but you have to
read them via the libc locale interface. You set the LC_CTYPE locale category
via setlocale (which will consult LC_CTYPE, LC_ALL and LANG environment AND
the locale database) and than ask for charset using nl_langinfo(CODESET).
To do the equivalent from the shell, call `locale charmap`. From perl, either
use langinfo in I18N::Langinfo, or just use the ':locale' IO stream option
provided by encoding pragma to read the file in unicode no matter what the
locale encoding was (this is perl 5.8.<something> -- in older one, you'd have
to use I18N::Langinfo anyway).

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>