{"thread":{"id":"6097","subject":"Re: i18n.logToUTF8: convert commit log message to UTF-8","startedAt":"2006-12-24T14:18:47Z","lastAt":"2006-12-25T22:54:47Z","messageCount":6,"participants":["Johannes Schindelin","Junio C Hamano","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"30240","messageId":"Pine.LNX.4.63.0612241505290.19693@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6097","inReplyTo":null,"subject":"Re: i18n.logToUTF8: convert commit log message to UTF-8","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-24T14:18:47Z","receivedAt":"2006-12-24T14:18:47Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nI don't like the convention to add the encoding after a trailing NUL. I'd \nrather have it as an \"encoding blabl\" header like the author and committer \nheaders. I.e.\n\n-- snip --\ntree 519fba5ec35f25cbac7f46574f214fb5eb95d2c8\nparent 41c7c86ccbcb846cacf48c5e283983fa797cf37b\nauthor Johannes Schindelin <Johannes.Schindelin@gmx.de> 1166969650 +0100\ncommitter Johannes Schindelin <Johannes.Schindelin@gmx.de> 1166969650 +0100\nencoding latin1\n\n    git-commit-tree: add encoding header if commitEncoding != utf-8\n-- snap --\n\nOur log routines all grok these commit objects.\n\nAnd I would prefer commit-tree to fail if we explicitely ask for \nconversion to UTF-8, but NO_ICONV is set, or reencoding fails for other \nreasons.\n\nCiao,\nDscho\n"},{"id":"30250","messageId":"Pine.LNX.4.63.0612241643440.19693@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6097","inReplyTo":"Pine.LNX.4.63.0612241505290.19693@wbgn013.biozentrum.uni-wuerzburg.de","subject":"[PATCH] commit encoding: store it in commit header rather than mucking with NUL","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-24T15:44:50Z","receivedAt":"2006-12-24T15:44:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIt also fixes a segmentation fault when iconv does not know the\nencoding (defaulting to no conversion).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tOn Sun, 24 Dec 2006, Johannes Schindelin wrote:\n\n\t> I don't like the convention to add the encoding after a trailing \n\t> NUL. I'd rather have it as an \"encoding blabl\" header like the \n\t> author and committer headers.\n\n\tThis is a patch implementing that, on top of pu.\n\n builtin-commit-tree.c |   20 ++++++-------\n commit.c              |   70 ++++++++++++++++++++----------------------------\n 2 files changed, 38 insertions(+), 52 deletions(-)\n\ndiff --git a/builtin-commit-tree.c b/builtin-commit-tree.c\nindex 9aff980..33c29f7 100644\n--- a/builtin-commit-tree.c\n+++ b/builtin-commit-tree.c\n@@ -92,6 +92,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)\n \tchar comment[1000];\n \tchar *buffer;\n \tunsigned int size;\n+\tint encoding_is_utf8;\n \n \tsetup_ident();\n \tgit_config(git_default_config);\n@@ -117,6 +118,8 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)\n \t\t\tparents++;\n \t}\n \n+\tencoding_is_utf8 = !strcmp(git_commit_encoding, \"utf-8\");\n+\n \tinit_buffer(&buffer, &size);\n \tadd_buffer(&buffer, &size, \"tree %s\\n\", sha1_to_hex(tree_sha1));\n \n@@ -130,7 +133,11 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)\n \n \t/* Person/date information */\n \tadd_buffer(&buffer, &size, \"author %s\\n\", git_author_info(1));\n-\tadd_buffer(&buffer, &size, \"committer %s\\n\\n\", git_committer_info(1));\n+\tadd_buffer(&buffer, &size, \"committer %s\\n\", git_committer_info(1));\n+\tif (!encoding_is_utf8)\n+\t\tadd_buffer(&buffer, &size,\n+\t\t\t\t\"encoding %s\\n\", git_commit_encoding);\n+\tadd_buffer(&buffer, &size, \"\\n\");\n \n \t/* And add the comment */\n \twhile (fgets(comment, sizeof(comment), stdin) != NULL)\n@@ -138,16 +145,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)\n \n \t/* And check the encoding */\n \tbuffer[size] = '\\0';\n-\tif (strcmp(git_commit_encoding, \"utf-8\")) {\n-\t\t/*\n-\t\t * Add trailing section: this is safe because\n-\t\t * add_buffer always allocates one byte more.\n-\t\t */\n-\t\tsize++;\n-\t\tadd_buffer(&buffer, &size,\n-\t\t\t   \"encoding %s\\n\", git_commit_encoding);\n-\t}\n-\telse if (!is_utf8(buffer))\n+\tif (encoding_is_utf8 && !is_utf8(buffer))\n \t\tfprintf(stderr, commit_utf8_warn);\n \n \tif (!write_sha1_file(buffer, size, commit_type, commit_sha1)) {\ndiff --git a/commit.c b/commit.c\nindex 58c4ecb..3132323 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -598,56 +598,43 @@ static int add_merge_info(enum cmit_fmt fmt, char *buf, const struct commit *com\n \treturn offset;\n }\n \n-const char *commit_trailer(const struct commit *commit)\n+static char *get_header(const struct commit *commit, const char *key)\n {\n-\tunsigned long size, len;\n-\tchar type[10];\n+\tint key_len = strlen(key);\n+\tconst char *line = commit->buffer;\n \n-\tif (sha1_object_info(commit->object.sha1, type, &size))\n-\t\treturn NULL;\n-\tlen = strlen(commit->buffer);\n-\tif (len == size)\n-\t\treturn NULL;\n-\treturn commit->buffer + len + 1;\n-}\n-\n-char *find_commit_trailer(const struct commit *commit, const char *key)\n-{\n-\tconst char *trailer = commit_trailer(commit);\n-\tint keylen = strlen(key);\n-\tif (!trailer)\n-\t\treturn NULL;\n-\twhile (*trailer) {\n-\t\tconst char *eol = strchr(trailer, '\\n');\n-\t\tif (!eol)\n-\t\t\teol = trailer + strlen(trailer);\n-\t\tif (!strncmp(trailer, key, keylen) && trailer[keylen] == ' ') {\n-\t\t\tint valsz = eol - trailer - keylen;\n-\t\t\tchar *val = xmalloc(valsz + 1);\n-\t\t\tmemcpy(val, trailer + keylen + 1, valsz);\n-\t\t\tval[valsz] = '\\0';\n-\t\t\tif (val[valsz-1] == '\\n')\n-\t\t\t\tval[valsz-1] = '\\0';\n-\t\t\treturn val;\n+\tfor (;;) {\n+\t\tconst char *eol = strchr(line, '\\n'), *next;\n+\n+\t\tif (line == eol)\n+\t\t\treturn NULL;\n+\t\tif (!eol) {\n+\t\t\teol = line + strlen(line);\n+\t\t\tnext = NULL;\n+\t\t} else\n+\t\t\tnext = eol + 1;\n+\t\tif (!strncmp(line, key, key_len) && line[key_len] == ' ') {\n+\t\t\tint len = eol - line - key_len;\n+\t\t\tchar *ret = xmalloc(len);\n+\t\t\tmemcpy(ret, line + key_len + 1, len - 1);\n+\t\t\tret[len - 1] = '\\0';\n+\t\t\treturn ret;\n \t\t}\n-\t\tif (!eol)\n-\t\t\tbreak;\n-\t\ttrailer = eol + 1;\n+\t\tline = next;\n \t}\n-\treturn NULL;\n }\n \n static char *logmsg_to_utf8(const struct commit *commit)\n {\n-\tchar *encoding = find_commit_trailer(commit, \"encoding\");\n+\tchar *encoding = get_header(commit, \"encoding\");\n \tchar *out;\n \n \tif (!encoding)\n-\t\treturn commit->buffer;\n+\t\treturn NULL;\n \tout = reencode_string(commit->buffer, \"utf-8\", encoding);\n \tfree(encoding);\n \tif (!out)\n-\t\treturn commit->buffer;\n+\t\treturn NULL;\n \treturn out;\n }\n \n@@ -662,11 +649,12 @@ unsigned long pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit\n \tint parents_shown = 0;\n \tconst char *msg = commit->buffer;\n \tint plain_non_ascii = 0;\n-\tint msg_reencoded = 0;\n+\tchar *reencoded = NULL;\n \n \tif (strcmp(git_commit_encoding, \"utf-8\") && i18n_log_to_utf8) {\n-\t\tmsg = logmsg_to_utf8(commit);\n-\t\tmsg_reencoded = 1;\n+\t\treencoded = logmsg_to_utf8(commit);\n+\t\tif (reencoded)\n+\t\t\tmsg = reencoded;\n \t}\n \n \tif (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)\n@@ -816,8 +804,8 @@ unsigned long pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit\n \t\tbuf[offset++] = '\\n';\n \tbuf[offset] = '\\0';\n \n-\tif (msg_reencoded)\n-\t\tfree((char*)msg);\n+\tif (reencoded)\n+\t\tfree(reencoded);\n \treturn offset;\n }\n \n-- \n1.4.4.3.g0001f-dirty\n"},{"id":"30259","messageId":"7v3b74q1c9.fsf@assigned-by-dhcp.cox.net","threadId":"6097","inReplyTo":"Pine.LNX.4.63.0612241643440.19693@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] commit encoding: store it in commit header rather than mucking with NUL","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-25T00:13:42Z","receivedAt":"2006-12-25T00:13:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks for paying a very close attention to what is in 'pu'.\nThe series was a work in progress not yet ready for publication,\nwhich I merged by mistake because I was way too sleepy.\n\nThere are three reasons (and a half -- it was not even compile\ntested) that I did not mean to merge it yet.\n\n - I did not want to break existing git implementations, but\n   hadn't audited the commit header parsers to see if they do\n   not get upset when they see unrecognized header fields in\n   commit objects.  The 'trailer' was just a quick hack to have\n   something working for me to test the output conversion code.\n   I've looked at the code since then and I think it is Ok to\n   add new ones after author/name fields like you did.\n\n   If we _were_ to do this, I think it is preferable to make it\n   a header, not a trailer like I did.\n\n - I was not sure if the \"assume the whole commit->buffer is in\n   the local encoding and recode it into UTF-8\" is correct.  The\n   message body ought to be in i18n.commitencoding from the\n   repository the commit comes from, which is now recorded in\n   the trailer (or the header as in your patch), but what I was\n   unsure about was the encoding in which author and committer\n   names are recorded.  I think they are set either by the\n   environment variables or user.name to be consistent with\n   i18n.commitencoding in the repository the commit is made, so\n   it is Ok after all.\n\n - Existing Porcelains such as gitk know i18n.commitencoding is\n   a hint to them by the core, and expect the core to give them\n   output in the local encoding.  With the change, the core\n   feeds UTF-8 to the caller, unless the Porcelain gets the log\n   with plumbing \"cat-file\".  This means they either have to\n   lose code to do their own recoding (which is probably a good\n   thing in the long run), or we would need to have a flag for\n   them to tell the core not to do the conversion.  But a new\n   flag to ask for older behaviour is always a wrong way of\n   transitioning across backward incompatibility.\n\n   I think the output conversion from the log should be more\n   explicitly asked for it, than just a mere configuration\n   variable that cannot be overriden by gitk and friends.\n"},{"id":"30260","messageId":"Pine.LNX.4.63.0612250134040.19693@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6097","inReplyTo":"7v3b74q1c9.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] commit encoding: store it in commit header rather than mucking with NUL","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-25T00:41:47Z","receivedAt":"2006-12-25T00:41:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 24 Dec 2006, Junio C Hamano wrote:\n\n> Thanks for paying a very close attention to what is in 'pu'.\n\nYou're welcome ;-)\n\n>  - I did not want to break existing git implementations, but\n>    hadn't audited the commit header parsers to see if they do\n>    not get upset when they see unrecognized header fields in\n>    commit objects.\n\nI did not really audit it, but am quite sure that only parse_commit() \nreally walks the buffer, and even this function just ignores what comes \nafter committer. But if I read the code correctly, then the mandatory \nheaders (\"tree parent* author committer\") have to come first, in that \norder.\n\n>  - I was not sure if the \"assume the whole commit->buffer is in\n>    the local encoding and recode it into UTF-8\" is correct.\n\nFor the purpose of showing it, there is no point in using two different \nencodings. I am not aware of any terminal (and do not own such a terminal \nanyway) which can display text with parts encoded differently from the \nrest.\n\n>  - Existing Porcelains such as gitk know i18n.commitencoding is\n>    a hint to them by the core, and expect the core to give them\n>    output in the local encoding.  With the change, the core\n>    feeds UTF-8 to the caller, unless the Porcelain gets the log\n>    with plumbing \"cat-file\".  This means they either have to\n>    lose code to do their own recoding (which is probably a good\n>    thing in the long run), or we would need to have a flag for\n>    them to tell the core not to do the conversion.  But a new\n>    flag to ask for older behaviour is always a wrong way of\n>    transitioning across backward incompatibility.\n> \n>    I think the output conversion from the log should be more\n>    explicitly asked for it, than just a mere configuration\n>    variable that cannot be overriden by gitk and friends.\n\nProbably a new flag \"--encoding\" to log, which sets git_commit_encoding. \nAnd the function you introduced should probably not blindly convert to \nUTF-8, but to the current git_commit_encoding.\n\nCiao,\nDscho\n"},{"id":"30262","messageId":"7vr6uoo2ow.fsf@assigned-by-dhcp.cox.net","threadId":"6097","inReplyTo":"Pine.LNX.4.63.0612250134040.19693@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] commit encoding: store it in commit header rather than mucking with NUL","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-25T07:27:27Z","receivedAt":"2006-12-25T07:27:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>>  - I was not sure if the \"assume the whole commit->buffer is in\n>>    the local encoding and recode it into UTF-8\" is correct.\n>\n> For the purpose of showing it, there is no point in using two different \n> encodings. I am not aware of any terminal (and do not own such a terminal \n> anyway) which can display text with parts encoded differently from the \n> rest.\n\nThat's not what I meant.  Because the definition of the contents\nof the commit has been (and will be --- the unpublished topic\nwas about suggesting stronger convention across Porcelains) just\nthe set of fixed headers plus binary blob, the use of different\nencodings is entirely up to the users.\n\nI was afraid that there might be something we did (or we did not\ndo) that encouraged people to have their names (via environment\nvariables, or perhaps user.name) always in UTF-8 while recording\nthe log messages in the legacy encoding, and if that kind of use\nis already done in the wild, we would end up having to not\nreencode the header field but reencode the body.\n\nBut I do not think we ever encouraged encoding names in UTF-8 or\nanything else (we did encourage use of UTF-8 in the commit log),\nso I think we are Ok.\n \n"},{"id":"30281","messageId":"empkme$qp$2@sea.gmane.org","threadId":"6097","inReplyTo":"7vr6uoo2ow.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] commit encoding: store it in commit header rather than mucking with NUL","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-25T22:54:47Z","receivedAt":"2006-12-25T22:54:47Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n> I was afraid that there might be something we did (or we did not\n> do) that encouraged people to have their names (via environment\n> variables, or perhaps user.name) always in UTF-8 while recording\n> the log messages in the legacy encoding, and if that kind of use\n> is already done in the wild, we would end up having to not\n> reencode the header field but reencode the body.\n> \n> But I do not think we ever encouraged encoding names in UTF-8 or\n> anything else (we did encourage use of UTF-8 in the commit log),\n> so I think we are Ok.\n  \nBy the way, it would be nice to have .mailmap like mechanism to tell git\nthat those two (three or more) names are the same comitter and should be\nshown (at least in git-shortlog) as this. This is because some people\nchanged their email, some people have sometimes middle initial and\nsometimes not, some people have name outside US-ASCII and it is sometimes\nbroken...\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"}]}