{"thread":{"id":"12110","subject":"[PATCH 2/2] test mailinfo rfc3676 support","startedAt":"2008-02-15T02:21:16Z","lastAt":"2008-02-16T14:34:44Z","messageCount":14,"participants":["Jay Soffian","Johannes Schindelin","Junio C Hamano","Derek Fawcus"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"68796","messageId":"1203042077-11385-1-git-send-email-jaysoffian@gmail.com","threadId":"12110","inReplyTo":null,"subject":"[PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2008-02-15T02:21:16Z","receivedAt":"2008-02-15T02:21:16Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"RFC 3676 establishes two parameters (Format and DelSP) to be used with\nthe Text/Plain media type. In the presence of these parameters, trailing\nwhitespace is used to indicate flowed lines and a canonical quote\nindicator is used to indicate quoted lines.\n\nmailinfo now unfolds, unquotes, and un-space-stuffs such messages.\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\nIt's been a while since I hacked C, so mucho scrutiny appreciated. The\nmailinfo testsuite still passes, and this patch is followed by one which\nadds a new test for this code, which also passes.\n\nThis is based off next, but mailinfo hasn't changed in a while, so it should apply cleanly to master (didn't test that though).\n\n builtin-mailinfo.c |   40 ++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 40 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex 2600847..deaf92b 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -20,6 +20,13 @@ static enum  {\n static enum  {\n \tTYPE_TEXT, TYPE_OTHER,\n } message_type;\n+/* RFC 3676 Text/Plain Format and DelSp Parameters */\n+static enum {\n+\tFORMAT_NONE, FORMAT_FIXED, FORMAT_FLOWED,\n+} tp_format;\n+static enum {\n+\tDELSP_NONE, DELSP_YES, DELSP_NO,\n+} tp_delsp;\n \n static char charset[256];\n static int patch_lines;\n@@ -193,6 +200,18 @@ static int handle_content_type(char *line)\n \n \tif (strcasestr(line, \"text/\") == NULL)\n \t\t message_type = TYPE_OTHER;\n+\telse if (strcasestr(line, \"text/plain\")) {\n+\t\tchar attr[256];\n+\t\tif (slurp_attr(line, \"format=\", attr) && !strcasecmp(attr, \"flowed\")) {\n+\t\t\ttp_format = FORMAT_FLOWED;\n+\t\t\tif (slurp_attr(line, \"delsp=\", attr) && !strcasecmp(attr, \"yes\"))\n+\t\t\t\ttp_delsp = DELSP_YES;\n+\t\t\telse\n+\t\t\t\ttp_delsp = DELSP_NO;\n+\t\t}\n+\t\telse\n+\t\t\ttp_format = FORMAT_FIXED;\n+\t}\n \tif (slurp_attr(line, \"boundary=\", boundary + 2)) {\n \t\tmemcpy(boundary, \"--\", 2);\n \t\tif (content_top++ >= &content[MAX_BOUNDARIES]) {\n@@ -681,6 +700,8 @@ again:\n \ttransfer_encoding = TE_DONTCARE;\n \tcharset[0] = 0;\n \tmessage_type = TYPE_TEXT;\n+\ttp_format = FORMAT_NONE;\n+\ttp_delsp = DELSP_NONE;\n \n \t/* slurp in this section's info */\n \twhile (read_one_header_line(line, sizeof(line), fin))\n@@ -770,6 +791,24 @@ static int handle_filter(char *line, unsigned linesize)\n {\n \tstatic int filter = 0;\n \n+\tif (tp_format == FORMAT_FLOWED && !!strcmp(line, \"-- \\n\")) {\n+\t\tchar *cp = line;\n+\t\twhile (*cp == '>' && *cp != 0)\n+\t\t\tcp++;\n+\t\tif (*cp == ' ')\n+\t\t\tcp++;\n+\t\tline = cp;\n+\t\tif (!!strcmp(line, \"-- \\n\")) {\n+\t\t\twhile (*cp != '\\n' && *cp !=0)\n+\t\t\t\tcp++;\n+\t\t\tif (cp > line && *cp == '\\n' && *(cp-1) == ' ') {\n+\t\t\t\tif (tp_delsp == DELSP_YES)\n+\t\t\t\t\t*(cp-1) = '\\0';\n+\t\t\t\telse\n+\t\t\t\t\t*cp = '\\0';\n+\t\t\t}\n+\t\t}\n+\t}\n \t/* filter tells us which part we left off on\n \t * a non-zero return indicates we hit a filter point\n \t */\n@@ -818,6 +857,7 @@ static void handle_body(void)\n \n \t\tswitch (transfer_encoding) {\n \t\tcase TE_BASE64:\n+\t\tcase TE_QP:\n \t\t{\n \t\t\tchar *op = line;\n \n-- \n1.5.4.1.1281.g75df\n"},{"id":"68795","messageId":"1203042077-11385-2-git-send-email-jaysoffian@gmail.com","threadId":"12110","inReplyTo":"1203042077-11385-1-git-send-email-jaysoffian@gmail.com","subject":"[PATCH 2/2] test mailinfo rfc3676 support","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2008-02-15T02:21:17Z","receivedAt":"2008-02-15T02:21:17Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Adds a format=flowed message to the sample.mbox in order to test\nmailinfo's rfc3676 support.\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\n t/t5100-mailinfo.sh |    2 +-\n t/t5100/info0009    |    5 ++\n t/t5100/msg0009     |    8 +++\n t/t5100/patch0009   |   98 ++++++++++++++++++++++++++++++++++++\n t/t5100/sample.mbox |  138 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 250 insertions(+), 1 deletions(-)\n create mode 100644 t/t5100/info0009\n create mode 100644 t/t5100/msg0009\n create mode 100644 t/t5100/patch0009\n\ndiff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh\nindex 9b1a745..d6c55c1 100755\n--- a/t/t5100-mailinfo.sh\n+++ b/t/t5100-mailinfo.sh\n@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \\\n \t'git mailsplit -o. ../t5100/sample.mbox >last &&\n \tlast=`cat last` &&\n \techo total is $last &&\n-\ttest `cat last` = 8'\n+\ttest `cat last` = 9'\n \n for mail in `echo 00*`\n do\ndiff --git a/t/t5100/info0009 b/t/t5100/info0009\nnew file mode 100644\nindex 0000000..63369be\n--- /dev/null\n+++ b/t/t5100/info0009\n@@ -0,0 +1,5 @@\n+Author: A U Thor\n+Email: a.u.thor@example.com\n+Subject: mailinfo: support rfc3676 (format=flowed) text/plain messages\n+Date: Thu, 14 Feb 2008 20:56:34 -0500\n+\ndiff --git a/t/t5100/msg0009 b/t/t5100/msg0009\nnew file mode 100644\nindex 0000000..eb9c7f8\n--- /dev/null\n+++ b/t/t5100/msg0009\n@@ -0,0 +1,8 @@\n+RFC 3676 establishes two parameters (Format and DelSP) to be used with\n+the Text/Plain media type. In the presence of these parameters, trailing\n+whitespace is used to indicate flowed lines and a canonical quote\n+indicator is used to indicate quoted lines.\n+\n+mailinfo now unfolds, unquotes, and un-space-stuffs such messages.\n+\n+Signed-off-by: A U Thor <a.u.thor@example.com.com\ndiff --git a/t/t5100/patch0009 b/t/t5100/patch0009\nnew file mode 100644\nindex 0000000..a167e73\n--- /dev/null\n+++ b/t/t5100/patch0009\n@@ -0,0 +1,98 @@\n+---\n+The next line will test space stuffing\n+From A U Thor <a.u.thor@example.com.com>\n+\n+The next line will test space stuffing also, and force Mail.app to send this as quoted-printable. This line will be flowed.\n+> Märchen\n+\n+A flowed quoted paragraph follows:\n+Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.\n+\n+And again with deeper quoting depth:\n+Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.\n+\n+\n+builtin-mailinfo.c |   40 ++++++++++++++++++++++++++++++++++++++++\n+1 files changed, 40 insertions(+), 0 deletions(-)\n+\n+diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\n+index 2600847..deaf92b 100644\n+--- a/builtin-mailinfo.c\n++++ b/builtin-mailinfo.c\n+@@ -20,6 +20,13 @@ static enum  {\n+static enum  {\n+\tTYPE_TEXT, TYPE_OTHER,\n+} message_type;\n++/* RFC 3676 Text/Plain Format and DelSp Parameters */\n++static enum {\n++\tFORMAT_NONE, FORMAT_FIXED, FORMAT_FLOWED,\n++} tp_format;\n++static enum {\n++\tDELSP_NONE, DELSP_YES, DELSP_NO,\n++} tp_delsp;\n+\n+static char charset[256];\n+static int patch_lines;\n+@@ -193,6 +200,18 @@ static int handle_content_type(char *line)\n+\n+\tif (strcasestr(line, \"text/\") == NULL)\n+\t\t message_type = TYPE_OTHER;\n++\telse if (strcasestr(line, \"text/plain\")) {\n++\t\tchar attr[256];\n++\t\tif (slurp_attr(line, \"format=\", attr) && !strcasecmp(attr, \"flowed\")) {\n++\t\t\ttp_format = FORMAT_FLOWED;\n++\t\t\tif (slurp_attr(line, \"delsp=\", attr) && !strcasecmp(attr, \"yes\"))\n++\t\t\t\ttp_delsp = DELSP_YES;\n++\t\t\telse\n++\t\t\t\ttp_delsp = DELSP_NO;\n++\t\t}\n++\t\telse\n++\t\t\ttp_format = FORMAT_FIXED;\n++\t}\n+\tif (slurp_attr(line, \"boundary=\", boundary + 2)) {\n+\t\tmemcpy(boundary, \"--\", 2);\n+\t\tif (content_top++ >= &content[MAX_BOUNDARIES]) {\n+@@ -681,6 +700,8 @@ again:\n+\ttransfer_encoding = TE_DONTCARE;\n+\tcharset[0] = 0;\n+\tmessage_type = TYPE_TEXT;\n++\ttp_format = FORMAT_NONE;\n++\ttp_delsp = DELSP_NONE;\n+\n+\t/* slurp in this section's info */\n+\twhile (read_one_header_line(line, sizeof(line), fin))\n+@@ -770,6 +791,24 @@ static int handle_filter(char *line, unsigned linesize)\n+{\n+\tstatic int filter = 0;\n+\n++\tif (tp_format == FORMAT_FLOWED && !!strcmp(line, \"-- \\n\")) {\n++\t\tchar *cp = line;\n++\t\twhile (*cp == '>' && *cp != 0)\n++\t\t\tcp++;\n++\t\tif (*cp == ' ')\n++\t\t\tcp++;\n++\t\tline = cp;\n++\t\tif (!!strcmp(line, \"-- \\n\")) {\n++\t\t\twhile (*cp != '\\n' && *cp !=0)\n++\t\t\t\tcp++;\n++\t\t\tif (cp > line && *cp == '\\n' && *(cp-1) == ' ') {\n++\t\t\t\tif (tp_delsp == DELSP_YES)\n++\t\t\t\t\t*(cp-1) = '\\0';\n++\t\t\t\telse\n++\t\t\t\t\t*cp = '\\0';\n++\t\t\t}\n++\t\t}\n++\t}\n+\t/* filter tells us which part we left off on\n+\t * a non-zero return indicates we hit a filter point\n+\t */\n+@@ -818,6 +857,7 @@ static void handle_body(void)\n+\n+\t\tswitch (transfer_encoding) {\n+\t\tcase TE_BASE64:\n++\t\tcase TE_QP:\n+\t\t{\n+\t\t\tchar *op = line;\n+\n+-- \n+1.5.4.1.1281.g75df\ndiff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox\nindex 070c166..a1f9833 100644\n--- a/t/t5100/sample.mbox\n+++ b/t/t5100/sample.mbox\n@@ -407,3 +407,141 @@ Subject: [PATCH] another patch\n \n Hey you forgot the patch!\n \n+From nobody Thu Feb 14 21:02:08 2008\n+Message-Id: <FCDFE42A-6A58-4F77-AEF2-E94C5373B14F@soffian.org>\n+From: A U Thor <a.u.thor@example.com>\n+To: A U Thor <a.u.thor@example.com>\n+Content-Type: text/plain; charset=UTF-8; format=flowed; delsp=yes\n+Content-Transfer-Encoding: quoted-printable\n+Mime-Version: 1.0 (Apple Message framework v919.2)\n+Subject: [PATCH] mailinfo: support rfc3676 (format=flowed) text/plain messages\n+Date: Thu, 14 Feb 2008 20:56:34 -0500\n+X-Mailer: Apple Mail (2.919.2)\n+\n+RFC 3676 establishes two parameters (Format and DelSP) to be used with\n+the Text/Plain media type. In the presence of these parameters, trailing\n+whitespace is used to indicate flowed lines and a canonical quote\n+indicator is used to indicate quoted lines.\n+\n+mailinfo now unfolds, unquotes, and un-space-stuffs such messages.\n+\n+Signed-off-by: A U Thor <a.u.thor@example.com.com\n+---\n+The next line will test space stuffing\n+ =46rom A U Thor <a.u.thor@example.com.com>\n+\n+The next line will test space stuffing also, and force Mail.app to =20\n+send this as quoted-printable. This line will be flowed.\n+ > M=C3=A4rchen\n+\n+A flowed quoted paragraph follows:\n+> Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do =20\n+> eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim =20=\n+\n+> ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut =20\n+> aliquip ex ea commodo consequat. Duis aute irure dolor in =20\n+> reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla =20=\n+\n+> pariatur. Excepteur sint occaecat cupidatat non proident, sunt in =20\n+> culpa qui officia deserunt mollit anim id est laborum.\n+\n+And again with deeper quoting depth:\n+>>> Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do =20\n+>>> eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut =20\n+>>> enim ad minim veniam, quis nostrud exercitation ullamco laboris =20\n+>>> nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in =20=\n+\n+>>> reprehenderit in voluptate velit esse cillum dolore eu fugiat =20\n+>>> nulla pariatur. Excepteur sint occaecat cupidatat non proident, =20\n+>>> sunt in culpa qui officia deserunt mollit anim id est laborum.\n+\n+\n+builtin-mailinfo.c |   40 ++++++++++++++++++++++++++++++++++++++++\n+1 files changed, 40 insertions(+), 0 deletions(-)\n+\n+diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\n+index 2600847..deaf92b 100644\n+--- a/builtin-mailinfo.c\n++++ b/builtin-mailinfo.c\n+@@ -20,6 +20,13 @@ static enum  {\n+static enum  {\n+\tTYPE_TEXT, TYPE_OTHER,\n+} message_type;\n++/* RFC 3676 Text/Plain Format and DelSp Parameters */\n++static enum {\n++\tFORMAT_NONE, FORMAT_FIXED, FORMAT_FLOWED,\n++} tp_format;\n++static enum {\n++\tDELSP_NONE, DELSP_YES, DELSP_NO,\n++} tp_delsp;\n+\n+static char charset[256];\n+static int patch_lines;\n+@@ -193,6 +200,18 @@ static int handle_content_type(char *line)\n+\n+\tif (strcasestr(line, \"text/\") =3D=3D NULL)\n+\t\t message_type =3D TYPE_OTHER;\n++\telse if (strcasestr(line, \"text/plain\")) {\n++\t\tchar attr[256];\n++\t\tif (slurp_attr(line, \"format=3D\", attr) && =\n+!strcasecmp(attr, =20\n+\"flowed\")) {\n++\t\t\ttp_format =3D FORMAT_FLOWED;\n++\t\t\tif (slurp_attr(line, \"delsp=3D\", attr) && =\n+!strcasecmp(attr, \"yes\"))\n++\t\t\t\ttp_delsp =3D DELSP_YES;\n++\t\t\telse\n++\t\t\t\ttp_delsp =3D DELSP_NO;\n++\t\t}\n++\t\telse\n++\t\t\ttp_format =3D FORMAT_FIXED;\n++\t}\n+\tif (slurp_attr(line, \"boundary=3D\", boundary + 2)) {\n+\t\tmemcpy(boundary, \"--\", 2);\n+\t\tif (content_top++ >=3D &content[MAX_BOUNDARIES]) {\n+@@ -681,6 +700,8 @@ again:\n+\ttransfer_encoding =3D TE_DONTCARE;\n+\tcharset[0] =3D 0;\n+\tmessage_type =3D TYPE_TEXT;\n++\ttp_format =3D FORMAT_NONE;\n++\ttp_delsp =3D DELSP_NONE;\n+\n+\t/* slurp in this section's info */\n+\twhile (read_one_header_line(line, sizeof(line), fin))\n+@@ -770,6 +791,24 @@ static int handle_filter(char *line, unsigned =20\n+linesize)\n+{\n+\tstatic int filter =3D 0;\n+\n++\tif (tp_format =3D=3D FORMAT_FLOWED && !!strcmp(line, \"-- \\n\")) {\n++\t\tchar *cp =3D line;\n++\t\twhile (*cp =3D=3D '>' && *cp !=3D 0)\n++\t\t\tcp++;\n++\t\tif (*cp =3D=3D ' ')\n++\t\t\tcp++;\n++\t\tline =3D cp;\n++\t\tif (!!strcmp(line, \"-- \\n\")) {\n++\t\t\twhile (*cp !=3D '\\n' && *cp !=3D0)\n++\t\t\t\tcp++;\n++\t\t\tif (cp > line && *cp =3D=3D '\\n' && *(cp-1) =3D=3D=\n+ ' ') {\n++\t\t\t\tif (tp_delsp =3D=3D DELSP_YES)\n++\t\t\t\t\t*(cp-1) =3D '\\0';\n++\t\t\t\telse\n++\t\t\t\t\t*cp =3D '\\0';\n++\t\t\t}\n++\t\t}\n++\t}\n+\t/* filter tells us which part we left off on\n+\t * a non-zero return indicates we hit a filter point\n+\t */\n+@@ -818,6 +857,7 @@ static void handle_body(void)\n+\n+\t\tswitch (transfer_encoding) {\n+\t\tcase TE_BASE64:\n++\t\tcase TE_QP:\n+\t\t{\n+\t\t\tchar *op =3D line;\n+\n+--=20\n+1.5.4.1.1281.g75df\n-- \n1.5.4.1.1281.g75df\n"},{"id":"68808","messageId":"alpine.LSU.1.00.0802151035100.30505@racer.site","threadId":"12110","inReplyTo":"1203042077-11385-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-15T10:41:31Z","receivedAt":"2008-02-15T10:41:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 14 Feb 2008, Jay Soffian wrote:\n\n> diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\n> index 2600847..deaf92b 100644\n> --- a/builtin-mailinfo.c\n> +++ b/builtin-mailinfo.c\n> @@ -20,6 +20,13 @@ static enum  {\n>  static enum  {\n>  \tTYPE_TEXT, TYPE_OTHER,\n>  } message_type;\n> +/* RFC 3676 Text/Plain Format and DelSp Parameters */\n> +static enum {\n> +\tFORMAT_NONE, FORMAT_FIXED, FORMAT_FLOWED,\n> +} tp_format;\n\nWhy not call it \"enum message_format\"?\n\n> +static enum {\n> +\tDELSP_NONE, DELSP_YES, DELSP_NO,\n> +} tp_delsp;\n\nWhy not call it \"enum message_is_delsp\"?\n\n> @@ -193,6 +200,18 @@ static int handle_content_type(char *line)\n>  \n>  \tif (strcasestr(line, \"text/\") == NULL)\n>  \t\t message_type = TYPE_OTHER;\n> +\telse if (strcasestr(line, \"text/plain\")) {\n> +\t\tchar attr[256];\n> +\t\tif (slurp_attr(line, \"format=\", attr) && !strcasecmp(attr, \"flowed\")) {\n> +\t\t\ttp_format = FORMAT_FLOWED;\n> +\t\t\tif (slurp_attr(line, \"delsp=\", attr) && !strcasecmp(attr, \"yes\"))\n> +\t\t\t\ttp_delsp = DELSP_YES;\n> +\t\t\telse\n> +\t\t\t\ttp_delsp = DELSP_NO;\n> +\t\t}\n> +\t\telse\n> +\t\t\ttp_format = FORMAT_FIXED;\n\nDoes that mean that the format is only set if the content type is \n\"text/plain\"?\n\n> @@ -681,6 +700,8 @@ again:\n>  \ttransfer_encoding = TE_DONTCARE;\n>  \tcharset[0] = 0;\n>  \tmessage_type = TYPE_TEXT;\n> +\ttp_format = FORMAT_NONE;\n> +\ttp_delsp = DELSP_NONE;\n>  \n>  \t/* slurp in this section's info */\n>  \twhile (read_one_header_line(line, sizeof(line), fin))\n> @@ -770,6 +791,24 @@ static int handle_filter(char *line, unsigned linesize)\n>  {\n>  \tstatic int filter = 0;\n>  \n> +\tif (tp_format == FORMAT_FLOWED && !!strcmp(line, \"-- \\n\")) {\n\nThe !! is unnecessary; please skip it.\n\n> +\t\tchar *cp = line;\n> +\t\twhile (*cp == '>' && *cp != 0)\n> +\t\t\tcp++;\n> +\t\tif (*cp == ' ')\n> +\t\t\tcp++;\n> +\t\tline = cp;\n\nHow about using strchrnul()?\n\nNote: I do not know enough about format=flawed to know why you should skip \nto \"> \".  I would have expected \"\\n\", though.\n\n> +\t\tif (!!strcmp(line, \"-- \\n\")) {\n\nThe !! is unnecessary; please skip it.\n\n> +\t\t\twhile (*cp != '\\n' && *cp !=0)\n> +\t\t\t\tcp++;\n\nAgain, this is the job for strchrnul().\n\n> +\t\t\tif (cp > line && *cp == '\\n' && *(cp-1) == ' ') {\n> +\t\t\t\tif (tp_delsp == DELSP_YES)\n> +\t\t\t\t\t*(cp-1) = '\\0';\n> +\t\t\t\telse\n> +\t\t\t\t\t*cp = '\\0';\n> +\t\t\t}\n\nOr maybe\n\t\t\t\tcp[0 - (tp_delsp == DELSP_YES)] = '\\0';\n\nBut maybe that is too cute.\n\nBut another thing struck me here: why setting *cp = '\\0'; only if *(cp-1) \n== ' ', even if tp_delsp != DELSP_YES?\n\n> @@ -818,6 +857,7 @@ static void handle_body(void)\n>  \n>  \t\tswitch (transfer_encoding) {\n>  \t\tcase TE_BASE64:\n> +\t\tcase TE_QP:\n>  \t\t{\n>  \t\t\tchar *op = line;\n\nDid that just slip in, or was this intended.  If the latter, is this \nrelated to format=flawed, or is it a bug fix in its own right?\n\nCiao,\nDscho\n"},{"id":"68810","messageId":"alpine.LSU.1.00.0802151058270.30505@racer.site","threadId":"12110","inReplyTo":"1203042077-11385-2-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH 2/2] test mailinfo rfc3676 support","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-15T11:01:05Z","receivedAt":"2008-02-15T11:01:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 14 Feb 2008, Jay Soffian wrote:\n\n> +@@ -20,6 +20,13 @@ static enum  {\n> +static enum  {\n> +\tTYPE_TEXT, TYPE_OTHER,\n> +} message_type;\n> ++/* RFC 3676 Text/Plain Format and DelSp Parameters */\n> ++static enum {\n> ++\tFORMAT_NONE, FORMAT_FIXED, FORMAT_FLOWED,\n> ++} tp_format;\n> ++static enum {\n> ++\tDELSP_NONE, DELSP_YES, DELSP_NO,\n> ++} tp_delsp;\n> +\n> +static char charset[256];\n> +static int patch_lines;\n\nHmm.  Such a corrupt patch (lacking spaces at the beginning of the line) \nwould not be accepted by git-apply.  I briefly thought about teaching \ngit-apply to grok that, with a flag.  But now I think that mailsplit \nshould handle that, no?\n\nQuestion is: can you \"de-corruptify\" such a patch? (Note: it would \nprobably need a validating step, too, i.e. count the lines it added a \nspace to, and match that up with the numbers in the @@ lines)\n\nCiao,\nDscho\n"},{"id":"68813","messageId":"76718490802150835i3f56cc03r149b5ecf946bbd58@mail.gmail.com","threadId":"12110","inReplyTo":"alpine.LSU.1.00.0802151035100.30505@racer.site","subject":"Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2008-02-15T16:35:28Z","receivedAt":"2008-02-15T16:35:28Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 15, 2008 at 5:41 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n\n>  Why not call it \"enum message_format\"?\n\nIt's only applicable to text/plain messages. Then again, this will be\nset to FORMAT_NONE for non-text/plain messages so I guess that's just as\ngood a name.\n\nBTW, since all I care about is format=flowed, I could just as easily use\nan int and call it something like message_is_flowed. This would actually\nsimplify the code a bit but I was trying to follow the existing code\nwhich uses an enum for message_type that could similarly have been an\nint called message_type_is_text.\n\n>  Why not call it \"enum message_is_delsp\"?\n\nSure, my reasoning is same as above.\n\n>  > @@ -193,6 +200,18 @@ static int handle_content_type(char *line)\n>  >\n>  >       if (strcasestr(line, \"text/\") == NULL)\n>  >                message_type = TYPE_OTHER;\n>  > +     else if (strcasestr(line, \"text/plain\")) {\n>  > +             char attr[256];\n>  > +             if (slurp_attr(line, \"format=\", attr) && !strcasecmp(attr, \"flowed\")) {\n>  > +                     tp_format = FORMAT_FLOWED;\n>  > +                     if (slurp_attr(line, \"delsp=\", attr) && !strcasecmp(attr, \"yes\"))\n>  > +                             tp_delsp = DELSP_YES;\n>  > +                     else\n>  > +                             tp_delsp = DELSP_NO;\n>  > +             }\n>  > +             else\n>  > +                     tp_format = FORMAT_FIXED;\n>\n>  Does that mean that the format is only set if the content type is\n>  \"text/plain\"?\n\ntp_format is initially set to FORMAT_NONE. Only for text/plain is it\nthem set to FORMAT_FIXED or FORMAT_FLOWED. Again, though, all I care\nabout is format=flowed so I could be using an int called\nmessage_is_flowed but was following the enum message_type example.\n\n>  > +     if (tp_format == FORMAT_FLOWED && !!strcmp(line, \"-- \\n\")) {\n>\n>  The !! is unnecessary; please skip it.\n\nHeh. I'd never seen that before I started perusing the git code, I was\njust following along. But looking more carefully now I see that the\nother code only does that if it's assigning to an int. I see that it's\nnot needed in a boolean context.\n\n>  How about using strchrnul()?\n\nCool. I'd never heard of it.\n\n>  > +                     if (cp > line && *cp == '\\n' && *(cp-1) == ' ') {\n>  > +                             if (tp_delsp == DELSP_YES)\n>  > +                                     *(cp-1) = '\\0';\n>  > +                             else\n>  > +                                     *cp = '\\0';\n>  > +                     }\n>\n>  Or maybe\n>                                 cp[0 - (tp_delsp == DELSP_YES)] = '\\0';\n>\n>  But maybe that is too cute.\n\nHeh, I think that's too clever by half.\n\n>  But another thing struck me here: why setting *cp = '\\0'; only if *(cp-1)\n>  == ' ', even if tp_delsp != DELSP_YES?\n\n\" \\n$\" indicates the line is flowed, meaning the MUA added the '\\n' to\nthe line and we need to remove it. The next question is whether the\nspace before the '\\n' was also inserted by the MUA. If DELSP_YES, it\nwas, so we want to remove it, else it was an existing space that we want\nto keep.\n\n>  > @@ -818,6 +857,7 @@ static void handle_body(void)\n>  >\n>  >               switch (transfer_encoding) {\n>  >               case TE_BASE64:\n>  > +             case TE_QP:\n>  >               {\n>  >                       char *op = line;\n>\n>  Did that just slip in, or was this intended.  If the latter, is this\n>  related to format=flawed, or is it a bug fix in its own right?\n\nIt was intended. The patch doesn't have enough context to have included\nthis comment inside the TE_BASE64 case:\n\n\t/* this is a decoded line that may contain\n\t * multiple new lines.  Pass only one chunk\n\t * at a time to handle_filter()\n\t */\n\nIt turns out that decoding QP can also cause a decoded line to contain\nextra newlines. So it's a bug fix, but I'm not sure it mattered before.\nI'll break it out into a separate patch though to make that clear. And I\nknow I should add a test first for that fix, but ugh, figuring out the\ntest case for a one-line code change is painful.\n\nThanks for the comments, I'll follow up with a revised patch.\n\nj.\n"},{"id":"68814","messageId":"76718490802150844w7cc583b7v4d3480ed43de5cd1@mail.gmail.com","threadId":"12110","inReplyTo":"alpine.LSU.1.00.0802151058270.30505@racer.site","subject":"Re: [PATCH 2/2] test mailinfo rfc3676 support","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2008-02-15T16:44:24Z","receivedAt":"2008-02-15T16:44:24Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 15, 2008 at 6:01 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>  Hmm.  Such a corrupt patch (lacking spaces at the beginning of the line)\n>  would not be accepted by git-apply.  I briefly thought about teaching\n>  git-apply to grok that, with a flag.  But now I think that mailsplit\n>  should handle that, no?\n>\n>  Question is: can you \"de-corruptify\" such a patch? (Note: it would\n>  probably need a validating step, too, i.e. count the lines it added a\n>  space to, and match that up with the numbers in the @@ lines)\n\nHeh, I see this was a confusing patch. It is a lot clearer to read if\nyou apply it and then take a look at what's added to sample.mbox, as\nwell as look at the new files in t5100.\n\nWhat I did to create this test was add the format=flowed code to\nmailinfo (my previous patch email), extract it with format-patch, then\ncut/paste it into my MUA and mail it to myself so I could have a sample\nformat=flowed message. I then added that to the sample.mbox to see if\nthe code I'd just added to mailinfo was working properly. Yes, it\n\"de-corrupted\" it just fine by removing the flowing.\n\nj.\n"},{"id":"68816","messageId":"7vr6fei1s4.fsf@gitster.siamese.dyndns.org","threadId":"12110","inReplyTo":"1203042077-11385-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-15T17:10:19Z","receivedAt":"2008-02-15T17:10:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I really do not like this.\n\nformat=flowed instructs the receivers MUA that it is Ok to\nreflow the text when the message is presented to the user.  That\nis exactly what we do _NOT_ want to happen to patches.  Your\nimplementation may cleanly salvage what the sender intended to\nsend, but salvaging when applying the patch is too late.\n\nYou review the patch, decide to apply or reject, and then\nfinally apply.  Unmangling of corrupt contents should be done\nbefore you review, not before you apply.\n\nWe have similar hacks to clean-up MIME attachments and CTE.\nThey are useful when your mailpath is not clean and can corrupt\ncontents even if you try to send it as text/plain in-line patch\nas a fallback measure to ensure the contents not to get\ncorrupted.  format=flowed is completely opposite --- you are\ngiving your and recipients MUA freedom to reflow the text, but\nthere is no valid reason to allow that when sending patches.\n\nWe do not even encourage MIME attachments and ask senders to try\nsending uncorrupt patch in-line, _even though_ MIME attachments\nis a way to try harder not to corrupt the payload.  Why should\nwe encourage format=flowed which is meant to do the opposite\n(i.e. we do not care about the exact content, we care more about\nhow better the paragraph looks and easier to read on screens\nwith different width)?\n\nThe format is not meant for the exact transmission of text (like\n\"patch\") but more for paragraphed prose that can be re-fit on\nnarrower display like phones.  Section 5. even goes to say\n\"Hand-aligned text, such as ASCII tables or art, source code,\netc., SHOULD be sent as fixed, not flowed lines.\"\n\nSide note.  I did not look at the patch very carefully, but can\nyou salvage a deleted text in the patch that removes a line that\nconsists of \"- \" (minus followed by a single space and then\nend-of-line), or any deleted or added text that ends with a SP\nwithout making them misinterpreted as \"flowed\" line for that\nmatter?\n\nI even suspect that the sending MUA client may misbehave for\nsuch a patch line.  In fact, doesn't section 4.2 say \"a\ngenerating agant should trim spaces before user-inserted hard\nline breaks.\"?  It implies to me that you cannot have a fixed\nline that ends with SP.\n\nSo just reject the patch when somebody sends you format=flowed\nand ask them to re-send without =flowed, and the world will be\na much better place.\n"},{"id":"68830","messageId":"76718490802151037m87995c2gacf29667259eae41@mail.gmail.com","threadId":"12110","inReplyTo":"7vr6fei1s4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2008-02-15T18:37:49Z","receivedAt":"2008-02-15T18:37:49Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 15, 2008 at 12:10 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> I really do not like this.\n\nUgh.\n\n>  format=flowed instructs the receivers MUA that it is Ok to\n>  reflow the text when the message is presented to the user.\n\nNo, it indicates that the MUA sender mangled the message, but did so in\na way that it can be unmanged* as long as the receiving MUA implements\nRFC 3676.\n\n* Trailing whitespace removed by the MUA before sending is the one\n  exception that cannot be unmangled; I'll address this further below.\n\n>  You review the patch, decide to apply or reject, and then\n>  finally apply.  Unmangling of corrupt contents should be done\n>  before you review, not before you apply.\n\nAs long as you're reviewing the patch in an MUA which implements RFC\n3676, you'll be reviewing the unmangled patch. By your reasoning,\nmailinfo shouldn't decode QP or BASE64 either. If you want to make 100%\nsure you're reviewing *exactly* what you plan to apply, then you need to\nrun it through git-am first and then review it via git-diff, no?\n\nIf you're reviewing in your MUA, your MUA may already be taking care of\nQP or BASE64 for you. If the MUA is RFC 3676 aware, it's un-flowing\nformat=flowed too. mailinfo should do the same.\n\n>  format=flowed is completely opposite --- you are\n>  giving your and recipients MUA freedom to reflow the text, but\n>  there is no valid reason to allow that when sending patches.\n\nNot all MUA's can be configured to disable format=flowed. Sure, it's\nbest if folks send 100% plain/text w/o any mangling, but their MUA may\nalready be doing B64 or QP out of their control.\n\n>  We do not even encourage MIME attachments and ask senders to try\n>  sending uncorrupt patch in-line, _even though_ MIME attachments\n>  is a way to try harder not to corrupt the payload.  Why should\n>  we encourage format=flowed\n\nWe're not trying to encourage it, just accommodate it where the sender\ncan't disable it.\n\n>  The format is not meant for the exact transmission of text (like\n>  \"patch\") but more for paragraphed prose that can be re-fit on\n>  narrower display like phones.  Section 5. even goes to say\n>  \"Hand-aligned text, such as ASCII tables or art, source code,\n>  etc., SHOULD be sent as fixed, not flowed lines.\"\n\nAgain, some MUA's aren't configurable. They apply format=flowed to any\ntext/plain message.\n\n>  Side note.  I did not look at the patch very carefully, but can\n>  you salvage a deleted text in the patch that removes a line that\n>  consists of \"- \" (minus followed by a single space and then\n>  end-of-line), or any deleted or added text that ends with a SP\n>  without making them misinterpreted as \"flowed\" line for that\n>  matter?\n>\n>  I even suspect that the sending MUA client may misbehave for\n>  such a patch line.  In fact, doesn't section 4.2 say \"a\n>  generating agant should trim spaces before user-inserted hard\n>  line breaks.\"?  It implies to me that you cannot have a fixed\n>  line that ends with SP.\n\nYes, the sending MUA likely striped trailing whitespace and this cannot\nbe recovered. Let's look at this in practice to see where it can be a\nproblem.\n\nExisting code generally shouldn't have trailing whitespace nor\nwhitespace only lines. However, let's say that it does and that the\npatch refers to one of these lines (either as context or a subtraction).\nIn this case that hunk will fail to apply, unless we teach git-apply to\nbe lenient if the only difference between the line in the patch and the\nexisting code is trailing whitespace.\n\nIn the case of an addition, the patch *shouldn't* contain trailing\nwhitespace anyway. If it did, git-apply would in its default\nconfiguration flag it as a whitespace error. So arguably, the MUA is\ndoing you a favor by stripping whitespace on such lines. :-)\n\n>  So just reject the patch when somebody sends you format=flowed\n>  and ask them to re-send without =flowed, and the world will be\n>  a much better place.\n\nI don't get it. Why not accommodate fascist RFC 3676 MUAs if we can for\nthe same reasons we accommodate QP, BASE64, and attachments instead of\ninline? I guess I can only think of two reasons:\n\n1) We need to accommodate QP, BASE64, etc since they may be due to the\n   MTA. By contrast, format=flowed is always an MUA issue.\n\n2) The other transformations are 100% safe in getting back exactly the\n   patch the user intended. format=flowed isn't. But per above, I'm not\n   sure the trailing-whitespace loss is a problem in practice.\n\nj.\n"},{"id":"68832","messageId":"76718490802151043q56340ea9i247dbb1601f8d225@mail.gmail.com","threadId":"12110","inReplyTo":"alpine.LSU.1.00.0802151035100.30505@racer.site","subject":"Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2008-02-15T18:43:03Z","receivedAt":"2008-02-15T18:43:03Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 15, 2008 at 5:41 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>  > +             char *cp = line;\n>  > +             while (*cp == '>' && *cp != 0)\n>  > +                     cp++;\n>\n>  How about using strchrnul()?\n\nActually strchrnul() here isn't correct. I want to strip leading '>'\nonly. strchrnul() will search for the first occurrence (skipping over\nnon-'>' to do so), which is not what I want.\n\n>  > +                     while (*cp != '\\n' && *cp !=0)\n>  > +                             cp++;\n>\n>  Again, this is the job for strchrnul().\n\nYep, here strchrnul would be fine.\n\nj.\n"},{"id":"68857","messageId":"alpine.LSU.1.00.0802160227180.30505@racer.site","threadId":"12110","inReplyTo":"76718490802151043q56340ea9i247dbb1601f8d225@mail.gmail.com","subject":"Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-16T02:30:35Z","receivedAt":"2008-02-16T02:30:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 15 Feb 2008, Jay Soffian wrote:\n\n> On Fri, Feb 15, 2008 at 5:41 AM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >  > +             char *cp = line;\n> >  > +             while (*cp == '>' && *cp != 0)\n> >  > +                     cp++;\n> >\n> >  How about using strchrnul()?\n> \n> Actually strchrnul() here isn't correct. I want to strip leading '>' \n> only. strchrnul() will search for the first occurrence (skipping over \n> non-'>' to do so), which is not what I want.\n\nOh, okay, I read again.\n\nYou just wanted to strip the leading \">\".  So I know even less what you \ntried to do.  But then, I do not know anything about format=flawed either.\n\nCiao,\nDscho\n"},{"id":"68877","messageId":"7vodahcrrl.fsf@gitster.siamese.dyndns.org","threadId":"12110","inReplyTo":"76718490802151037m87995c2gacf29667259eae41@mail.gmail.com","subject":"Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-16T06:57:50Z","receivedAt":"2008-02-16T06:57:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jay Soffian\" <jaysoffian@gmail.com> writes:\n\n> On Fri, Feb 15, 2008 at 12:10 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> I really do not like this.\n>\n>>  format=flowed instructs the receivers MUA that it is Ok to\n>>  reflow the text when the message is presented to the user.\n>\n> No, it indicates that the MUA sender mangled the message, but did so in\n> a way that it can be unmanged* as long as the receiving MUA implements\n> RFC 3676.\n>\n> * Trailing whitespace removed by the MUA before sending is the one\n>   exception that cannot be unmangled; I'll address this further below.\n\nThere is no way for you to address this other than brushing it\naside, saying \"it does not matter\".  The information lost is\ninformation lost.\n\n> Existing code generally shouldn't have trailing whitespace nor\n> whitespace only lines. ...\n\nSays who?\n\nWe and the kernel folks are probably one of the more whitespace\nstrict projects, but I am sure both of us get our fair share of\npatches that fix whitespace breakages while updating the\nsurrounding area.  There are existing breakages, and also there\nare lines that have deliberate trailing whitespaces.\n\nAlso, remember that this is not May 2005.  git is not about the\nkernel and git itself.  We passed that point long time ago.\nOther communities have different whitespace policies.\n\n> ... However, let's say that it does and that the\n> patch refers to one of these lines (either as context or a subtraction).\n> In this case that hunk will fail to apply, unless we teach git-apply to\n> be lenient if the only difference between the line in the patch and the\n> existing code is trailing whitespace.\n\nFailing to apply is not something I am worried about.\n\nSilently applying bogosity is much more problematic, because it\nwould be _silent_, and \"git am\" is often used to apply hundreds\nof patches in one go.  We need to be able to trust the tool and\nthe tool should error out when there is any ambiguity.\n\n> In the case of an addition, the patch *shouldn't* contain trailing\n> whitespace anyway. If it did, git-apply would in its default\n> configuration flag it as a whitespace error.\n\nIt just \"warns\".  It does not strip by default.  It should be a\nconscious user decision (done with \"am --whitespace=fix\") and\nthe project policy.\n\n>>  So just reject the patch when somebody sends you format=flowed\n>>  and ask them to re-send without =flowed, and the world will be\n>>  a much better place.\n>\n> I don't get it. Why not accommodate fascist RFC 3676 MUAs if we can for\n> the same reasons we accommodate QP, BASE64, and attachments instead of\n> inline?\n\nI need to ask people listening from the sidelines on the list\nhere.  Was my explanation why MIME attachments and QP/BASE64 are\n_fundamentally different_ from format=flowed insufficient?\n\nThe point is, format=flowed is _not_ meant for precise transfer\nof the content matter.  The design objective of format=flowed\nlies elsewhere.  It achieves nicer looking line-wrapping by\nsacrificing the precise transfer of the content.  It probably\ndoes a very good job, allowing you to communicate with pals on\ntext-email enabled phones, although I do not have one so I\ncannot judge ;-).\n\nAttachments, QP and BASE64 are all about getting the contents as\nintact as possible.  Because they tend to make reviews harder,\ngit and the kernel community frown upon them.  But our tools\ntolerate them, because attachment is a fine or even preferred\nway to transfer the patches in other circles.\n\nBut format=flowed is different.  It loses information.  Not just\none space at the end of the original line, but if you have very\nlong line that has more than one spaces at an unfortunate place,\nthe sending MUA can cut the line there and leave a single\ntrailing space for the receiving end to reflow.\n\nRFC3676 may be a good text communication medium.  It is just not\nsuitable for patch transfer.  Just don't use it.\n"},{"id":"68882","messageId":"76718490802152343g6a987c8ay80493187d0a3ccba@mail.gmail.com","threadId":"12110","inReplyTo":"7vodahcrrl.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2008-02-16T07:43:41Z","receivedAt":"2008-02-16T07:43:41Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Feb 16, 2008 1:57 AM, Junio C Hamano <gitster@pobox.com> wrote:\n\n> But format=flowed is different.  It loses information.  Not just\n> one space at the end of the original line, but if you have very\n> long line that has more than one spaces at an unfortunate place,\n> the sending MUA can cut the line there and leave a single\n> trailing space for the receiving end to reflow.\n\nWell, according to the RFC, that shouldn't be the case. The only\nlost information should be trailing whitespace. Embedded\nwhitespace should not be altered.\n\n> RFC3676 may be a good text communication medium.  It is just not\n> suitable for patch transfer.  Just don't use it.\n\nWould you consider making this configurable. Something like:\n\napplymail.allowed_flowed = true/false/warn\n\nIf you're completely opposed though, we should modify git-am\n(and/or mailinfo) to reject format=flowed messages entirely, no?\n\nj.\n"},{"id":"68892","messageId":"7vlk5l9q7z.fsf@gitster.siamese.dyndns.org","threadId":"12110","inReplyTo":"76718490802152343g6a987c8ay80493187d0a3ccba@mail.gmail.com","subject":"Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-16T09:59:28Z","receivedAt":"2008-02-16T09:59:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jay Soffian\" <jaysoffian@gmail.com> writes:\n\n> On Feb 16, 2008 1:57 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Well, according to the RFC, that shouldn't be the case. The only\n> lost information should be trailing whitespace. Embedded\n> whitespace should not be altered.\n\nUnfortunately, actual implementations and people's use matter\nmore.  I usually do not use graphical MUAs but when I tried to\npaste long lines to Thunderbird (or was it Evolution, I do not\nrecall), it behaved as if I was typing the words, folded them at\nconvenient inter word spaces.  I would not blindly trust what\nRFC says.\n\nThere also seem to be a strong correlation between people who\nallow their MUA to send format=flowed when sending patches and\npeople who cut and paste their patches to their MUA, corrupting\nwhitespaces.\n\n>> RFC3676 may be a good text communication medium.  It is just not\n>> suitable for patch transfer.  Just don't use it.\n>\n> Would you consider making this configurable. Something like:\n>\n> applymail.allowed_flowed = true/false/warn\n>\n> If you're completely opposed though, we should modify git-am\n> (and/or mailinfo) to reject format=flowed messages entirely, no?\n\nI would not even consider applying flowed message these days (my\nworkflow is to review in MUA, save perhaps worthy ones to a\nseparate mailbox and after re-reviewing apply), so they will not\nhit \"git am\" and it would not personally affect me.  Honestly, I\ndo not care very much either way.\n\nBut for some projects (perhaps the ones that do not value their\nsource as much as we do ;-)) accepting a subset of flowed\ncontents might be reasonable and even useful.  Maybe something\nlike this would be reasonably safe and still useful?\n\n - A format=flowed message that actually has a flowed line is\n   always rejected;\n\n - If there is no flowed line (i.e. lines that end with SP) in\n   the message, we unstuff the initial space to produce the\n   result (there won't be anything else that is funny, will\n   there?  In a patch we do not care much about the quotation of\n   discussions):\n \n   - If apply.whitespace is set to nowarn, we do not warn even\n     though we might have lost the trailing whitespaces.\n\n   - If apply.whitespace is set to warn, we warn about the\n     flowed message.\n\n   - If apply.whitespace is set to error or fix, we error out,\n     but still leaving the result for manual inspection.\n\nI dunno.\n\nBy the way, I do not think a solution that only uses\nconfiguration is usable.\n\nWhen you have to apply many patches, you set your configuration\nto the most strict (i.e. apply.whitespace=error), and when the\nprocessing errors out, you then inspect the situation and\nmanually override with the command line --whitespace=fix (or\n\"warn\") to process that one message.  You need both.\n\nSince mailinfo now has only a very few users (quiltimport and\n\"am\"), we probably could add --whitespace option to mailinfo,\nand teach \"am\" to pass --whitespace command line option it was\ngiven (otherwise the value from apply.whitespace configuration)\nwhen running mailinfo, if we were to do a \"safer subset\" as\noutlined above.\n"},{"id":"68902","messageId":"20080216143444.GU2456@cisco.com","threadId":"12110","inReplyTo":"7vr6fei1s4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages","fromName":"Derek Fawcus","fromEmail":"dfawcus@cisco.com","sentAt":"2008-02-16T14:34:44Z","receivedAt":"2008-02-16T14:34:44Z","isPatch":true,"sender":{"key":"dfawcus@cisco.com","avatar":null},"body":"On Fri, Feb 15, 2008 at 09:10:19AM -0800, Junio C Hamano wrote:\n> I really do not like this.\n> \n> format=flowed instructs the receivers MUA that it is Ok to\n> reflow the text when the message is presented to the user.  That\n> is exactly what we do _NOT_ want to happen to patches.  Your\n> implementation may cleanly salvage what the sender intended to\n> send, but salvaging when applying the patch is too late.\n> \n> You review the patch, decide to apply or reject, and then\n> finally apply.  Unmangling of corrupt contents should be done\n> before you review, not before you apply.\n\nI have to agree that this is a bad idea - for the reasons stated,\nand one additional:\n\nThere are MUA's which send 'format=fixed' email marked as 'format=flowed'.\n\nAt which point the receiving MUA performing the deflowing _will_ get\na corrupt patch.\n\nI saw this recently wrt to internal company code reviews.  We have\na variety of different MUAs in use on a variety of different OSs.\n\nSome 'properly' implement format=flowed,  some do not.   Some always\nsend format=flowed for text,  some can have it disabled to send\nformat=fixed.  Despite what the RFCs say,  some of them also QP\nor base64 encode such flowed pieces of text.  It is a nightmare.\n\nHowever it seems thay all use fixed format for attachments,  unfortunately\nsome them always tag those as application/octet-stream,  so one cannot\nautomatically process them (in the absense of other clues).\n\nI am able to work around some of these issues by configuing mutt to\ndo some guesses when it sees application/x-patch,  or application/octet-stream\nand a file with say as '.diff' extension.\n\nBut ultimately for some of these one is forced to contact the original\nauthor and get access to the actual patch file on the filesystem.\nWhile this can be done for a 'small' community,  it is not something\nthat would work in the wider world.\n\nSo - format=flowed is simply unsuitable for fixed format text and should\nnot be used.\n\n[Actually it rather turns out format=flowed is a bad idea all together,\n you should see what it does to log files...]\n\nDF\n"}]}