{"thread":{"id":"16468","subject":"[PATCH] send-email: Fix Pine address book parsing","startedAt":"2008-11-26T02:55:00Z","lastAt":"2008-11-26T19:46:15Z","messageCount":6,"participants":["Trent Piepho","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"96511","messageId":"1227668100-5563-1-git-send-email-tpiepho@freescale.com","threadId":"16468","inReplyTo":null,"subject":"[PATCH] send-email: Fix Pine address book parsing","fromName":"Trent Piepho","fromEmail":"tpiepho@freescale.com","sentAt":"2008-11-26T02:55:00Z","receivedAt":"2008-11-26T02:55:00Z","isPatch":true,"sender":{"key":"tpiepho@gmail.com","avatar":null},"body":"See: http://www.washington.edu/pine/tech-notes/low-level.html\n\nEntries with a fcc or comment field after the address weren't parsed\ncorrectly.\n\nContinuation lines, identified by leading spaces, were also not handled.\n\nDistribution lists which had ( ) around a list of addresses did not have\nthe parenthesis removed.\n\nSigned-off-by: Trent Piepho \n---\n git-send-email.perl | 9 ++++++---\n 1 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 94ca5c8..007e2c6 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -345,10 +345,13 @@ my %parse_alias = (\n \t\t\t# spaces delimit multiple addresses\n \t\t\t$aliases{$1} = [ split(/\\s+/, $2) ];\n \t\t}}},\n-\tpine => sub { my $fh = shift; while (<$fh>) {\n-\t\tif (/^(\\S+)\\t.*\\t(.*)$/) {\n+\tpine => sub { my $fh = shift; my $f='\\t[^\\t]*';\n+\t for (my $x = ''; defined($x); $x = $_) {\n+\t\t\tchomp $x;\n+\t\t $x .= $1 while(defined($_ = <$fh>) && /^ +(.*)$/);\n+\t\t\t$x =~ /^(\\S+)$f\\t\\(?([^\\t]+?)\\)?(:?$f){0,2}$/ or next;\n \t\t\t$aliases{$1} = [ split(/\\s*,\\s*/, $2) ];\n-\t\t}}},\n+\t\t}},\n \tgnus => sub { my $fh = shift; while (<$fh>) {\n \t\tif (/\\(define-mail-alias\\s+\"(\\S+?)\"\\s+\"(\\S+?)\"\\)/) {\n \t\t\t$aliases{$1} = [ $2 ];\n-- \n1.5.4.1\n"},{"id":"96513","messageId":"7vod03hyna.fsf@gitster.siamese.dyndns.org","threadId":"16468","inReplyTo":"1227668100-5563-1-git-send-email-tpiepho@freescale.com","subject":"Re: [PATCH] send-email: Fix Pine address book parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-26T04:58:01Z","receivedAt":"2008-11-26T04:58:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Trent Piepho writes:\n\n> See: http://www.washington.edu/pine/tech-notes/low-level.html\n>\n> Entries with a fcc or comment field after the address weren't parsed\n> correctly.\n>\n> Continuation lines, identified by leading spaces, were also not handled.\n>\n> Distribution lists which had ( ) around a list of addresses did not have\n> the parenthesis removed.\n\n> +\tpine => sub { my $fh = shift; my $f='\\t[^\\t]*';\n> +\t for (my $x = ''; defined($x); $x = $_) {\n> +\t\t\tchomp $x;\n> +\t\t $x .= $1 while(defined($_ = <$fh>) && /^ +(.*)$/);\n> +\t\t\t$x =~ /^(\\S+)$f\\t\\(?([^\\t]+?)\\)?(:?$f){0,2}$/ or next;\n\nHmm, so you chomp each continuation line with /^ +(.*)$/ and concatenate\nthat to the hold buffer ($x) as long as you see continuation lines,\na non-continuation line that you read ahead is given to the next round\n(the third part of for(;;) control), checked if you hit an EOF and then\nchomped. Which means the complicated regexp about the parentheses is\napplied to a logical single line in $x that does not have any newline in\nit, right?\n\nI wonder what this does:\n\n\t$x .= $1 while (defined($_ = <$fh>) && /^ +(.*)$/);\n\nwhen you have \"a b\" in $x and feed \" c\\n d\\ne\\n\" to it. When it leaves\nthe loop, you would have \"e\\n\" in $_ for the next round, and \"a bcd\" (note\nthat \"bcd\" becomes one word) in $x, which I suspect may not be what you\nwant.\n\nBut I do not use Pine nor its aliases file.\n"},{"id":"96515","messageId":"Pine.LNX.4.64.0811252137250.5161@t2.domain.actdsltmp","threadId":"16468","inReplyTo":"7vod03hyna.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] send-email: Fix Pine address book parsing","fromName":"Trent Piepho","fromEmail":"tpiepho@freescale.com","sentAt":"2008-11-26T05:59:45Z","receivedAt":"2008-11-26T05:59:45Z","isPatch":true,"sender":{"key":"tpiepho@gmail.com","avatar":null},"body":"On Tue, 25 Nov 2008, Junio C Hamano wrote:\n> Trent Piepho writes:\n>> See: http://www.washington.edu/pine/tech-notes/low-level.html\n>>\n>> Entries with a fcc or comment field after the address weren't parsed\n>> correctly.\n>>\n>> Continuation lines, identified by leading spaces, were also not handled.\n>>\n>> Distribution lists which had ( ) around a list of addresses did not have\n>> the parenthesis removed.\n>\n>> +\tpine => sub { my $fh = shift; my $f='\\t[^\\t]*';\n>> +\t for (my $x = ''; defined($x); $x = $_) {\n>> +\t\t\tchomp $x;\n>> +\t\t $x .= $1 while(defined($_ = <$fh>) && /^ +(.*)$/);\n>> +\t\t\t$x =~ /^(\\S+)$f\\t\\(?([^\\t]+?)\\)?(:?$f){0,2}$/ or next;\n>\n> Hmm, so you chomp each continuation line with /^ +(.*)$/ and concatenate\n> that to the hold buffer ($x) as long as you see continuation lines,\n> a non-continuation line that you read ahead is given to the next round\n> (the third part of for(;;) control), checked if you hit an EOF and then\n> chomped. Which means the complicated regexp about the parentheses is\n> applied to a logical single line in $x that does not have any newline in\n> it, right?\n\nYes. The previous regex would just grab the email address with (\\S+)$, but\nthat's not right. There can be email address with spaces in them, like\n\"John Doe \". And the email address isn't always the last\nfield. So each field has to be put in the regex and \\S+ and \\s* have to\nbecome [^\\t]* and \\t to count fields properly. That's why the regex got so\ncomplex.\n\n> I wonder what this does:\n>\n> \t$x .= $1 while (defined($_ = <$fh>) && /^ +(.*)$/);\n>\n> when you have \"a b\" in $x and feed \" c\\n d\\ne\\n\" to it. When it leaves\n> the loop, you would have \"e\\n\" in $_ for the next round, and \"a bcd\" (note\n> that \"bcd\" becomes one word) in $x, which I suspect may not be what you\n> want.\n\nThe tech docs I linked to just say pine continues lines with leading space,\nbut not how many spaces exactly. From what I can see it appears to usually\nuse three spaces, but sometimes it uses one space when wrapping a very long\ncomment field. It also appears to only split lines between whitespace and\nnon-whitespace. So if \"a b c d\\n\" were to be wrapped, it would be something\nlike \"a b \\n c \\n d\\n\". If I didn't eat the leading spaces in the\ncontinuations, it would be re-assembled as \"a b c d\". This might cause\nan address to become \"John Doe \"\n"},{"id":"96516","messageId":"7vy6z7gf4x.fsf@gitster.siamese.dyndns.org","threadId":"16468","inReplyTo":"Pine.LNX.4.64.0811252137250.5161@t2.domain.actdsltmp","subject":"Re: [PATCH] send-email: Fix Pine address book parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-26T06:44:46Z","receivedAt":"2008-11-26T06:44:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Trent Piepho writes:\n\n> The tech docs I linked to just say pine continues lines with leading space,\n> but not how many spaces exactly.\n\nMy reading of the wording \"spaces\" it uses is that any number. I agree it\nis underspecified what would happen to them.\n\n> It also appears to only split lines between whitespace and\n> non-whitespace. ...\n> ... like \"a b \\n c \\n d\\n\". If I didn't eat the leading spaces in the\n> continuations, it would be re-assembled as \"a b c d\". This might cause\n> an address to become \"John Doe \"\n\nWhich would still work. If you had two addresses a and b and smashed them\ntogether into ab on the other hand it wouldn't. That is why I asked.\n\nIf you know for sure (e.g. by reading the Pine source) that it only splits\na line at a whitespace to non-whitespace transition, that it keeps the\nwhitespace at the end of the first line, and that the non-whitespace and\neverything after that on the second line (prefixed by extra unspecified\nnumber of spaces as the continuation sign), then I think what you had in\nyour patch is exactly what we want. I just wanted to make sure you know\nwhat you are doing, as I do not use Pine nor its address book myself.\n"},{"id":"96536","messageId":"Pine.LNX.4.64.0811260243440.5161@t2.domain.actdsltmp","threadId":"16468","inReplyTo":"7vy6z7gf4x.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] send-email: Fix Pine address book parsing","fromName":"Trent Piepho","fromEmail":"tpiepho@freescale.com","sentAt":"2008-11-26T11:13:09Z","receivedAt":"2008-11-26T11:13:09Z","isPatch":true,"sender":{"key":"tpiepho@gmail.com","avatar":null},"body":"On Tue, 25 Nov 2008, Junio C Hamano wrote:\n> Trent Piepho writes:\n>\n>> The tech docs I linked to just say pine continues lines with leading space,\n>> but not how many spaces exactly.\n>\n> My reading of the wording \"spaces\" it uses is that any number. I agree it\n> is underspecified what would happen to them.\n>\n>> It also appears to only split lines between whitespace and\n>> non-whitespace. ...\n>> ... like \"a b \\n c \\n d\\n\". If I didn't eat the leading spaces in the\n>> continuations, it would be re-assembled as \"a b c d\". This might cause\n>> an address to become \"John Doe \"\n>\n> Which would still work. If you had two addresses a and b and smashed them\n> together into ab on the other hand it wouldn't. That is why I asked.\n\nThough if Pine decided to wrap something mid-word, e.g. \"a@b.com\" to\n\"a@b.\\n com\\n\" then no eating the leading space would be wrong.\n\n> If you know for sure (e.g. by reading the Pine source) that it only splits\n> a line at a whitespace to non-whitespace transition, that it keeps the\n> whitespace at the end of the first line, and that the non-whitespace and\n> everything after that on the second line (prefixed by extra unspecified\n> number of spaces as the continuation sign), then I think what you had in\n> your patch is exactly what we want. I just wanted to make sure you know\n> what you are doing, as I do not use Pine nor its address book myself.\n\nI tried all kinds of extreme address book entries and it would never put\nspaces at the beginning of a line unless they had been inserted to make a\ncontinuation.\n\nBut just for you I downloaded the pine code, which is 20 years old and not\na lot of fun to read. There isn't any code that just does line wrapping. \nThe code that prints out a address book entry will at various points check\nif it should output a newline. And it whatever it prints after those\npoints isn't allowed to start with space. The code that reads the entries\neats all the spaces before and after each field. If long comments are\nwrapped, it will change \"a b\" into \"a\\n : b\\n\" and effectively does \ns/^ +: / /; when reading it back. But email address don't get wrapped\nthat way so we don't need to care about the ':'.\n"},{"id":"96572","messageId":"7vwseqfeyg.fsf@gitster.siamese.dyndns.org","threadId":"16468","inReplyTo":"Pine.LNX.4.64.0811260243440.5161@t2.domain.actdsltmp","subject":"Re: [PATCH] send-email: Fix Pine address book parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-26T19:46:15Z","receivedAt":"2008-11-26T19:46:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Trent Piepho writes:\n\n> ... There isn't any code that just does line wrapping. \n> The code that prints out a address book entry will at various points check\n> if it should output a newline. And it whatever it prints after those\n> points isn't allowed to start with space. The code that reads the entries\n> eats all the spaces before and after each field. If long comments are\n> wrapped, it will change \"a b\" into \"a\\n : b\\n\" and effectively does \n> s/^ +: / /; when reading it back. But email address don't get wrapped\n> that way so we don't need to care about the ':'.\n\nI never was worried about eating and losing the leading space that was\nused only to signal the continuation line, but was wondering if we need to\ninsert an artificial space in place of the newline we are eating, but it\nappears that we can say with confidence that worry is unfounded.\n\nWill queue. Thanks.\n"}]}