* [BUG] format-patch does not wrap From-field after author name @ 2011-04-14 17:01 Erik Faye-Lund 2011-04-14 17:12 ` Junio C Hamano 2011-04-14 17:52 ` Jeff King 0 siblings, 2 replies; 15+ messages in thread From: Erik Faye-Lund @ 2011-04-14 17:01 UTC (permalink / raw) To: Git Mailing List With certain combinations of author name and email lengths, git format-patch does not correctly wrap the From line to be below 78 characters, violating rfc2047. This happens because pp_user_info in pretty.c use add_rfc2047 (which wraps) for the name, but strbuf_add (which does not wrap) for the email part. Here's a test that illustrate the problem: diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 9c66367..a4b8b59 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -793,4 +793,19 @@ test_expect_success 'format-patch wraps extremely long headers (rfc2047)' ' test_cmp expect subject ' +M8="foo_bar_" +M64=$M8$M8$M8$M8$M8$M8$M8$M8 +cat >expect <<'EOF' +From: foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar + <foobar@foo.bar> +EOF +test_expect_success 'format-patch wraps non-quotable headers' ' + rm -rf patches/ && + echo content >>file && + git add file && + git commit -mfoo --author "$M64 <foobar@foo.bar>" && + git format-patch --stdout -1 >patch && + sed -n "/^From: /p; /^ /p; /^$/q" <patch >from && + test_cmp expect from +' test_done ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 17:01 [BUG] format-patch does not wrap From-field after author name Erik Faye-Lund @ 2011-04-14 17:12 ` Junio C Hamano 2011-04-14 17:50 ` Jeff King 2011-04-14 17:52 ` Jeff King 1 sibling, 1 reply; 15+ messages in thread From: Junio C Hamano @ 2011-04-14 17:12 UTC (permalink / raw) To: Erik Faye-Lund; +Cc: Git Mailing List, Jeff King Erik Faye-Lund <kusmabite@gmail.com> writes: > With certain combinations of author name and email lengths, git > format-patch does not correctly wrap the From line to be below 78 > characters, violating rfc2047. Didn't we have this fixed quite a while ago with the series that ends at c22e7de (format-patch: rfc2047-encode newlines in headers, 2011-02-23)? What version of git do you use? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 17:12 ` Junio C Hamano @ 2011-04-14 17:50 ` Jeff King 2011-04-14 21:19 ` Erik Faye-Lund 0 siblings, 1 reply; 15+ messages in thread From: Jeff King @ 2011-04-14 17:50 UTC (permalink / raw) To: Junio C Hamano; +Cc: Erik Faye-Lund, Git Mailing List On Thu, Apr 14, 2011 at 10:12:44AM -0700, Junio C Hamano wrote: > Erik Faye-Lund <kusmabite@gmail.com> writes: > > > With certain combinations of author name and email lengths, git > > format-patch does not correctly wrap the From line to be below 78 > > characters, violating rfc2047. > > Didn't we have this fixed quite a while ago with the series that ends at > c22e7de (format-patch: rfc2047-encode newlines in headers, 2011-02-23)? > > What version of git do you use? No, that doesn't fix it. The problem is a short name and a long email address. We rfc2047-encode and wrap the name, but then tack the email onto the end without caring about line length. According to rfc2047, we are specifically forbidden to use an encoded-word in an addr-spec. So it would not make sense to assemble the header first and then feed the whole thing to add_rfc2047. Fortunately, this is an easy thing to wrap, since we can't actually break the addr-spec across lines. We can just check if the line is long, and if so, wrap the whole address to the next line, which is the best we can do (and presumably nobody has an address portion over 78 characters). That patch looks something like: diff --git a/pretty.c b/pretty.c index e1d8a8f..63a7c2f 100644 --- a/pretty.c +++ b/pretty.c @@ -287,6 +287,7 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb, if (fmt == CMIT_FMT_EMAIL) { char *name_tail = strchr(line, '<'); int display_name_length; + int final_line; if (!name_tail) return; while (line < name_tail && isspace(name_tail[-1])) @@ -294,6 +295,11 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb, display_name_length = name_tail - line; strbuf_addstr(sb, "From: "); add_rfc2047(sb, line, display_name_length, encoding); + for (final_line = 0; final_line < sb->len; final_line++) + if (sb->buf[sb->len - final_line - 1] == '\n') + break; + if (namelen - display_name_length + final_line > 78) + strbuf_addch(sb, '\n'); strbuf_add(sb, name_tail, namelen - display_name_length); strbuf_addch(sb, '\n'); } else { Note that it relies on the commit header having a space before the "<", which forms the continuation whitespace for the header. This is probably reasonable, but we could double-check if we want to handle malformed commit headers better. Or we could just ignore it. AFAICS, this doesn't actually violate rfc2047, nor rfc5322. The 78-character limit is simply a SHOULD, and we have up to 998 for MUST. For a single-address header[1], this seems kind of unlikely to me. -Peff [1] For multi-address headers like "format-patch --cc=foo --cc=bar", it looks like we already break them across lines. No idea if "send-email" is sensible in this area or not. ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 17:50 ` Jeff King @ 2011-04-14 21:19 ` Erik Faye-Lund 2011-04-14 21:42 ` Jeff King 0 siblings, 1 reply; 15+ messages in thread From: Erik Faye-Lund @ 2011-04-14 21:19 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Git Mailing List On Thu, Apr 14, 2011 at 7:50 PM, Jeff King <peff@peff.net> wrote: > On Thu, Apr 14, 2011 at 10:12:44AM -0700, Junio C Hamano wrote: > >> Erik Faye-Lund <kusmabite@gmail.com> writes: >> >> > With certain combinations of author name and email lengths, git >> > format-patch does not correctly wrap the From line to be below 78 >> > characters, violating rfc2047. >> >> Didn't we have this fixed quite a while ago with the series that ends at >> c22e7de (format-patch: rfc2047-encode newlines in headers, 2011-02-23)? >> >> What version of git do you use? I believe it was straight from todays master, but I'm not still at that machine. > No, that doesn't fix it. The problem is a short name and a long email > address. We rfc2047-encode and wrap the name, but then tack the email > onto the end without caring about line length. > > According to rfc2047, we are specifically forbidden to use an > encoded-word in an addr-spec. So it would not make sense to assemble the > header first and then feed the whole thing to add_rfc2047. > > Fortunately, this is an easy thing to wrap, since we can't actually > break the addr-spec across lines. We can just check if the line is long, > and if so, wrap the whole address to the next line, which is the best we > can do (and presumably nobody has an address portion over 78 > characters). That patch looks something like: > > diff --git a/pretty.c b/pretty.c > index e1d8a8f..63a7c2f 100644 > --- a/pretty.c > +++ b/pretty.c > @@ -287,6 +287,7 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb, > if (fmt == CMIT_FMT_EMAIL) { > char *name_tail = strchr(line, '<'); > int display_name_length; > + int final_line; > if (!name_tail) > return; > while (line < name_tail && isspace(name_tail[-1])) > @@ -294,6 +295,11 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb, > display_name_length = name_tail - line; > strbuf_addstr(sb, "From: "); > add_rfc2047(sb, line, display_name_length, encoding); > + for (final_line = 0; final_line < sb->len; final_line++) > + if (sb->buf[sb->len - final_line - 1] == '\n') > + break; > + if (namelen - display_name_length + final_line > 78) > + strbuf_addch(sb, '\n'); > strbuf_add(sb, name_tail, namelen - display_name_length); > strbuf_addch(sb, '\n'); > } else { > Yes, this is the reasonable fix. Basically copying the wrapping logic from add_rfc2047... > Note that it relies on the commit header having a space before the "<", > which forms the continuation whitespace for the header. This is probably > reasonable, but we could double-check if we want to handle malformed > commit headers better. I think that's a reasonable assumption; this field comes straight from the commit. It should already be well-formed, no? > Or we could just ignore it. AFAICS, this doesn't actually violate > rfc2047, nor rfc5322. The 78-character limit is simply a SHOULD, and > we have up to 998 for MUST. For a single-address header[1], this seems > kind of unlikely to me. True. But since the fix is as simple as it is, perhaps it's worth it just for the clean conscience? Don't get me wrong, this was just something I stumbled over. I don't have any real-world case where it breaks. > [1] For multi-address headers like "format-patch --cc=foo --cc=bar", it > looks like we already break them across lines. Yes, but this is even worse: these fields don't get encoded at all! > No idea if "send-email" > is sensible in this area or not. ...but luckily send-email is sensible. The first thing it does is to unfold each header line. Addresses are rfc2047-decoded and recoded (if needed, utf-8 is assumed as the encoding if none is found), and To/Cc headers are laid out one address per line. This still produce invalid To/Cc headers if send-email isn't used, though. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 21:19 ` Erik Faye-Lund @ 2011-04-14 21:42 ` Jeff King 2011-04-14 22:18 ` Jeff King 2011-04-14 22:21 ` Erik Faye-Lund 0 siblings, 2 replies; 15+ messages in thread From: Jeff King @ 2011-04-14 21:42 UTC (permalink / raw) To: Erik Faye-Lund; +Cc: Junio C Hamano, Git Mailing List On Thu, Apr 14, 2011 at 11:19:09PM +0200, Erik Faye-Lund wrote: > > Note that it relies on the commit header having a space before the "<", > > which forms the continuation whitespace for the header. This is probably > > reasonable, but we could double-check if we want to handle malformed > > commit headers better. > > I think that's a reasonable assumption; this field comes straight from > the commit. It should already be well-formed, no? Probably; it was just something I noted while writing the patch, and I like to be paranoid about assumptions. :) > > Or we could just ignore it. AFAICS, this doesn't actually violate > > rfc2047, nor rfc5322. The 78-character limit is simply a SHOULD, and > > we have up to 998 for MUST. For a single-address header[1], this seems > > kind of unlikely to me. > > True. But since the fix is as simple as it is, perhaps it's worth it > just for the clean conscience? Fair enough. Patch to follow. > > [1] For multi-address headers like "format-patch --cc=foo --cc=bar", it > > looks like we already break them across lines. > > Yes, but this is even worse: these fields don't get encoded at all! Ugh, you're right. That is a totally separate issue, and one I really don't want to get into. Because it means we have to _parse_ those headers and understand which part is a name and which is an address. People who use "--cc" or format.headers will have to deal with that themselves. I consider both to be somewhat useless, since you can post-process the mbox after format-patch is run (or in your MUA). Whereas quoting and encoding fields in format-patch is necessary to give unambiguous input to the MUA (be it send-email or whatever). -Peff ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 21:42 ` Jeff King @ 2011-04-14 22:18 ` Jeff King 2011-04-14 22:21 ` Erik Faye-Lund 1 sibling, 0 replies; 15+ messages in thread From: Jeff King @ 2011-04-14 22:18 UTC (permalink / raw) To: Erik Faye-Lund; +Cc: Junio C Hamano, Git Mailing List On Thu, Apr 14, 2011 at 05:42:30PM -0400, Jeff King wrote: > > > Or we could just ignore it. AFAICS, this doesn't actually violate > > > rfc2047, nor rfc5322. The 78-character limit is simply a SHOULD, and > > > we have up to 998 for MUST. For a single-address header[1], this seems > > > kind of unlikely to me. > > > > True. But since the fix is as simple as it is, perhaps it's worth it > > just for the clean conscience? > > Fair enough. Patch to follow. Here it is. -- >8 -- Subject: [PATCH] format-patch: wrap email addresses after long names We already wrap names in "from" headers, which tend to be the long part of an address. But it's also possible for a long name to not be wrapped, but to make us want to wrap the email address. For example (imagine for the sake of readability we want to wrap at 50 characters instead of 78): From: this is my really long git name <foo@example.com> The name does not overflow the line, but the name and email together do. So we would rather see: From: this is my really long git name <git@example.com> Because we wrap the name separately during add_rfc2047, we neglected this case. Instead, we should see how long the final line of the wrapped name ended up, and decide whether or not to wrap based on that. We can't break the address into multiple parts, so we either leave it with the name, or put it by itself on a line. Test by Erik Faye-Lund. Signed-off-by: Jeff King <peff@peff.net> --- pretty.c | 9 +++++++++ t/t4014-format-patch.sh | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 0 deletions(-) diff --git a/pretty.c b/pretty.c index e1d8a8f..ba95de9 100644 --- a/pretty.c +++ b/pretty.c @@ -287,6 +287,7 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb, if (fmt == CMIT_FMT_EMAIL) { char *name_tail = strchr(line, '<'); int display_name_length; + int final_line; if (!name_tail) return; while (line < name_tail && isspace(name_tail[-1])) @@ -294,6 +295,14 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb, display_name_length = name_tail - line; strbuf_addstr(sb, "From: "); add_rfc2047(sb, line, display_name_length, encoding); + for (final_line = 0; final_line < sb->len; final_line++) + if (sb->buf[sb->len - final_line - 1] == '\n') + break; + if (namelen - display_name_length + final_line > 78) { + strbuf_addch(sb, '\n'); + if (!isspace(name_tail[0])) + strbuf_addch(sb, ' '); + } strbuf_add(sb, name_tail, namelen - display_name_length); strbuf_addch(sb, '\n'); } else { diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index c3cdb52..7d06716 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -793,4 +793,19 @@ test_expect_success 'format-patch wraps extremely long headers (rfc2047)' ' test_cmp expect subject ' +M8="foo_bar_" +M64=$M8$M8$M8$M8$M8$M8$M8$M8 +cat >expect <<EOF +From: $M64 + <foobar@foo.bar> +EOF +test_expect_success 'format-patch wraps non-quotable headers' ' + rm -rf patches/ && + echo content >>file && + git add file && + git commit -mfoo --author "$M64 <foobar@foo.bar>" && + git format-patch --stdout -1 >patch && + sed -n "/^From: /p; /^ /p; /^$/q" <patch >from && + test_cmp expect from +' test_done -- 1.7.5.rc1.3.ga78bf ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 21:42 ` Jeff King 2011-04-14 22:18 ` Jeff King @ 2011-04-14 22:21 ` Erik Faye-Lund 2011-04-14 22:29 ` Jeff King 1 sibling, 1 reply; 15+ messages in thread From: Erik Faye-Lund @ 2011-04-14 22:21 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Git Mailing List On Thu, Apr 14, 2011 at 11:42 PM, Jeff King <peff@peff.net> wrote: > On Thu, Apr 14, 2011 at 11:19:09PM +0200, Erik Faye-Lund wrote: >> > Or we could just ignore it. AFAICS, this doesn't actually violate >> > rfc2047, nor rfc5322. The 78-character limit is simply a SHOULD, and >> > we have up to 998 for MUST. For a single-address header[1], this seems >> > kind of unlikely to me. >> >> True. But since the fix is as simple as it is, perhaps it's worth it >> just for the clean conscience? > > Fair enough. Patch to follow. > Thinking about it a bit more, I'm getting a bit more unsure: - The 78-limit is about user-interfaces, not protocol robustness. - Since send-email unwraps the line and does not re-wrap it, even if we have a name like this it's likely that the work gets undone right away. - So that means that send-email should probably also be fixed. But now I'm wondering if we've crossed the point where this will just lead to less obvious code for very little gain. So I think I might have over-though this. >> > [1] For multi-address headers like "format-patch --cc=foo --cc=bar", it >> > looks like we already break them across lines. >> >> Yes, but this is even worse: these fields don't get encoded at all! > > Ugh, you're right. That is a totally separate issue, and one I really > don't want to get into. Indeed. I have an itch around this area (I've been playing with porting send-email to C), so I might look at it at some point soon. > Because it means we have to _parse_ those > headers and understand which part is a name and which is an address. > That part is surprisingly easy: If it contains a '<', then it's on the form "Foo Bar Baz <foo@bar.baz>". If not, it's "foo@bar.baz" (assuming it's UTF-8 encoded rfc5322 mailbox'es we assume, which would make the most sense to me) > People who use "--cc" or format.headers will have to deal with that > themselves. I consider both to be somewhat useless, since you can > post-process the mbox after format-patch is run (or in your MUA). > Whereas quoting and encoding fields in format-patch is necessary to give > unambiguous input to the MUA (be it send-email or whatever). > I agree. I'm actually a tad surprised we support it in the first place. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 22:21 ` Erik Faye-Lund @ 2011-04-14 22:29 ` Jeff King 2011-04-14 22:43 ` Erik Faye-Lund 0 siblings, 1 reply; 15+ messages in thread From: Jeff King @ 2011-04-14 22:29 UTC (permalink / raw) To: Erik Faye-Lund; +Cc: Junio C Hamano, Git Mailing List On Fri, Apr 15, 2011 at 12:21:24AM +0200, Erik Faye-Lund wrote: > >> True. But since the fix is as simple as it is, perhaps it's worth it > >> just for the clean conscience? > > > > Fair enough. Patch to follow. > > > > Thinking about it a bit more, I'm getting a bit more unsure: > - The 78-limit is about user-interfaces, not protocol robustness. True. In theory we should also be limiting to avoid the 998-character hard protocol limit, but that is getting ridiculously unlikely. > - Since send-email unwraps the line and does not re-wrap it, even if > we have a name like this it's likely that the work gets undone right > away. Not everybody uses send-email. So you are also helping MUAs which consume the output of format-patch. That being said, I doubt that this will make a difference to anybody. The real reason that we put wrapping into add_rfc2047 was for subjects, which _do_ get long. > - So that means that send-email should probably also be fixed. But now > I'm wondering if we've crossed the point where this will just lead to > less obvious code for very little gain. It is ugly code. I'm just as happy if we drop it. > > Because it means we have to _parse_ those > > headers and understand which part is a name and which is an address. > > That part is surprisingly easy: If it contains a '<', then it's on the form > "Foo Bar Baz <foo@bar.baz>". If not, it's "foo@bar.baz" (assuming it's > UTF-8 encoded rfc5322 mailbox'es we assume, which would make the most > sense to me) What about: "Foo \"The Bar\" Baz" <foo@example.com> or Foo "The Bar" Baz <foo@example.com> or Foo (The Bar) Baz <foo@example.com> I.e., are we taking rfc822-style addresses, or are we taking something that looks vaguely like an email address, and just treating everything left of "<" as literal? -Peff ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 22:29 ` Jeff King @ 2011-04-14 22:43 ` Erik Faye-Lund 2011-04-15 3:30 ` Jeff King 0 siblings, 1 reply; 15+ messages in thread From: Erik Faye-Lund @ 2011-04-14 22:43 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Git Mailing List On Fri, Apr 15, 2011 at 12:29 AM, Jeff King <peff@peff.net> wrote: > On Fri, Apr 15, 2011 at 12:21:24AM +0200, Erik Faye-Lund wrote: > >> >> True. But since the fix is as simple as it is, perhaps it's worth it >> >> just for the clean conscience? >> > >> > Fair enough. Patch to follow. >> > >> >> Thinking about it a bit more, I'm getting a bit more unsure: >> - The 78-limit is about user-interfaces, not protocol robustness. > > True. In theory we should also be limiting to avoid the 998-character > hard protocol limit, but that is getting ridiculously unlikely. I think that's over in my definition of "insanity land", yeah :) >> - Since send-email unwraps the line and does not re-wrap it, even if >> we have a name like this it's likely that the work gets undone right >> away. > > Not everybody uses send-email. So you are also helping MUAs which > consume the output of format-patch. Good point. > That being said, I doubt that this will make a difference to anybody. > The real reason that we put wrapping into add_rfc2047 was for subjects, > which _do_ get long. Absolutely. >> - So that means that send-email should probably also be fixed. But now >> I'm wondering if we've crossed the point where this will just lead to >> less obvious code for very little gain. > > It is ugly code. > > I'm just as happy if we drop it. OK, then I'll try to forget about this issue for now. Sorry for troubling you. >> > Because it means we have to _parse_ those >> > headers and understand which part is a name and which is an address. >> >> That part is surprisingly easy: If it contains a '<', then it's on the form >> "Foo Bar Baz <foo@bar.baz>". If not, it's "foo@bar.baz" (assuming it's >> UTF-8 encoded rfc5322 mailbox'es we assume, which would make the most >> sense to me) > > What about: > > "Foo \"The Bar\" Baz" <foo@example.com> > > or > > Foo "The Bar" Baz <foo@example.com> > > or > > Foo (The Bar) Baz <foo@example.com> > > I.e., are we taking rfc822-style addresses, or are we taking something > that looks vaguely like an email address, and just treating everything > left of "<" as literal? I was just thinking of interpreting everything left of '<' literally and encode it (if needed). Currently, we interpret the entire string literally, encoding the name would an improvement. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 22:43 ` Erik Faye-Lund @ 2011-04-15 3:30 ` Jeff King 2011-04-15 8:32 ` Erik Faye-Lund 0 siblings, 1 reply; 15+ messages in thread From: Jeff King @ 2011-04-15 3:30 UTC (permalink / raw) To: Erik Faye-Lund; +Cc: Junio C Hamano, Git Mailing List On Fri, Apr 15, 2011 at 12:43:08AM +0200, Erik Faye-Lund wrote: > >> That part is surprisingly easy: If it contains a '<', then it's on the form > >> "Foo Bar Baz <foo@bar.baz>". If not, it's "foo@bar.baz" (assuming it's > >> UTF-8 encoded rfc5322 mailbox'es we assume, which would make the most > >> sense to me) > > > > What about: > > > > "Foo \"The Bar\" Baz" <foo@example.com> > > > > or > > > > Foo "The Bar" Baz <foo@example.com> > > > > or > > > > Foo (The Bar) Baz <foo@example.com> > > > > I.e., are we taking rfc822-style addresses, or are we taking something > > that looks vaguely like an email address, and just treating everything > > left of "<" as literal? > > I was just thinking of interpreting everything left of '<' literally > and encode it (if needed). Currently, we interpret the entire string > literally, encoding the name would an improvement. Won't that be a regression for people who already know that we take things literally and are manually quoting and/or rfc2047-encoding the contents? -Peff ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-15 3:30 ` Jeff King @ 2011-04-15 8:32 ` Erik Faye-Lund 2011-04-16 1:45 ` Jeff King 0 siblings, 1 reply; 15+ messages in thread From: Erik Faye-Lund @ 2011-04-15 8:32 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Git Mailing List On Fri, Apr 15, 2011 at 5:30 AM, Jeff King <peff@peff.net> wrote: > On Fri, Apr 15, 2011 at 12:43:08AM +0200, Erik Faye-Lund wrote: > >> >> That part is surprisingly easy: If it contains a '<', then it's on the form >> >> "Foo Bar Baz <foo@bar.baz>". If not, it's "foo@bar.baz" (assuming it's >> >> UTF-8 encoded rfc5322 mailbox'es we assume, which would make the most >> >> sense to me) >> > >> > What about: >> > >> > "Foo \"The Bar\" Baz" <foo@example.com> >> > >> > or >> > >> > Foo "The Bar" Baz <foo@example.com> >> > >> > or >> > >> > Foo (The Bar) Baz <foo@example.com> >> > >> > I.e., are we taking rfc822-style addresses, or are we taking something >> > that looks vaguely like an email address, and just treating everything >> > left of "<" as literal? >> >> I was just thinking of interpreting everything left of '<' literally >> and encode it (if needed). Currently, we interpret the entire string >> literally, encoding the name would an improvement. > > Won't that be a regression for people who already know that we take > things literally and are manually quoting and/or rfc2047-encoding the > contents? Yes. But won't that always be the case when someone depends on buggy behavior? Besides, send-email takes interprets it's --to and --cc arguments as well as sendemail.to and sendemail.cc config options literally (i.e quoting if needed without any attempts on unquoting first). IMO having two closely related programs with similar options that behave different in border-cases is pretty ugly. ESPECIALLY when one of them has a habit of forwarding unknown options to the other, like send-email does... ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-15 8:32 ` Erik Faye-Lund @ 2011-04-16 1:45 ` Jeff King 0 siblings, 0 replies; 15+ messages in thread From: Jeff King @ 2011-04-16 1:45 UTC (permalink / raw) To: Erik Faye-Lund; +Cc: Junio C Hamano, Git Mailing List On Fri, Apr 15, 2011 at 10:32:27AM +0200, Erik Faye-Lund wrote: > >> I was just thinking of interpreting everything left of '<' literally > >> and encode it (if needed). Currently, we interpret the entire string > >> literally, encoding the name would an improvement. > > > > Won't that be a regression for people who already know that we take > > things literally and are manually quoting and/or rfc2047-encoding the > > contents? > > Yes. But won't that always be the case when someone depends on buggy behavior? I guess I don't see the current behavior as necessarily buggy, just sub-optimal. I can imagine people have worked around it by embedding rfc2047-encoded content manually. But I admit I don't really care that much, and I don't know what common use is. Grepping the list archives didn't turn up anything useful. > Besides, send-email takes interprets it's --to and --cc arguments as > well as sendemail.to and sendemail.cc config options literally (i.e > quoting if needed without any attempts on unquoting first). IMO having > two closely related programs with similar options that behave > different in border-cases is pretty ugly. ESPECIALLY when one of them > has a habit of forwarding unknown options to the other, like > send-email does... Yeah, it would be nice to resolve that inconsistency. -Peff ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 17:01 [BUG] format-patch does not wrap From-field after author name Erik Faye-Lund 2011-04-14 17:12 ` Junio C Hamano @ 2011-04-14 17:52 ` Jeff King 2011-04-14 21:06 ` Erik Faye-Lund 2011-04-14 21:07 ` Erik Faye-Lund 1 sibling, 2 replies; 15+ messages in thread From: Jeff King @ 2011-04-14 17:52 UTC (permalink / raw) To: Erik Faye-Lund; +Cc: Git Mailing List On Thu, Apr 14, 2011 at 07:01:41PM +0200, Erik Faye-Lund wrote: > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index 9c66367..a4b8b59 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -793,4 +793,19 @@ test_expect_success 'format-patch wraps extremely > long headers (rfc2047)' ' Speaking of wrapping, your MUA seems to have mangled the patch. > +M8="foo_bar_" > +M64=$M8$M8$M8$M8$M8$M8$M8$M8 > +cat >expect <<'EOF' > +From: foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar > + <foobar@foo.bar> > +EOF Your expect data is missing the trailing "_". You could probably just do: cat >expect <<EOF From: $M64 <foobar@foo.bar> EOF which is even simpler. -Peff ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 17:52 ` Jeff King @ 2011-04-14 21:06 ` Erik Faye-Lund 2011-04-14 21:07 ` Erik Faye-Lund 1 sibling, 0 replies; 15+ messages in thread From: Erik Faye-Lund @ 2011-04-14 21:06 UTC (permalink / raw) To: Jeff King; +Cc: Git Mailing List On Thu, Apr 14, 2011 at 7:52 PM, Jeff King <peff@peff.net> wrote: > On Thu, Apr 14, 2011 at 07:01:41PM +0200, Erik Faye-Lund wrote: > >> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh >> index 9c66367..a4b8b59 100755 >> --- a/t/t4014-format-patch.sh >> +++ b/t/t4014-format-patch.sh >> @@ -793,4 +793,19 @@ test_expect_success 'format-patch wraps extremely >> long headers (rfc2047)' ' > > Speaking of wrapping, your MUA seems to have mangled the patch. > >> +M8="foo_bar_" >> +M64=$M8$M8$M8$M8$M8$M8$M8$M8 >> +cat >expect <<'EOF' >> +From: foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar >> + <foobar@foo.bar> >> +EOF > > Your expect data is missing the trailing "_". You could probably just > do: > > cat >expect <<EOF > From: $M64 > <foobar@foo.bar> > EOF > > which is even simpler. > > -Peff > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] format-patch does not wrap From-field after author name 2011-04-14 17:52 ` Jeff King 2011-04-14 21:06 ` Erik Faye-Lund @ 2011-04-14 21:07 ` Erik Faye-Lund 1 sibling, 0 replies; 15+ messages in thread From: Erik Faye-Lund @ 2011-04-14 21:07 UTC (permalink / raw) To: Jeff King; +Cc: Git Mailing List On Thu, Apr 14, 2011 at 7:52 PM, Jeff King <peff@peff.net> wrote: > On Thu, Apr 14, 2011 at 07:01:41PM +0200, Erik Faye-Lund wrote: > >> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh >> index 9c66367..a4b8b59 100755 >> --- a/t/t4014-format-patch.sh >> +++ b/t/t4014-format-patch.sh >> @@ -793,4 +793,19 @@ test_expect_success 'format-patch wraps extremely >> long headers (rfc2047)' ' > > Speaking of wrapping, your MUA seems to have mangled the patch. > Hehe, yes. How ironic :) What can I say, silly GMail? :P >> +M8="foo_bar_" >> +M64=$M8$M8$M8$M8$M8$M8$M8$M8 >> +cat >expect <<'EOF' >> +From: foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar >> + <foobar@foo.bar> >> +EOF > > Your expect data is missing the trailing "_". You could probably just > do: > > cat >expect <<EOF > From: $M64 > <foobar@foo.bar> > EOF > > which is even simpler. > Yes. Much better, thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-04-16 1:45 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-04-14 17:01 [BUG] format-patch does not wrap From-field after author name Erik Faye-Lund 2011-04-14 17:12 ` Junio C Hamano 2011-04-14 17:50 ` Jeff King 2011-04-14 21:19 ` Erik Faye-Lund 2011-04-14 21:42 ` Jeff King 2011-04-14 22:18 ` Jeff King 2011-04-14 22:21 ` Erik Faye-Lund 2011-04-14 22:29 ` Jeff King 2011-04-14 22:43 ` Erik Faye-Lund 2011-04-15 3:30 ` Jeff King 2011-04-15 8:32 ` Erik Faye-Lund 2011-04-16 1:45 ` Jeff King 2011-04-14 17:52 ` Jeff King 2011-04-14 21:06 ` Erik Faye-Lund 2011-04-14 21:07 ` Erik Faye-Lund
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).