{"thread":{"id":"25859","subject":"[PATCH v4 0/4] add --recode-patch parameter to mailinfo and am","startedAt":"2010-11-28T19:10:13Z","lastAt":"2011-04-16T06:22:58Z","messageCount":9,"participants":["ZHANG, Le","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":4},"messages":[{"id":"156759","messageId":"1290971417-4474-1-git-send-email-r0bertz@gentoo.org","threadId":"25859","inReplyTo":null,"subject":"[PATCH v4 0/4] add --recode-patch parameter to mailinfo and am","fromName":"ZHANG, Le","fromEmail":"r0bertz@gentoo.org","sentAt":"2010-11-28T19:10:13Z","receivedAt":"2010-11-28T19:10:13Z","isPatch":true,"sender":{"key":"r0bertz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/29025?v=4"},"body":"I have a translation project which uses UTF-8 as charset.\nSo the patch must be encoded in UTF-8, not just the commit msg etc.\nAnd we use google group as our mailing list.\n\nRecently, mails saved from gmail are encoded using local charset if all the\ncharacters in the patch are in that specific local charset even if the orignal\nmail is in UTF-8. This seems smart but it caused inconvenience for\nour project.\n\nSince we have no control on what google will do, so I took another way,\ni.e. add this option to git-mailinfo. I hope this could benefit others as\nwell.\n\nChangelog:\n\nv4 -> v3:\n* Added a target_charset parameter to convert_to_utf8() in mailinfo.c.\n* Introduced a new config varible: i18n.patchencoding, which will be used solely\n  by --recode-patch parameter.\n\nv2 -> v3:\n* Removed 'const' type qualifier from handle_patch()'s parameter\n* Fixed typos in commit msg\n\nv1 -> v2:\n* Clarified how -u/--encoding is handled in git-mailinfo's documentation\n\n\nZHANG, Le (4):\n  mailinfo.c: convert_to_utf8(): added a target_charset parameter\n  i18n.patchencoding: introduce a new config variable\n  git mailinfo: added a --recode-patch parameter\n  git am: added a --recode-patch parameter\n\n Documentation/git-am.txt       |    4 ++++\n Documentation/git-mailinfo.txt |    6 +++++-\n builtin/mailinfo.c             |   27 +++++++++++++++++----------\n cache.h                        |    1 +\n config.c                       |    3 +++\n environment.c                  |    1 +\n git-am.sh                      |   13 +++++++++++--\n 7 files changed, 42 insertions(+), 13 deletions(-)\n\n-- \n1.7.3.2.344.gb3680.dirty\n"},{"id":"156760","messageId":"1290971417-4474-2-git-send-email-r0bertz@gentoo.org","threadId":"25859","inReplyTo":"1290971417-4474-1-git-send-email-r0bertz@gentoo.org","subject":"[PATCH v4 1/4] mailinfo.c: convert_to_utf8(): added a target_charset parameter","fromName":"ZHANG, Le","fromEmail":"r0bertz@gentoo.org","sentAt":"2010-11-28T19:10:14Z","receivedAt":"2010-11-28T19:10:14Z","isPatch":true,"sender":{"key":"r0bertz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/29025?v=4"},"body":"This is required for my recode-patch patch which needs a seperate patch_charset variable.\n\nSigned-off-by: ZHANG, Le <r0bertz@gentoo.org>\n---\n builtin/mailinfo.c |   16 ++++++++--------\n 1 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\nindex 2320d98..1406d9f 100644\n--- a/builtin/mailinfo.c\n+++ b/builtin/mailinfo.c\n@@ -492,22 +492,22 @@ static const char *guess_charset(const struct strbuf *line, const char *target_c\n \treturn \"ISO8859-1\";\n }\n \n-static void convert_to_utf8(struct strbuf *line, const char *charset)\n+static void convert_to_utf8(struct strbuf *line, const char *charset, const char *target_charset)\n {\n \tchar *out;\n \n \tif (!charset || !*charset) {\n-\t\tcharset = guess_charset(line, metainfo_charset);\n+\t\tcharset = guess_charset(line, target_charset);\n \t\tif (!charset)\n \t\t\treturn;\n \t}\n \n-\tif (!strcasecmp(metainfo_charset, charset))\n+\tif (!strcasecmp(target_charset, charset))\n \t\treturn;\n-\tout = reencode_string(line->buf, metainfo_charset, charset);\n+\tout = reencode_string(line->buf, target_charset, charset);\n \tif (!out)\n \t\tdie(\"cannot convert from %s to %s\",\n-\t\t    charset, metainfo_charset);\n+\t\t    charset, target_charset);\n \tstrbuf_attach(line, out, strlen(out), strlen(out));\n }\n \n@@ -577,7 +577,7 @@ static int decode_header_bq(struct strbuf *it)\n \t\t\tbreak;\n \t\t}\n \t\tif (metainfo_charset)\n-\t\t\tconvert_to_utf8(dec, charset_q.buf);\n+\t\t\tconvert_to_utf8(dec, charset_q.buf, metainfo_charset);\n \n \t\tstrbuf_addbuf(&outbuf, dec);\n \t\tstrbuf_release(dec);\n@@ -602,7 +602,7 @@ static void decode_header(struct strbuf *it)\n \t * This can be binary guck but there is no charset specified.\n \t */\n \tif (metainfo_charset)\n-\t\tconvert_to_utf8(it, \"\");\n+\t\tconvert_to_utf8(it, \"\", metainfo_charset);\n }\n \n static void decode_transfer_encoding(struct strbuf *line)\n@@ -796,7 +796,7 @@ static int handle_commit_msg(struct strbuf *line)\n \n \t/* normalize the log message to UTF-8. */\n \tif (metainfo_charset)\n-\t\tconvert_to_utf8(line, charset.buf);\n+\t\tconvert_to_utf8(line, charset.buf, metainfo_charset);\n \n \tif (use_scissors && is_scissors_line(line)) {\n \t\tint i;\n-- \n1.7.3.2.344.gb3680.dirty\n"},{"id":"156761","messageId":"1290971417-4474-3-git-send-email-r0bertz@gentoo.org","threadId":"25859","inReplyTo":"1290971417-4474-1-git-send-email-r0bertz@gentoo.org","subject":"[PATCH v4 2/4] i18n.patchencoding: introduce a new config variable","fromName":"ZHANG, Le","fromEmail":"r0bertz@gentoo.org","sentAt":"2010-11-28T19:10:15Z","receivedAt":"2010-11-28T19:10:15Z","isPatch":true,"sender":{"key":"r0bertz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/29025?v=4"},"body":"This varible will be used by git mailinfo's --recode-patch parameter only.\n\nSigned-off-by: ZHANG, Le <r0bertz@gentoo.org>\n---\n cache.h       |    1 +\n config.c      |    3 +++\n environment.c |    1 +\n 3 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 33decd9..d04aeff 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1015,6 +1015,7 @@ extern int user_ident_explicitly_given;\n extern int user_ident_sufficiently_given(void);\n \n extern const char *git_commit_encoding;\n+extern const char *git_patch_encoding;\n extern const char *git_log_output_encoding;\n extern const char *git_mailmap_file;\n \ndiff --git a/config.c b/config.c\nindex c63d683..14b0f92 100644\n--- a/config.c\n+++ b/config.c\n@@ -674,6 +674,9 @@ static int git_default_i18n_config(const char *var, const char *value)\n \tif (!strcmp(var, \"i18n.commitencoding\"))\n \t\treturn git_config_string(&git_commit_encoding, var, value);\n \n+\tif (!strcmp(var, \"i18n.patchencoding\"))\n+\t\treturn git_config_string(&git_patch_encoding, var, value);\n+\n \tif (!strcmp(var, \"i18n.logoutputencoding\"))\n \t\treturn git_config_string(&git_log_output_encoding, var, value);\n \ndiff --git a/environment.c b/environment.c\nindex de5581f..b2870f4 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -23,6 +23,7 @@ int log_all_ref_updates = -1; /* unspecified */\n int warn_ambiguous_refs = 1;\n int repository_format_version;\n const char *git_commit_encoding;\n+const char *git_patch_encoding;\n const char *git_log_output_encoding;\n int shared_repository = PERM_UMASK;\n const char *apply_default_whitespace;\n-- \n1.7.3.2.344.gb3680.dirty\n"},{"id":"156762","messageId":"1290971417-4474-4-git-send-email-r0bertz@gentoo.org","threadId":"25859","inReplyTo":"1290971417-4474-1-git-send-email-r0bertz@gentoo.org","subject":"[PATCH v4 3/4] git mailinfo: added a --recode-patch parameter","fromName":"ZHANG, Le","fromEmail":"r0bertz@gentoo.org","sentAt":"2010-11-28T19:10:16Z","receivedAt":"2010-11-28T19:10:16Z","isPatch":true,"sender":{"key":"r0bertz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/29025?v=4"},"body":"When this parameter is specified, patch will be converted to a target encoding before applied.\nThe target encoding defaults to UTF-8. It could also be specified by i18n.patchencoding.\n\nSigned-off-by: ZHANG, Le <r0bertz@gentoo.org>\n---\n Documentation/git-mailinfo.txt |    6 +++++-\n builtin/mailinfo.c             |   11 +++++++++--\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-mailinfo.txt b/Documentation/git-mailinfo.txt\nindex 3ea5aad..3e84817 100644\n--- a/Documentation/git-mailinfo.txt\n+++ b/Documentation/git-mailinfo.txt\n@@ -45,7 +45,7 @@ OPTIONS\n \tthem.  This used to be optional but now it is the default.\n +\n Note that the patch is always used as-is without charset\n-conversion, even with this flag.\n+conversion, even with this flag; use '--recode-patch' for that.\n \n --encoding=<encoding>::\n \tSimilar to -u.  But when re-coding, the charset specified here is\n@@ -54,6 +54,10 @@ conversion, even with this flag.\n -n::\n \tDisable all charset re-coding of the metadata.\n \n+--recode-patch::\n+\tConvert the patch from the e-mail to UTF-8 (or the value of the\n+\tconfiguration variable i18n.patchencoding if it is set).\n+\n --scissors::\n \tRemove everything in body before a scissors line.  A line that\n \tmainly consists of scissors (either \">8\" or \"8<\") and perforation\ndiff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\nindex 1406d9f..96181e6 100644\n--- a/builtin/mailinfo.c\n+++ b/builtin/mailinfo.c\n@@ -12,6 +12,8 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;\n static int keep_subject;\n static int keep_non_patch_brackets_in_subject;\n static const char *metainfo_charset;\n+static const char *patch_charset;\n+static int recode_patch;\n static struct strbuf line = STRBUF_INIT;\n static struct strbuf name = STRBUF_INIT;\n static struct strbuf email = STRBUF_INIT;\n@@ -828,8 +830,10 @@ static int handle_commit_msg(struct strbuf *line)\n \treturn 0;\n }\n \n-static void handle_patch(const struct strbuf *line)\n+static void handle_patch(struct strbuf *line)\n {\n+\tif (recode_patch)\n+\t\tconvert_to_utf8(line, charset.buf, patch_charset);\n \tfwrite(line->buf, 1, line->len, patchfile);\n \tpatch_lines++;\n }\n@@ -1021,7 +1025,7 @@ static int git_mailinfo_config(const char *var, const char *value, void *unused)\n }\n \n static const char mailinfo_usage[] =\n-\t\"git mailinfo [-k|-b] [-u | --encoding=<encoding> | -n] [--scissors | --no-scissors] msg patch < mail >info\";\n+\t\"git mailinfo [-k|-b] [-u | --encoding=<encoding> | -n] [--recode-patch] [--scissors | --no-scissors] msg patch < mail >info\";\n \n int cmd_mailinfo(int argc, const char **argv, const char *prefix)\n {\n@@ -1034,6 +1038,7 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)\n \n \tdef_charset = (git_commit_encoding ? git_commit_encoding : \"UTF-8\");\n \tmetainfo_charset = def_charset;\n+\tpatch_charset = git_patch_encoding ? git_patch_encoding : \"UTF-8\";\n \n \twhile (1 < argc && argv[1][0] == '-') {\n \t\tif (!strcmp(argv[1], \"-k\"))\n@@ -1046,6 +1051,8 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)\n \t\t\tmetainfo_charset = NULL;\n \t\telse if (!prefixcmp(argv[1], \"--encoding=\"))\n \t\t\tmetainfo_charset = argv[1] + 11;\n+\t\telse if (!prefixcmp(argv[1], \"--recode-patch\"))\n+\t\t\trecode_patch = 1;\n \t\telse if (!strcmp(argv[1], \"--scissors\"))\n \t\t\tuse_scissors = 1;\n \t\telse if (!strcmp(argv[1], \"--no-scissors\"))\n-- \n1.7.3.2.344.gb3680.dirty\n"},{"id":"156763","messageId":"1290971417-4474-5-git-send-email-r0bertz@gentoo.org","threadId":"25859","inReplyTo":"1290971417-4474-1-git-send-email-r0bertz@gentoo.org","subject":"[PATCH v4 4/4] git am: added a --recode-patch parameter","fromName":"ZHANG, Le","fromEmail":"r0bertz@gentoo.org","sentAt":"2010-11-28T19:10:17Z","receivedAt":"2010-11-28T19:10:17Z","isPatch":true,"sender":{"key":"r0bertz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/29025?v=4"},"body":"When this parameter is specified, git am will pass this parameter to git mailinfo.\n\nSigned-off-by: ZHANG, Le <r0bertz@gentoo.org>\n---\n Documentation/git-am.txt |    4 ++++\n git-am.sh                |   13 +++++++++++--\n 2 files changed, 15 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex 51297d0..24ba5ec 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -73,6 +73,10 @@ default.   You can use `--no-utf8` to override this.\n \tPass `-n` flag to 'git mailinfo' (see\n \tlinkgit:git-mailinfo[1]).\n \n+--recode-patch::\n+\tPass `--recode-patch` flag to 'git mailinfo' (see\n+\tlinkgit:git-mailinfo[1]).\n+\n -3::\n --3way::\n \tWhen the patch does not apply cleanly, fall back on\ndiff --git a/git-am.sh b/git-am.sh\nindex df09b42..8010119 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -14,6 +14,7 @@ b,binary*       (historical option -- no-op)\n q,quiet         be quiet\n s,signoff       add a Signed-off-by line to the commit message\n u,utf8          recode into utf8 (default)\n+recode-patch    pass --recode-patch flag to git-mailinfo\n k,keep          pass -k flag to git-mailinfo\n keep-cr         pass --keep-cr flag to git-mailsplit for mbox format\n no-keep-cr      do not pass --keep-cr flag to git-mailsplit independent of am.keepcr\n@@ -295,7 +296,7 @@ split_patches () {\n prec=4\n dotest=\"$GIT_DIR/rebase-apply\"\n sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=\n-resolvemsg= resume= scissors= no_inbody_headers=\n+resolvemsg= resume= scissors= no_inbody_headers= recode_patch=\n git_apply_opt=\n committer_date_is_author_date=\n ignore_date=\n@@ -321,6 +322,8 @@ do\n \t\tutf8=t ;; # this is now default\n \t--no-utf8)\n \t\tutf8= ;;\n+\t--recode-patch)\n+\t\trecode_patch=t ;;\n \t-k|--keep)\n \t\tkeep=t ;;\n \t-c|--scissors)\n@@ -464,6 +467,7 @@ else\n \techo \"$threeway\" >\"$dotest/threeway\"\n \techo \"$sign\" >\"$dotest/sign\"\n \techo \"$utf8\" >\"$dotest/utf8\"\n+\techo \"$recode_patch\" >\"$dotest/recode_patch\"\n \techo \"$keep\" >\"$dotest/keep\"\n \techo \"$keepcr\" >\"$dotest/keepcr\"\n \techo \"$scissors\" >\"$dotest/scissors\"\n@@ -505,6 +509,10 @@ then\n else\n \tutf8=-n\n fi\n+if test \"$(cat \"$dotest/recode_patch\")\" = t\n+then\n+\trecodepatch=--recode-patch\n+fi\n if test \"$(cat \"$dotest/keep\")\" = t\n then\n \tkeep=-k\n@@ -581,7 +589,8 @@ do\n \t# by the user, or the user can tell us to do so by --resolved flag.\n \tcase \"$resume\" in\n \t'')\n-\t\tgit mailinfo $keep $no_inbody_headers $scissors $utf8 \"$dotest/msg\" \"$dotest/patch\" \\\n+\t\tgit mailinfo $keep $no_inbody_headers $scissors $utf8 \\\n+\t\t\t$recodepatch \"$dotest/msg\" \"$dotest/patch\" \\\n \t\t\t<\"$dotest/$msgnum\" >\"$dotest/info\" ||\n \t\t\tstop_here $this\n \n-- \n1.7.3.2.344.gb3680.dirty\n"},{"id":"156833","messageId":"7vsjyj3mmr.fsf@alter.siamese.dyndns.org","threadId":"25859","inReplyTo":"1290971417-4474-2-git-send-email-r0bertz@gentoo.org","subject":"Re: [PATCH v4 1/4] mailinfo.c: convert_to_utf8(): added a target_charset parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-29T20:23:08Z","receivedAt":"2010-11-29T20:23:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZHANG, Le\" <r0bertz@gentoo.org> writes:\n\n> This is required for my recode-patch patch which needs a seperate patch_charset variable.\n>\n> Signed-off-by: ZHANG, Le <r0bertz@gentoo.org>\n> ---\n\nThanks.\n\nPlease describe what the new parameter means.  Is it used to convert the\ncontents in \"line\" from \"charset\" to \"target_charset\"?  Perhaps it is a\ngood time to rename the function to \"convert_to()\", its \"charset\"\nparameter to \"from_charset\", and name the new parameter \"to_charset\", in\norder to reduce confusion?\n\nIt is not just _your_ patch but will help other/later patches, so you may\nwant to phrase the proposed commit log message a bit differently (and with\na narrower page width, like 68-74 chars per line).  Perhaps like...\n\n    mailinfo.c: Allow convert_to_utf8() to specify both src/dst charset\n\n    The convert_to_utf8() function actually converts to whatever charset\n    \"metainfo_charset\" variable contains, which is not necessarily UTF-8.\n    Rename it to convert_to(), and give an extra parameter \"to_charset\" to\n    specify what charset to re-encode to.  Also rename its \"charset\"\n    parameter to \"from_charset\" to clarify which is which.\n"},{"id":"156834","messageId":"7vlj4b3mme.fsf@alter.siamese.dyndns.org","threadId":"25859","inReplyTo":"1290971417-4474-3-git-send-email-r0bertz@gentoo.org","subject":"Re: [PATCH v4 2/4] i18n.patchencoding: introduce a new config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-29T20:23:21Z","receivedAt":"2010-11-29T20:23:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZHANG, Le\" <r0bertz@gentoo.org> writes:\n\n> This varible will be used by git mailinfo's --recode-patch parameter only.\n\nI have a few complaints and observations about this:\n\n - The patch order is screwed up in the series.  Without knowing what\n   the --recode-patch option does, the reader is forced to look-ahead\n   before judging this patch.\n\n - No documentation in the same patch as the feature is added.  I am\n   guessing that the new configuration variable (and the new option we\n   will see laster) means \"the patchfile I got is in this encoding but the\n   mail header does not mark it as such, so I am giving what encoding it\n   is\", but this forces the reader to look-ahead.\n\n - \"It will be used by ... only\", says who?  In an environment where\n   people send patches in a local encoding but want to keep their\n   repository in a different encoding, it may not be totally implausible\n   to wish \"format-patch\" to pay attention to this variable to _produce_\n   the output in that encoding, especially given the name of the variable\n   that does not say anything about in which direction it is used, no?\n\n - Assuming that I guessed the meaning of this option and parameter right,\n   I am not sure if this should be a configuration variable.  It implies\n   that the majority of patches, if not all, are in this single local\n   encoding that is different from the encoding used in the repository.\n   Is it common?  I dunno.\n"},{"id":"156836","messageId":"7veia33mlu.fsf@alter.siamese.dyndns.org","threadId":"25859","inReplyTo":"1290971417-4474-4-git-send-email-r0bertz@gentoo.org","subject":"Re: [PATCH v4 3/4] git mailinfo: added a --recode-patch parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-29T20:23:41Z","receivedAt":"2010-11-29T20:23:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZHANG, Le\" <r0bertz@gentoo.org> writes:\n\n> When this parameter is specified, patch will be converted to a target\n> encoding before applied.  The target encoding defaults to UTF-8. It\n> could also be specified by i18n.patchencoding.\n>\n> Signed-off-by: ZHANG, Le <r0bertz@gentoo.org>\n> ---\n\nAh, please forget (most of) what I said in my review of 2/4.  This series\nis not about incoming e-mails that misidentify their content encoding, but\nis about accepting patches from e-mails that are in different encoding and\ncorrectly says which encoding they are in.  We make mailinfo convert the\npayload to UTF-8 or \"i18n.patchencoding\".\n\nThe above is a clear indication that \"i18n.patchencoding\" is grossly\nmisnamed.  It sounds as if it is about the encoding of patches, and\nbecause we are discussing \"mailinfo\", I mistook that is about the encoding\nused in the incoming patches.\n\nBut it is not.\n\nIt is about encoding of blobs and paths used in the repository.  With your\nseries, the mailinfo codepath that deals with incoming patches happens to\nbecome the first user to pay attention to the repository data encoding,\nbut that is not a good reason to name it \"i18n.patchencoding\".\n\nIn the longer run, it is entirely plausible that we will want to know\nabout different encodings used for these things:\n\n - encoding in which blobs and tree paths are stored in the repository.\n   This is what you called \"i18n.patchencoding\".\n\n - encoding in which the log messages of the commits are stored in the\n   repository.  We have \"i18n.commitencoding\" variable for that.\n\n - encoding in which incoming patches are given to \"mailinfo\".  This is\n   read from the e-mail message.\n\n - encoding in which blobs are checked out to the working tree.  In a\n   distant future, we may want to re-encode blobs from the repository\n   encoding to this encoding while checking out, and the other way while\n   checking in.\n\n - encoding in which readdir(3) returns paths from the working tree.  In a\n   distant future, we may want to re-encode paths from the repository\n   encoding to this encoding while checking out, and the other way while\n   checking in.\n\nI think as a short-term solution to your immediate issue, your series may\nbe good enough, but if we are to go this route, we really should make it\n\"the repository encoding\".  And the variable that replaces your\n\"patch_charset\" should be declared in cache.h, defined in environment.c,\nparsed by git_default_config(), and be accessible to everybody.\n\nAn alternative is to use i18n.commitencoding for that purpose, but that\nwill cause issues with existing repositories---they have been happy\nwithout us recoding the payload, and they will get upset if we suddenly\nstart doing so.  So I'd say that a new \"i18n.repositoryencoding\"\nconfiguration variable and repository_encoding variable would be a better\ndesign in the longer term.  They\n\n (1) default to \"binary\" (or \"bytestring\", \"literal\", \"verbatim\",\n     whatever) to tell us _not_ to do any encoding conversion to the\n     contents when accepting patches, generating patches, checking out,\n     checking in, or running git-archive; and\n\n (2) when set, certain operations will pay attention to it; in your\n     situation, \"mailinfo\" will convert incoming e-mails to its value\n     (presumably \"UTF-8\").\n\nYet even longer term, we probably would need to make the blob encoding\ninto an attribute to the path (e.g. think about an i18n documentation\nproject, where different translations may need to be stored in different\nencodings).\n\nThis will open another big can of worms, though.  Unless the incoming\ne-mail is split into a MIME multipart that contains one-patch per file\nwith each part in different encoding, a single plaintext message needs to\nbe in a single encoding, so your mailinfo patch would most likely need to\nencode to one canonical encoding (i.e. \"UTF-8\"), and then reencode to per\npath encoding when feeding the patch.  Then there is another issue of what\nto do with the tree paths that appear in \"diff --git\" and \"rename from ...\"\nheaders which most likely to be a single canonical encoding in the project.\n\nBut we need to start from somewhere, so let's make the first step \"a\nsingle project wide blob and tree path encoding\".\n\nOpinions?\n"},{"id":"165952","messageId":"20110416062255.GC18591@adriano.hsd1.ca.comcast.net","threadId":"25859","inReplyTo":"7vsjyj3mmr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 1/4] mailinfo.c: convert_to_utf8(): added a target_charset parameter","fromName":"ZHANG, Le","fromEmail":"r0bertz@gentoo.org","sentAt":"2011-04-16T06:22:58Z","receivedAt":"2011-04-16T06:22:58Z","isPatch":true,"sender":{"key":"r0bertz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/29025?v=4"},"body":"On 12:23 Mon 29 Nov     , Junio C Hamano wrote:\n> \"ZHANG, Le\" <r0bertz@gentoo.org> writes:\n> \n> > This is required for my recode-patch patch which needs a seperate patch_charset variable.\n> >\n> > Signed-off-by: ZHANG, Le <r0bertz@gentoo.org>\n> > ---\n> \n> Thanks.\n> \n> Please describe what the new parameter means.  Is it used to convert the\n> contents in \"line\" from \"charset\" to \"target_charset\"?  Perhaps it is a\n> good time to rename the function to \"convert_to()\", its \"charset\"\n> parameter to \"from_charset\", and name the new parameter \"to_charset\", in\n> order to reduce confusion?\n> \n> It is not just _your_ patch but will help other/later patches, so you may\n> want to phrase the proposed commit log message a bit differently (and with\n> a narrower page width, like 68-74 chars per line).  Perhaps like...\n> \n>     mailinfo.c: Allow convert_to_utf8() to specify both src/dst charset\n> \n>     The convert_to_utf8() function actually converts to whatever charset\n>     \"metainfo_charset\" variable contains, which is not necessarily UTF-8.\n>     Rename it to convert_to(), and give an extra parameter \"to_charset\" to\n>     specify what charset to re-encode to.  Also rename its \"charset\"\n>     parameter to \"from_charset\" to clarify which is which.\n\nThank you for reviewing! And sorry for late response.\n\nFor this problem only, I'd like to propose a slightly better approach based on\nyour suggestion.\n\nCurrently, in mailinfo.c, there are 3 calls to convert_to_utf8(). Two of calls\nspecifies from_charset, the third doesn't. For those two calls which already\nspecifies from_charset, there is no need to guess from_charset. So I make two\nconvert functions. One is guess_and_convert_to(), the other is convert_to().\nThe next version of repository_encoding patch[1] will use convert_to().\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/162345/focus=162424\n\nHere is the patch:\n\ndiff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\nindex 71e6262..0f42ff1 100644\n--- a/builtin/mailinfo.c\n+++ b/builtin/mailinfo.c\n@@ -472,6 +472,20 @@ static struct strbuf *decode_b_segment(const struct strbuf *b_seg)\n \treturn out;\n }\n \n+static void convert_to(struct strbuf *line, const char *to_charset, const char *from_charset)\n+{\n+\tchar *out;\n+\n+\tif (!strcasecmp(to_charset, from_charset))\n+\t\treturn;\n+\n+\tout = reencode_string(line->buf, to_charset, from_charset);\n+\tif (!out)\n+\t\tdie(\"cannot convert from %s to %s\",\n+\t\t    from_charset, to_charset);\n+\tstrbuf_attach(line, out, strlen(out), strlen(out));\n+}\n+\n /*\n  * When there is no known charset, guess.\n  *\n@@ -483,32 +497,14 @@ static struct strbuf *decode_b_segment(const struct strbuf *b_seg)\n  * Otherwise, we default to assuming it is Latin1 for historical\n  * reasons.\n  */\n-static const char *guess_charset(const struct strbuf *line, const char *target_charset)\n+static void guess_and_convert_to(struct strbuf *line, const char *to_charset)\n {\n-\tif (is_encoding_utf8(target_charset)) {\n+\tif (is_encoding_utf8(to_charset)) {\n \t\tif (is_utf8(line->buf))\n-\t\t\treturn NULL;\n-\t}\n-\treturn \"ISO8859-1\";\n-}\n-\n-static void convert_to_utf8(struct strbuf *line, const char *charset)\n-{\n-\tchar *out;\n-\n-\tif (!charset || !*charset) {\n-\t\tcharset = guess_charset(line, metainfo_charset);\n-\t\tif (!charset)\n \t\t\treturn;\n \t}\n \n-\tif (!strcasecmp(metainfo_charset, charset))\n-\t\treturn;\n-\tout = reencode_string(line->buf, metainfo_charset, charset);\n-\tif (!out)\n-\t\tdie(\"cannot convert from %s to %s\",\n-\t\t    charset, metainfo_charset);\n-\tstrbuf_attach(line, out, strlen(out), strlen(out));\n+    convert_to(line, to_charset, \"ISO8859-1\");\n }\n \n static int decode_header_bq(struct strbuf *it)\n@@ -577,7 +573,7 @@ static int decode_header_bq(struct strbuf *it)\n \t\t\tbreak;\n \t\t}\n \t\tif (metainfo_charset)\n-\t\t\tconvert_to_utf8(dec, charset_q.buf);\n+\t\t\tconvert_to(dec, metainfo_charset, charset_q.buf);\n \n \t\tstrbuf_addbuf(&outbuf, dec);\n \t\tstrbuf_release(dec);\n@@ -602,7 +598,7 @@ static void decode_header(struct strbuf *it)\n \t * This can be binary guck but there is no charset specified.\n \t */\n \tif (metainfo_charset)\n-\t\tconvert_to_utf8(it, \"\");\n+\t\tguess_and_convert_to(it, metainfo_charset);\n }\n \n static void decode_transfer_encoding(struct strbuf *line)\n@@ -796,7 +792,7 @@ static int handle_commit_msg(struct strbuf *line)\n \n \t/* normalize the log message to UTF-8. */\n \tif (metainfo_charset)\n-\t\tconvert_to_utf8(line, charset.buf);\n+\t\tconvert_to(line, metainfo_charset, charset.buf);\n \n \tif (use_scissors && is_scissors_line(line)) {\n \t\tint i;\n\n\n-- \nZHANG, Le\nhttp://zhangle.co\n0260 C902 B8F8 6506 6586 2B90 BC51 C808 1E4E 2973\n"}]}