{"thread":{"id":"6155","subject":"Possible regression in git-rev-list --header","startedAt":"2006-12-30T17:56:19Z","lastAt":"2007-01-04T15:21:04Z","messageCount":26,"participants":["Marco Costalba","Jakub Narebski","Johannes Schindelin","Junio C Hamano","Lars Hjemli","Andreas Ericsson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"30503","messageId":"e5bfff550612300956mef4691fqf607fad173c571da@mail.gmail.com","threadId":"6155","inReplyTo":null,"subject":"Possible regression in git-rev-list --header","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2006-12-30T17:56:19Z","receivedAt":"2006-12-30T17:56:19Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"When 'commitencoding' variable is set in config file then git-rev-list\ncalled with --header option sends also the encoding information.\n\n$ git rev-list --header -n1 HEAD\n6d751699cb04150abd79a730187d4e2ed6330c05\ntree 70209eebdc59d108948feb15c3c5497f299ef290\nparent 49a8186d7352d0454df79b289fecb18c8e535c32\nauthor Marco Costalba 1167500660 +0100\ncommitter Marco Costalba 1167500660 +0100\nencoding UTF-8\n\n Test commit\n\n Let's see what git-rev-list --header spits out.\n\n Signed-off-by: Marco Costalba \n\n$ git --version\ngit version 1.5.0.rc0.g1ed48\n\n\nThis is very unfortunate because qgit parsing routine it's totally\nbroken after this change. Please revert the patch, or at least make\ngit-rev-list --header quiet as default, IOW add an option\n--show-encoding that defaults to disabled.\n\nSorry to ask you this but _all_ the qgit users out there will see a\nbroken main view revision's list for all the commits created after the\npatch went in.\n\nThanks\nMarco\n"},{"id":"30504","messageId":"en6b34$8ni$1@sea.gmane.org","threadId":"6155","inReplyTo":"e5bfff550612300956mef4691fqf607fad173c571da@mail.gmail.com","subject":"Re: Possible regression in git-rev-list --header","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-30T18:30:48Z","receivedAt":"2006-12-30T18:30:48Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Marco Costalba wrote:\n\n> When 'commitencoding' variable is set in config file then git-rev-list\n> called with --header option sends also the encoding information.\n> \n> $ git rev-list --header -n1 HEAD\n> 6d751699cb04150abd79a730187d4e2ed6330c05\n> tree 70209eebdc59d108948feb15c3c5497f299ef290\n> parent 49a8186d7352d0454df79b289fecb18c8e535c32\n> author Marco Costalba 1167500660 +0100\n> committer Marco Costalba 1167500660 +0100\n> encoding UTF-8\n> \n> Test commit\n> \n> Let's see what git-rev-list --header spits out.\n> \n> Signed-off-by: Marco Costalba \n> \n> $ git --version\n> git version 1.5.0.rc0.g1ed48\n> \n> \n> This is very unfortunate because qgit parsing routine it's totally\n> broken after this change. Please revert the patch, or at least make\n> git-rev-list --header quiet as default, IOW add an option\n> --show-encoding that defaults to disabled.\n\nWell, that is incompatibile change, but it also shows design errors\nin qgit, as it should ignore unknown headers, and rely on \"\\n\\n\" to\nseparate header from the body. Isn't it?\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"30507","messageId":"Pine.LNX.4.63.0612301955340.19693@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6155","inReplyTo":"e5bfff550612300956mef4691fqf607fad173c571da@mail.gmail.com","subject":"Re: Possible regression in git-rev-list --header","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-30T18:57:39Z","receivedAt":"2006-12-30T18:57:39Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 30 Dec 2006, Marco Costalba wrote:\n\n> When 'commitencoding' variable is set in config file then git-rev-list\n> called with --header option sends also the encoding information.\n\nAs Jakub pointed out, qgit should not expect to know all headers. I am \nvery sorry, since I said I looked at all parsers of the commit header in \ngit, but that was _only_ git, and no porcelains.\n\nPlease fix qgit, since I really consider this a bug.\n\nCiao,\nDscho\n"},{"id":"30514","messageId":"7v7iw9jftv.fsf@assigned-by-dhcp.cox.net","threadId":"6155","inReplyTo":"Pine.LNX.4.63.0612301955340.19693@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: Possible regression in git-rev-list --header","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-30T20:20:44Z","receivedAt":"2006-12-30T20:20:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Sat, 30 Dec 2006, Marco Costalba wrote:\n>\n>> When 'commitencoding' variable is set in config file then git-rev-list\n>> called with --header option sends also the encoding information.\n>\n> As Jakub pointed out, qgit should not expect to know all headers. I am \n> very sorry, since I said I looked at all parsers of the commit header in \n> git, but that was _only_ git, and no porcelains.\n>\n> Please fix qgit, since I really consider this a bug.\n\nI have to agree with Johannes. In principle Porcelains should\nbe prepared to see and ignore unknown headers.\n\nHowever, this commit created by `commit-tree` certalinly can be\nimproved.\n\n $ git rev-list --header -n1 HEAD\n 6d751699cb04150abd79a730187d4e2ed6330c05\n tree 70209eebdc59d108948feb15c3c5497f299ef290\n parent 49a8186d7352d0454df79b289fecb18c8e535c32\n author Marco Costalba 1167500660 +0100\n committer Marco Costalba 1167500660 +0100\n encoding UTF-8\n\n Test commit\n\n Let's see what git-rev-list --header spits out.\n\n Signed-off-by: Marco Costalba \n\n-- >8 --\ncommit-tree: cope with different ways \"utf-8\" can be spelled.\n\nPeople can spell config.commitencoding differently from what we\ninternally have (\"utf-8\") to mean UTF-8. Try to accept them and\ntreat them equally.\n\nSigned-off-by: Junio C Hamano \n---\n\ndiff --git a/builtin-commit-tree.c b/builtin-commit-tree.c\nindex 146aaff..0651e59 100644\n--- a/builtin-commit-tree.c\n+++ b/builtin-commit-tree.c\n@@ -119,8 +119,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* Not having i18n.commitencoding is the same as having utf-8 */\n-\tencoding_is_utf8 = (!git_commit_encoding ||\n-\t\t\t !strcmp(git_commit_encoding, \"utf-8\"));\n+\tencoding_is_utf8 = is_encoding_utf8(git_commit_encoding);\n \n \tinit_buffer(&buffer, &size);\n \tadd_buffer(&buffer, &size, \"tree %s\\n\", sha1_to_hex(tree_sha1));\ndiff --git a/utf8.c b/utf8.c\nindex 1eedd8b..7c80eec 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -277,6 +277,15 @@ void print_wrapped_text(const char *text, int indent, int indent2, int width)\n \t}\n }\n \n+int is_encoding_utf8(const char *name)\n+{\n+\tif (!name)\n+\t\treturn 1;\n+\tif (!strcasecmp(name, \"utf-8\") || !strcasecmp(name, \"utf8\"))\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n /*\n * Given a buffer and its encoding, return it re-encoded\n * with iconv. If the conversion fails, returns NULL.\ndiff --git a/utf8.h b/utf8.h\nindex cae2a8e..a07c5a8 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -3,6 +3,8 @@\n \n int utf8_width(const char **start);\n int is_utf8(const char *text);\n+int is_encoding_utf8(const char *name);\n+\n void print_wrapped_text(const char *text, int indent, int indent2, int len);\n \n #ifndef NO_ICONV\n"},{"id":"30515","messageId":"7v3b6xjfrr.fsf_-_@assigned-by-dhcp.cox.net","threadId":"6155","inReplyTo":"Pine.LNX.4.63.0612301955340.19693@wbgn013.biozentrum.uni-wuerzburg.de","subject":"[PATCH] Move commit reencoding parameter parsing to revision.c","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-30T20:22:00Z","receivedAt":"2006-12-30T20:22:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Also I think we need this patch for completeness on top of what\nwe have in 'master'.\n\n-- >8 --\n[PATCH] Move commit reencoding parameter parsing to revision.c\n\nThis way, git-rev-list and git-diff-tree with --pretty can use\nit. Because they are both plumbing, we default not to do any\nconversion unless --encoding= is explicitly given from\nthe command line.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-rev-list.txt | 1 +\n Documentation/pretty-formats.txt | 8 ++++++++\n builtin-diff-tree.c | 6 ++++++\n builtin-rev-list.c | 6 ++++++\n revision.c | 8 ++++++++\n 5 files changed, 29 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt\nindex 9e0dcf8..86c94e7 100644\n--- a/Documentation/git-rev-list.txt\n+++ b/Documentation/git-rev-list.txt\n@@ -21,6 +21,7 @@ SYNOPSIS\n \t [ \\--stdin ]\n \t [ \\--topo-order ]\n \t [ \\--parents ]\n+\t [ \\--encoding[=] ]\n \t [ \\--(author|committer|grep)= ]\n \t [ [\\--objects | \\--objects-edge] [ \\--unpacked ] ]\n \t [ \\--pretty | \\--header ]\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 996f628..8cba13f 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -76,3 +76,11 @@ displayed in full, regardless of whether --abbrev or\n --no-abbrev are used, and 'parents' information show the\n true parent commits, without taking grafts nor history\n simplification into account.\n+\n+\n+--encoding[=]::\n+\tThe commit objects record the encoding used for the log message\n+\tin their encoding header; this option can be used to tell the\n+\tcommand to re-code the commit log message in the encoding\n+\tpreferred by the user. For non plumbing commands this\n+\tdefaults to UTF-8.\ndiff --git a/builtin-diff-tree.c b/builtin-diff-tree.c\nindex 24cb2d7..6ce2c0f 100644\n--- a/builtin-diff-tree.c\n+++ b/builtin-diff-tree.c\n@@ -67,6 +67,12 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \tstatic struct rev_info *opt = &log_tree_opt;\n \tint read_stdin = 0;\n \n+\t/* default to turn the conversion off -- diff-tree is not a\n+\t * Porcelain but as low plumbing as it can go, as far as\n+\t * two-tree comparison is concerned.\n+\t */\n+\tgit_log_output_encoding = \"\";\n+\n \tinit_revisions(opt, prefix);\n \tgit_config(git_default_config); /* no \"diff\" UI options */\n \tnr_sha1 = 0;\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex 1bb3a06..0c3dce6 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -226,6 +226,12 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tint i;\n \tint read_from_stdin = 0;\n \n+\t/* default to turn the conversion off -- rev-list is not a\n+\t * Porcelain but as low plumbing as it can go, as far as\n+\t * revision traversal is concerned.\n+\t */\n+\tgit_log_output_encoding = \"\";\n+\n \tinit_revisions(&revs, prefix);\n \trevs.abbrev = 0;\n \trevs.commit_format = CMIT_FMT_UNSPECIFIED;\ndiff --git a/revision.c b/revision.c\nindex af9f874..6e4ec46 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1039,6 +1039,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\t\t\tall_match = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strncmp(arg, \"--encoding=\", 11)) {\n+\t\t\t\targ += 11;\n+\t\t\t\tif (strcmp(arg, \"none\"))\n+\t\t\t\t\tgit_log_output_encoding = strdup(arg);\n+\t\t\t\telse\n+\t\t\t\t\tgit_log_output_encoding = \"\";\n+\t\t\t\tcontinue;\n+\t\t\t}\n \n \t\t\topts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);\n \t\t\tif (opts > 0) {\n-- \n1.5.0.rc0.g6bb1\n"},{"id":"30516","messageId":"7vlkkphvrb.fsf@assigned-by-dhcp.cox.net","threadId":"6155","inReplyTo":"7v7iw9jftv.fsf@assigned-by-dhcp.cox.net","subject":"Re: Possible regression in git-rev-list --header","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-30T22:19:36Z","receivedAt":"2006-12-30T22:19:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Johannes Schindelin writes:\n>\n>> On Sat, 30 Dec 2006, Marco Costalba wrote:\n>>\n>>> When 'commitencoding' variable is set in config file then git-rev-list\n>>> called with --header option sends also the encoding information.\n>>\n>> As Jakub pointed out, qgit should not expect to know all headers. I am \n>> very sorry, since I said I looked at all parsers of the commit header in \n>> git, but that was _only_ git, and no porcelains.\n>>\n>> Please fix qgit, since I really consider this a bug.\n>\n> I have to agree with Johannes. In principle Porcelains should\n> be prepared to see and ignore unknown headers.\n>\n> However, this commit created by `commit-tree` certalinly can be\n> improved.\n> ...\n\nAnother thing. I think it would make sense to remove \"encoding\"\nheader after pretty_print_commit successfully re-codes the\nbuffer. An alternative is to rewrite \"encoding\" header to show\nwhich encoding the log now uses (and omit it if it is UTF-8).\n\nThe attached patch does the latter, but I think removing the\nheader altogether in this case would make it easier to use\noverall. If you want to, you can change\n\n\tif (is_encoding_utf8(encoding))\n\nin the patch with \"if (1)\" (the above macro is what was\nintroduced in my previous patch).\n\nPro for having \"encoding\" rewritten as this patch does is that\nthe output is still self-identifying. You can tell what\nencoding you are supposed to interpret the log messages in. Con\nagainst it is that the output is after re-coding as the user\nasked (either with the config or --encoding= option),\nand the extra header becomes redundant information.\n\n-- >8 --\n\n commit.c | 44 ++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 44 insertions(+), 0 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex e13b9cb..bf82fe8 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -624,6 +624,48 @@ static char *get_header(const struct commit *commit, const char *key)\n \t}\n }\n \n+static char *replace_encoding_header(char *buf, char *encoding)\n+{\n+\tchar *encoding_header = strstr(buf, \"\\nencoding \");\n+\tchar *end_of_encoding_header;\n+\tint encoding_header_pos;\n+\tint encoding_header_len;\n+\tint new_len;\n+\tint need_len;\n+\tint buflen = strlen(buf) + 1;\n+\n+\tif (!encoding_header)\n+\t\treturn buf; /* should not happen but be defensive */\n+\tencoding_header++;\n+\tend_of_encoding_header = strchr(encoding_header, '\\n');\n+\tif (!end_of_encoding_header)\n+\t\treturn buf; /* should not happen but be defensive */\n+\tend_of_encoding_header++;\n+\n+\tencoding_header_len = end_of_encoding_header - encoding_header;\n+\tencoding_header_pos = encoding_header - buf;\n+\n+\tif (is_encoding_utf8(encoding)) {\n+\t\t/* we have re-coded to UTF-8; drop the header */\n+\t\tmemmove(encoding_header, end_of_encoding_header,\n+\t\t\tbuflen - (encoding_header_pos + encoding_header_len));\n+\t\treturn buf;\n+\t}\n+\tnew_len = strlen(encoding);\n+\tneed_len = new_len + strlen(\"encoding \\n\");\n+\tif (encoding_header_len < need_len) {\n+\t\tbuf = xrealloc(buf, buflen + (need_len - encoding_header_len));\n+\t\tencoding_header = buf + encoding_header_pos;\n+\t\tend_of_encoding_header = encoding_header + encoding_header_len;\n+\t}\n+\tmemmove(end_of_encoding_header + (need_len - encoding_header_len),\n+\t\tend_of_encoding_header,\n+\t\tbuflen - (encoding_header_pos + encoding_header_len));\n+\tmemcpy(encoding_header + 9, encoding, strlen(encoding));\n+\tencoding_header[9 + new_len] = '\\n';\n+\treturn buf;\n+}\n+\n static char *logmsg_reencode(const struct commit *commit)\n {\n \tchar *encoding;\n@@ -642,6 +684,8 @@ static char *logmsg_reencode(const struct commit *commit)\n \t\treturn NULL;\n \t}\n \tout = reencode_string(commit->buffer, output_encoding, encoding);\n+\tout = replace_encoding_header(out, output_encoding);\n+\n \tfree(encoding);\n \tif (!out)\n \t\treturn NULL;\n"},{"id":"30517","messageId":"Pine.LNX.4.63.0612310134370.19693@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6155","inReplyTo":"7v7iw9jftv.fsf@assigned-by-dhcp.cox.net","subject":"Re: Possible regression in git-rev-list --header","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-31T00:37:55Z","receivedAt":"2006-12-31T00:37:55Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 30 Dec 2006, Junio C Hamano wrote:\n\n> commit-tree: cope with different ways \"utf-8\" can be spelled.\n\nDoes iconv handle these? If not, we should reset git_commit_encoding.\n\nCiao,\nDscho\n"},{"id":"30521","messageId":"Pine.LNX.4.63.0612310210330.25709@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6155","inReplyTo":"7v3b6xjfrr.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Move commit reencoding parameter parsing to revision.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-31T01:10:58Z","receivedAt":"2006-12-31T01:10:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nthis patch makes a lot of sense IMHO.\n\nCiao,\nDscho\n"},{"id":"30522","messageId":"Pine.LNX.4.63.0612310211300.25709@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6155","inReplyTo":"7vlkkphvrb.fsf@assigned-by-dhcp.cox.net","subject":"Re: Possible regression in git-rev-list --header","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-31T01:13:27Z","receivedAt":"2006-12-31T01:13:27Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 30 Dec 2006, Junio C Hamano wrote:\n\n> Another thing. I think it would make sense to remove \"encoding\" header \n> after pretty_print_commit successfully re-codes the buffer. An \n> alternative is to rewrite \"encoding\" header to show which encoding the \n> log now uses (and omit it if it is UTF-8).\n\nI think it would be wrong. Sure, the output may be encoded differently, \nbut the _original_ commit was not. And this is the information I want \nto see when I look at the raw commit.\n\nIf you do not want to see all headers, you have to choose a different \n--pretty option.\n\nCiao,\nDscho\n \n"},{"id":"30526","messageId":"7v64bsj0s4.fsf@assigned-by-dhcp.cox.net","threadId":"6155","inReplyTo":"Pine.LNX.4.63.0612310211300.25709@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: Possible regression in git-rev-list --header","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-31T01:45:47Z","receivedAt":"2006-12-31T01:45:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Sat, 30 Dec 2006, Junio C Hamano wrote:\n>\n>> Another thing. I think it would make sense to remove \"encoding\" header \n>> after pretty_print_commit successfully re-codes the buffer. An \n>> alternative is to rewrite \"encoding\" header to show which encoding the \n>> log now uses (and omit it if it is UTF-8).\n>\n> I think it would be wrong. Sure, the output may be encoded differently, \n> but the _original_ commit was not. And this is the information I want \n> to see when I look at the raw commit.\n\nWhen you want to see the raw commit, you would not ask for it to\nre-code, so \"removal after successfully re-codes\" would not kick\nin (if you _really_ want to look at the raw commit, I guess\ncat-file can help, but let's not go there). Re-coding the\nmessage but still showing what the original encoding was does\nnot sound making much sense to me.\n\nI've pushed this out after a small rework.\n\nThe rule is:\n\n - if you ask for re-coding (either by i18n.* configuration or\n an explicit --encoding option from the command line), and if\n re-coding successfully does its job, you do not see\n \"encoding\" header;\n\n - if the buffer cannot successfully be re-coded, no re-coding\n is done, and the caller can inspect \"encoding\" header.\n\n - if you do not ask for re-coding, \"encoding\" header is left as\n is, so is the commit log message. The caller can deal with\n any re-coding itself.\n"},{"id":"30575","messageId":"e5bfff550612310345j4c882b7av51879ca1175a1c6b@mail.gmail.com","threadId":"6155","inReplyTo":"7v64bsj0s4.fsf@assigned-by-dhcp.cox.net","subject":"Re: Possible regression in git-rev-list --header","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2006-12-31T11:45:37Z","receivedAt":"2006-12-31T11:45:37Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"I'm sorry to reply only now but I have problems with my internet\nconnection, I am also not able to test Junio patches now.\n\nRegarding qgit parsing 'bug' I would like to point out something that\nprobably is not clear.\n\n1) Parsing routine _must_ be able to sustain the loading of more then\n40000 revisions in a couple of seconds, so must be very quick. A lot\nof effort has been put to index the header info at maximum speed. Now\nit takes about 300ms to parse the whole linux tree. You can have this\nonly if the header format is 'fixed enough', it means that you would\nnot expect whole new lines (new '\\n' chars) to appear from nowhere in\nheader, with the exception of log message and parents info lines of\ncourse.\n\n2) I _have_ to rely on headers info because that's where I get data to\nshow the user, that's the whole point of calling git-rev-list with\n--headers option.\n\n3) The rule of double '\\n\\n' as an indicator of the start of log\nmessage it's not only very slow, it's also broken in cases of no log,\nsee 7b7abfe3dd81d in Linux tree.\n\n4) We are talking of keeping back compatibility, _fixing_ qgit does\nnot solves the issues with current qgit users.\n\nSo please I really would ask again to *do not print that encoding\nextra line if not requested to do so*\n\nThanks again\nMarco\n\nP.S: One way to really speed up the indexing would be if git-rev-list\nprints at the beginning the length of the record, i.e. the distance\nfrom the next '\\0' terminating point. I don't know if this info is\nalready available to git-rev-list before to print the record or has to\nfind it anyway, in the latter case there is no point in doing this.\nBut in the former case it would be possible to avoid a costly\nfind('\\0'), starting from the beginning of log message. Log message is\nmore then the half of all the record length so it would be possible to\navoid a good amount of unuseful work because only the position of the\nnext '\\0' is needed during parsing, not the log message, that is\nretrieved and shown to the user only on demand, i.e. only for the\nselected revision.\n"},{"id":"30583","messageId":"Pine.LNX.4.63.0612311613180.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6155","inReplyTo":"e5bfff550612310345j4c882b7av51879ca1175a1c6b@mail.gmail.com","subject":"Re: Possible regression in git-rev-list --header","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-31T15:27:03Z","receivedAt":"2006-12-31T15:27:03Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 31 Dec 2006, Marco Costalba wrote:\n\n> Regarding qgit parsing 'bug' I would like to point out something that\n> probably is not clear.\n\nBut it _is_ a bug!\n\n> 1) Parsing routine _must_ be able to sustain the loading of more then\n> 40000 revisions in a couple of seconds, so must be very quick. A lot\n> of effort has been put to index the header info at maximum speed. Now\n> it takes about 300ms to parse the whole linux tree. You can have this\n> only if the header format is 'fixed enough', it means that you would\n> not expect whole new lines (new '\\n' chars) to appear from nowhere in\n> header, with the exception of log message and parents info lines of\n> course.\n\nI don't see why this would slow down parsing _at all_. Besides, you should \nreally stop relying on the header format being fixed for all eternity. \nThere has been talk about putting more useful information into the header, \nand there _are_ valid reasons to keep the header extensible.\n\nFurther, if you rely on parsing being super-fast, why not just parse \n_only_ the header information that you actually need? The header still \nconsists of\n\n\t- exactly one \"tree\",\n\t- an arbitrary amount of \"parent\" lines,\n\t- exactly one \"author\", and\n\t- exactly one \"committer\" line\n\nAfter that may come optional headers, but by that time you should \n_already_ have stopped parsing! And the order is fixed already \n(parse_commit_buffer() relies on it).\n\nAfter all, you have an initial parsing for the purpose of organizing the \ncommits, and you can have _another_ for the purpose of displaying the \nmessage (you can remember the offset where the first parsing stopped to \naccelerate the second). The latter parsing should be done individually, \nwhen displaying the commit.\n\nAnd I still have to disagree with Junio that the encoding header is no \nlonger needed when displaying the commit message. The \"tree\" and \"parent\" \nheaders are also displayed, even if their information is already used for \npurposes of displaying them.\n\nThe commit header contains information about that particular commit, and \nif I ask to see the headers, I want to see them, and not be treated like \nan idiot who does not know how to handle that information.\n\n(If I ask for git-log to show everything encoded in Latin-1, it might \nstill be interesting to know who used which encoding. And if it is \ndisplayed in my local encoding, but the commit header says UTF-8, I _do_ \nknow that this is the original encoding, not the displayed one, thank you \nvery much!)\n\nSo please, Marco, fix that bug in qgit. Otherwise you will restrict our \nability to enhance commit objects with useful meta information _anyway_. \nIOW, even if the encoding header is not shown (which I would not like), \nyou should fix that bug.\n\nCiao,\nDscho\n"},{"id":"30584","messageId":"e5bfff550612310743t4ca1b017ubceddffafd06ac59@mail.gmail.com","threadId":"6155","inReplyTo":"Pine.LNX.4.63.0612311613180.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: Possible regression in git-rev-list --header","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2006-12-31T15:43:13Z","receivedAt":"2006-12-31T15:43:13Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 12/31/06, Johannes Schindelin wrote:\n>\n> Further, if you rely on parsing being super-fast, why not just parse\n> _only_ the header information that you actually need? The header still\n> consists of\n>\n> - exactly one \"tree\",\n> - an arbitrary amount of \"parent\" lines,\n> - exactly one \"author\", and\n> - exactly one \"committer\" line\n>\n> After that may come optional headers,\n\nIf you intorduce the concept of an 'optional header part' you\nlogically and naturally _may_ also introduce the concept of disabling\nthe display of _that_ optional header, or better, to keep back\ncompatibility (I just remaind that's more then one year and an half\nthat git-rev-list works in that way!) you may accept the concept of an\noption to show the additional optional header part. That's just what\nI'm asking, no more no less.\n\n>but by that time you should\n> _already_ have stopped parsing! And the order is fixed already\n> (parse_commit_buffer() relies on it).\n>\n> After all, you have an initial parsing for the purpose of organizing the\n> commits, and you can have _another_ for the purpose of displaying the\n> message (you can remember the offset where the first parsing stopped to\n> accelerate the second). The latter parsing should be done individually,\n> when displaying the commit.\n>\n\nThe problem with your proposed algorithm is that you don't have _one_\ncommit but a sequence of commits to parse, so when you have parsed\nuntil the committer line you must need to know where the next commit\nstarts, IOW you have to find the next '\\0', that's what I was trying\nto expose in my previous e-mail postscriptum.\n\n\nThanks\nMarco\n"},{"id":"30602","messageId":"7vodpja0u8.fsf@assigned-by-dhcp.cox.net","threadId":"6155","inReplyTo":"e5bfff550612310743t4ca1b017ubceddffafd06ac59@mail.gmail.com","subject":"Re: Possible regression in git-rev-list --header","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-01-01T03:21:35Z","receivedAt":"2007-01-01T03:21:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> On 12/31/06, Johannes Schindelin wrote:\n>>\n>> Further, if you rely on parsing being super-fast, why not just parse\n>> _only_ the header information that you actually need? The header still\n>> consists of\n>>\n>> - exactly one \"tree\",\n>> - an arbitrary amount of \"parent\" lines,\n>> - exactly one \"author\", and\n>> - exactly one \"committer\" line\n>>\n>> After that may come optional headers,\n\nThey are more like 'other' headers. Nobody said the set of\nheaders are cast in stone forever. The only things parsers\nsafely can assume are that the original four kinds come at the\nbeginning in the above order, and there is a blank line that\nseparates headers and the body.\n\n> If you intorduce the concept of an 'optional header part' you\n> logically and naturally _may_ also introduce the concept of disabling\n> the display of _that_ optional header, or better, to keep back\n> compatibility...\n\nWhile I am somewhat sympathetic, and am willing to apologize for\ntrying to advance the i18n support without enough advance\nwarning, I think you already know what you are saying does not\nmake much sense in the larger picture and as the longer term\nsolution. Does any MUA ask the filesystem, POP3 server or IMAP\nserver not to give X-* headers?\n\nWe could declare \"headers are cast in stone and we will not\nenhance it in any way forever\", and go back to my original hack\nto use hidden trailer, but I do not think it would solve\nanything. Porcelains that would try to take advantage of the\ntrailer would now start assuming incorrectly that the set of\ntrailers are cast in stone and will break when new information\nis added to the trailer, which would bring us back to exactly\nwhere we are now.\n\nHaving said that, I think what is in the current tip of 'master'\nis of much less impact for normal repositories than the one that\nbit you, in two aspects.\n\n * Your sample commit had \"encoding UTF-8\" header, presumably\n because the repository that the commit was created in had\n core.commitencoding set to \"UTF-8\". The intent was not to\n add anything when the log message is UTF-8, but the earlier\n code was checking only for \"utf-8\". With the current tip of\n master this should not happen anymore.\n\n * When the output encoding conversion is done successfully, the\n current tip of master drops \"encoding\" header from the\n output, so in your sample situation where the commit has\n (incorrectly) \"encoding UTF-8\" and your output encoding is\n also \"UTF-8\" (because either you have core.commitencoding set\n to it, or you do not have the configuration and let git fall\n back to its default which is UTF-8), you would not see the\n \"encoding\" header in the output from the rev-list.\n\nThe reason we did the latter, by the way, does not have anything\nto do with helping broken parsers. We drop the header after\nre-coding the log message into an encoding specified by the user\n(which is presumably different from what the commit was\noriginally recorded in) because the encoding recorded on\n\"encoding\" header would not match the re-coded log message\nanymore.\n"},{"id":"30703","messageId":"Pine.LNX.4.63.0701022219050.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6155","inReplyTo":"7vodpja0u8.fsf@assigned-by-dhcp.cox.net","subject":"Re: Possible regression in git-rev-list --header","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-01-02T21:32:24Z","receivedAt":"2007-01-02T21:32:24Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 31 Dec 2006, Junio C Hamano wrote:\n\n> \"Marco Costalba\" writes:\n> \n> > On 12/31/06, Johannes Schindelin wrote:\n> >>\n> >> Further, if you rely on parsing being super-fast, why not just parse\n> >> _only_ the header information that you actually need? The header still\n> >> consists of\n> >>\n> >> - exactly one \"tree\",\n> >> - an arbitrary amount of \"parent\" lines,\n> >> - exactly one \"author\", and\n> >> - exactly one \"committer\" line\n> >>\n> >> After that may come optional headers,\n> \n> They are more like 'other' headers.\n\nI should have been more clear: optional for the committer.\n\n> > If you intorduce the concept of an 'optional header part' you\n> > logically and naturally _may_ also introduce the concept of disabling\n> > the display of _that_ optional header, or better, to keep back\n> > compatibility...\n> \n> While I am somewhat sympathetic, and am willing to apologize for\n> trying to advance the i18n support without enough advance\n> warning, I think you already know what you are saying does not\n> make much sense in the larger picture and as the longer term\n> solution.\n\nBesides, when you say\n\n> The problem with your proposed algorithm is that you don't have _one_ \n> commit but a sequence of commits to parse, so when you have parsed until \n> the committer line you must need to know where the next commit starts, \n> IOW you have to find the next '\\0', that's what I was trying to expose \n> in my previous e-mail postscriptum.\n\nI don't get _at all_ what could be a problem _with_ the encoding header \nthat is no problem _without_ it. I assume you want to tell me something \nmore than that you do not want to change your code? If so, I missed it.\n\n> * When the output encoding conversion is done successfully, the\n> current tip of master drops \"encoding\" header from the\n> output, [...]\n\nEarlier, I said that I do not feel strongly about that issue.\n\nBut now I do.\n\nIf you drop the \"encoding\" header from the commit buffer, just because you \nreencoded it to whatever encoding happens to be the one the caller just \nasked for, you are _not_ interpreting the data, but _changing_ it.\n\nThat is not what git is about, IMHO. It would be a completely different \nthing if the caller had a way to ask for _specific_ headers, and asks to \nbe left alone with all the other cruft. But the caller does not even have \nthe chance to say that, let alone ask specifically _for_ it.\n\nThe encoding header bears information, just like the tree header or the \ncommitter header. I find it highly irritating that I am shielded from it. \nThe encoding header has _nothing_ to do with the encoding that the output \nis being encoded with, but _all_ with how the commit message was encoded \n_by the committer_.\n\n> The reason we did the latter, by the way, does not have anything\n> to do with helping broken parsers. We drop the header after\n> re-coding the log message into an encoding specified by the user\n> (which is presumably different from what the commit was\n> originally recorded in) because the encoding recorded on\n> \"encoding\" header would not match the re-coded log message\n> anymore.\n\nBy the same reasoning, you'd have to rewrite the committer line to reflect \nthe current GIT_COMMITTER_IDENT, or hide it. If you want to convince me, \nyou have to try harder.\n\nAnd Marco has to fix the header parsing anyway.\n\nSo, please, Junio, can you rethink that decision?\n\nCiao,\nDscho\n"},{"id":"30711","messageId":"7vps9xrs9y.fsf@assigned-by-dhcp.cox.net","threadId":"6155","inReplyTo":"Pine.LNX.4.63.0701022219050.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: Possible regression in git-rev-list --header","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-01-02T22:13:45Z","receivedAt":"2007-01-02T22:13:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> The reason we did the latter, by the way, does not have anything\n>> to do with helping broken parsers. We drop the header after\n>> re-coding the log message into an encoding specified by the user\n>> (which is presumably different from what the commit was\n>> originally recorded in) because the encoding recorded on\n>> \"encoding\" header would not match the re-coded log message\n>> anymore.\n>\n> By the same reasoning, you'd have to rewrite the committer line to reflect \n> the current GIT_COMMITTER_IDENT, or hide it. If you want to convince me, \n> you have to try harder.\n\nSorry, but you completely lost me with that analogy.\n\nI think showing log message in the user's preferred encoding is\nmore like passing the output to the colorization mechanism and\nthen to the pager. We are interacting with humans at that\npoint, and we are changing the presentation without changing the\nsemantics of the data. \n\nI do not see why committer identity needs to be rewritten nor\nhidden by the same reasoning.\n\n> And Marco has to fix the header parsing anyway.\n\nNo question about that. If iconv() punts, qgit will see\n\"encoding\" header to deal with even when the re-coding is in\neffect. I think it may be a sensible thing for qgit to replace\nthe log message and show \"log message in this encoding, which\ncannot be shown in this window\" instead in such a case, but that\nis up to Porcelain.\n"},{"id":"30715","messageId":"Pine.LNX.4.63.0701022322030.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6155","inReplyTo":"7vps9xrs9y.fsf@assigned-by-dhcp.cox.net","subject":"Re: Possible regression in git-rev-list --header","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-01-02T22:28:58Z","receivedAt":"2007-01-02T22:28:58Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 2 Jan 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> >> The reason we did the latter, by the way, does not have anything\n> >> to do with helping broken parsers. We drop the header after\n> >> re-coding the log message into an encoding specified by the user\n> >> (which is presumably different from what the commit was\n> >> originally recorded in) because the encoding recorded on\n> >> \"encoding\" header would not match the re-coded log message\n> >> anymore.\n> >\n> > By the same reasoning, you'd have to rewrite the committer line to reflect \n> > the current GIT_COMMITTER_IDENT, or hide it. If you want to convince me, \n> > you have to try harder.\n> \n> Sorry, but you completely lost me with that analogy.\n> \n> I think showing log message in the user's preferred encoding is\n> more like passing the output to the colorization mechanism and\n> then to the pager. We are interacting with humans at that\n> point, and we are changing the presentation without changing the\n> semantics of the data. \n> \n> I do not see why committer identity needs to be rewritten nor\n> hidden by the same reasoning.\n> \n> > And Marco has to fix the header parsing anyway.\n> \n> No question about that. If iconv() punts, qgit will see\n> \"encoding\" header to deal with even when the re-coding is in\n> effect. I think it may be a sensible thing for qgit to replace\n> the log message and show \"log message in this encoding, which\n> cannot be shown in this window\" instead in such a case, but that\n> is up to Porcelain.\n\nAh! Now I get your reasoning. But it is wrong. You are misusing headers -- \nwhich should be metadata describing the commit -- to pass a vital \ninformation to a porcelain: \"re-encoding failed, please have a try \nyourself\".\n\nBut the encoding header describes a certain aspect of the commit object: \nhow it was _originally_ encoded.\n\nIt's just like mails: often I look at the X-Mailer: header to find out if \nthe sender by any chance used Windows, or Mac, because it may help me help \nthe sender when incomplete information was provided.\n\nAnd just like with the mail, I would not like the tool to _hide_ the \nheaders from me when I ask for them, just because it happened to use them \nalready to display the characters correctly.\n\nCiao,\nDscho\n"},{"id":"30716","messageId":"e5bfff550701021429j1587949fgfe19effcaf74e6df@mail.gmail.com","threadId":"6155","inReplyTo":"7vps9xrs9y.fsf@assigned-by-dhcp.cox.net","subject":"Re: Possible regression in git-rev-list --header","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-01-02T22:29:14Z","receivedAt":"2007-01-02T22:29:14Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 1/2/07, Junio C Hamano wrote:\n>\n> > And Marco has to fix the header parsing anyway.\n>\n> No question about that. If iconv() punts, qgit will see\n> \"encoding\" header to deal with even when the re-coding is in\n> effect. I think it may be a sensible thing for qgit to replace\n> the log message and show \"log message in this encoding, which\n> cannot be shown in this window\" instead in such a case, but that\n> is up to Porcelain.\n>\n\nYes, git-rev-list output is stored in memory as a big chunk of row\nbytes, conversion\nto utf8 (the internal format used by Qt string class) is done _only_\nfor displayed items, just\nbefore to show them to user so is not performance critical. I agree\nwith Junio, that info could be used to teach Qt string conversion\nfunction how to handle the data. Something like:\n\n convertWithCodec(const char* data, codec* codecName)\n\nSo perhaps if Qt is better then iconv we could display a good log\nmessage anyway.\n"},{"id":"30734","messageId":"e5bfff550701030121n700fab25x63278457c884a3f7@mail.gmail.com","threadId":"6155","inReplyTo":"7vodpja0u8.fsf@assigned-by-dhcp.cox.net","subject":"Re: Possible regression in git-rev-list --header","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-01-03T09:21:21Z","receivedAt":"2007-01-03T09:21:21Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 1/1/07, Junio C Hamano wrote:\n> \"Marco Costalba\" writes:\n>\n> > On 12/31/06, Johannes Schindelin wrote:\n> >>\n> >> Further, if you rely on parsing being super-fast, why not just parse\n> >> _only_ the header information that you actually need? The header still\n> >> consists of\n> >>\n> >> - exactly one \"tree\",\n> >> - an arbitrary amount of \"parent\" lines,\n> >> - exactly one \"author\", and\n> >> - exactly one \"committer\" line\n> >>\n> >> After that may come optional headers,\n>\n> They are more like 'other' headers. Nobody said the set of\n> headers are cast in stone forever. The only things parsers\n> safely can assume are that the original four kinds come at the\n> beginning in the above order, and there is a blank line that\n> separates headers and the body.\n>\n\nI'm cooking the qgit parser fix, please confirm the following\nassumption is correct:\n\n\tWhen git-rev-list is called with --header option, after the first\n\tline with the commit sha, the following information is produced\n\t\n\t- one line with \"tree\"\n\t- an arbitrary amount of \"parent\" lines\n\t- one line with \"author\"\n\t- one line with \"committer\"\n\t- zero or more *non blank* lines with other info, as the encoding\n\t- one blank line\n\t- zero or one line with log title\n\t- zero or more lines with log message\n\t- a terminating '\\0'\n\n\nThanks\nMarco\n"},{"id":"30735","messageId":"7vodpgpgj0.fsf@assigned-by-dhcp.cox.net","threadId":"6155","inReplyTo":"e5bfff550701030121n700fab25x63278457c884a3f7@mail.gmail.com","subject":"Re: Possible regression in git-rev-list --header","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-01-03T10:10:27Z","receivedAt":"2007-01-03T10:10:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> On 1/1/07, Junio C Hamano wrote:\n>> \"Marco Costalba\" writes:\n>>\n>> > On 12/31/06, Johannes Schindelin wrote:\n>> >>\n>> >> Further, if you rely on parsing being super-fast, why not just parse\n>> >> _only_ the header information that you actually need? The header still\n>> >> consists of\n>> >>\n>> >> - exactly one \"tree\",\n>> >> - an arbitrary amount of \"parent\" lines,\n>> >> - exactly one \"author\", and\n>> >> - exactly one \"committer\" line\n>> >>\n>> >> After that may come optional headers,\n>>\n>> They are more like 'other' headers. Nobody said the set of\n>> headers are cast in stone forever. The only things parsers\n>> safely can assume are that the original four kinds come at the\n>> beginning in the above order, and there is a blank line that\n>> separates headers and the body.\n>>\n>\n> I'm cooking the qgit parser fix, please confirm the following\n> assumption is correct:\n>\n> \tWhen git-rev-list is called with --header option, after the first\n> \tline with the commit sha, the following information is produced\n> \t\n> \t- one line with \"tree\"\n> \t- an arbitrary amount of \"parent\" lines\n> \t- one line with \"author\"\n> \t- one line with \"committer\"\n> \t- zero or more *non blank* lines with other info, as the encoding\n> \t- one blank line\n> \t- zero or one line with log title\n> \t- zero or more lines with log message\n> \t- a terminating '\\0'\n\nI think that is correct, except that rev-list does not even care\nthe distinction between log title and log message.\n\nI do not think of a reason to change the order of the first four\noffhand, so while it might be prudent to prepare the code to\naccept them in any order if it is not too much trouble from\npurist point of view, I do not think it is necessary in practice\nand you should be able to depend on the order in which they\nappear. I do not think it would hurt anybody if we right now\ndeclared that other extra headers people have talked about (such\nas \"notes\") should always come after the first four.\n"},{"id":"30736","messageId":"8c5c35580701030221w5c04e2eaq757d12c4ca0b0780@mail.gmail.com","threadId":"6155","inReplyTo":"e5bfff550701030121n700fab25x63278457c884a3f7@mail.gmail.com","subject":"Re: Possible regression in git-rev-list --header","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2007-01-03T10:21:33Z","receivedAt":"2007-01-03T10:21:33Z","isPatch":false,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"On 1/3/07, Marco Costalba wrote:\n> - one blank line\n> - zero or one line with log title\n> - zero or more lines with log message\n> - a terminating '\\0'\n\nI think the should be:\n -zero or more blank lines\n -zero or more non-blank lines with log title\n -zero or more blank lines\n -zero or more lines with log message\n -a terminating '\\0'\n\n...which implies that all of these rules (except the last) are\noptional and only triggers if the _previous_ rule triggered.\n\nIn other words:\n\n commit ::= header [blank+ [title+ [blank+ text*]]] '\\0'\n header ::= tree parent* author committer\n\n-- \nlarsh\n"},{"id":"30738","messageId":"e5bfff550701030235g6f26044h8dd73e8abe1ec9e3@mail.gmail.com","threadId":"6155","inReplyTo":"8c5c35580701030221w5c04e2eaq757d12c4ca0b0780@mail.gmail.com","subject":"Re: Possible regression in git-rev-list --header","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-01-03T10:35:17Z","receivedAt":"2007-01-03T10:35:17Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 1/3/07, Lars Hjemli wrote:\n> On 1/3/07, Marco Costalba wrote:\n> > - one blank line\n> > - zero or one line with log title\n> > - zero or more lines with log message\n> > - a terminating '\\0'\n>\n> I think the should be:\n> -zero or more blank lines\n\nIsn't it zero or _one_ blank line? Why more then one? would be it\nuseful? surely is slower to parse.\n\n> -zero or more non-blank lines with log title\n\nmulti lines titles are allowed? never saw one of them.\n\n> -zero or more blank lines\n\nWhy? this is necessary only to disambiguate muti (non blank) lines\ntitles, but as Junio pointed out distinction between log title and log\nmessage is only in the Porcelain, not encoded in git. So the Porcalain\nis going to show _one_line title if any an the remaining stuff in the\nlog message.\n\n> -zero or more lines with log message\n> -a terminating '\\0'\n>\n\n Marco\n"},{"id":"30739","messageId":"8c5c35580701030238t651af0f3kcff7600768dda722@mail.gmail.com","threadId":"6155","inReplyTo":"8c5c35580701030221w5c04e2eaq757d12c4ca0b0780@mail.gmail.com","subject":"Re: Possible regression in git-rev-list --header","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2007-01-03T10:38:07Z","receivedAt":"2007-01-03T10:38:07Z","isPatch":false,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"On 1/3/07, Lars Hjemli wrote:\n> commit ::= header [blank+ [title+ [blank+ text*]]] '\\0'\n> header ::= tree parent* author committer\n\nSorry, this should have been\n\n commit ::= header [blank+ [title+ [blank+ text*]]] '\\0'\n header ::= tree parent* author committer other_header*\n\n-- \nlarsh\n"},{"id":"30741","messageId":"8c5c35580701030314t69b6a7dbhf9cb99de4567b93e@mail.gmail.com","threadId":"6155","inReplyTo":"e5bfff550701030235g6f26044h8dd73e8abe1ec9e3@mail.gmail.com","subject":"Re: Possible regression in git-rev-list --header","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2007-01-03T11:14:10Z","receivedAt":"2007-01-03T11:14:10Z","isPatch":false,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"On 1/3/07, Marco Costalba wrote:\n> On 1/3/07, Lars Hjemli wrote:\n> > On 1/3/07, Marco Costalba wrote:\n> > > - one blank line\n> > > - zero or one line with log title\n> > > - zero or more lines with log message\n> > > - a terminating '\\0'\n> >\n> > I think the should be:\n> > -zero or more blank lines\n>\n> Isn't it zero or _one_ blank line? Why more then one? would be it\n> useful? surely is slower to parse.\n\nMy point is just that the log message is unformatted. There is nothing\nstopping you from\nadding blank lines at the start of the message.\n\nBut then I tested it, and it seems as 'git-commit' strips off any\nleading blank lines (but git-commit-tree does not). So expecting one\nblank _might_ be good enough.\n\n>\n> > -zero or more non-blank lines with log title\n>\n> multi lines titles are allowed? never saw one of them.\n\nIt is allowed (I tested this)\n\n>\n> > -zero or more blank lines\n>\n> Why? this is necessary only to disambiguate muti (non blank) lines\n> titles, but as Junio pointed out distinction between log title and log\n> message is only in the Porcelain, not encoded in git. So the Porcalain\n> is going to show _one_line title if any an the remaining stuff in the\n> log message.\n\nWell, if the porcelain chooses to do so, it probably is ok. But I\nthink the parsing/presentation of the log title/message would be more\ncorrect/better formatted if the parser is more \"adaptive\" to the\ncontent.\n\n-- \nlarsh\n"},{"id":"30814","messageId":"459D1ABF.8010500@op5.se","threadId":"6155","inReplyTo":"e5bfff550701030235g6f26044h8dd73e8abe1ec9e3@mail.gmail.com","subject":"Re: Possible regression in git-rev-list --header","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-01-04T15:18:23Z","receivedAt":"2007-01-04T15:18:23Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Marco Costalba wrote:\n> On 1/3/07, Lars Hjemli wrote:\n>> On 1/3/07, Marco Costalba wrote:\n>> > - one blank line\n>> > - zero or one line with log title\n>> > - zero or more lines with log message\n>> > - a terminating '\\0'\n>>\n>> I think the should be:\n>> -zero or more blank lines\n> \n> Isn't it zero or _one_ blank line? Why more then one? would be it\n> useful? surely is slower to parse.\n> \n\nI've imported a repository from CVS to git (the nagiosplugin repo), \nwhere one author consistently put an empty line before the \"commit \nsubject\". I'm fairly certain this is done by the tool he uses (some GUI \nthing, no doubt).\n\nI'm currently looking into stripping optional empty lines appearing \npostheaders-prebody from rev-list output, as it completely fudges qgit \nand gitk viewing alike. Good thing the author in question only does \ntranslations and no code work, or I'd be screaming with frustration \ntrying to figure out which commit does what. Mind you, CVS's lack of \nforensic tools doesn't exactly inspire the clear and concise commit \nmessages we see in git, but still...\n\n\n>> -zero or more non-blank lines with log title\n> \n> multi lines titles are allowed? never saw one of them.\n> \n\nThey aren't disallowed. The log message part only by convention consists \nof a title line followed by a blank and then the message body. It's \nentirely possible to just start writing the message straight off, or put \nany number of blank lines wherever.\n\n>> -zero or more blank lines\n> \n> Why? this is necessary only to disambiguate muti (non blank) lines\n> titles, but as Junio pointed out distinction between log title and log\n> message is only in the Porcelain, not encoded in git. So the Porcalain\n> is going to show _one_line title if any an the remaining stuff in the\n> log message.\n> \n\nSensible. There's no (reliable) way of knowing if the first line with \ntext on is actually a title or just the start of a 25-line sentence, or \neven the signoff line, or completely empty.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"30815","messageId":"459D1B60.1050604@op5.se","threadId":"6155","inReplyTo":"8c5c35580701030314t69b6a7dbhf9cb99de4567b93e@mail.gmail.com","subject":"Re: Possible regression in git-rev-list --header","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-01-04T15:21:04Z","receivedAt":"2007-01-04T15:21:04Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Lars Hjemli wrote:\n> On 1/3/07, Marco Costalba wrote:\n>> On 1/3/07, Lars Hjemli wrote:\n>> > On 1/3/07, Marco Costalba wrote:\n>> > > - one blank line\n>> > > - zero or one line with log title\n>> > > - zero or more lines with log message\n>> > > - a terminating '\\0'\n>> >\n>> > I think the should be:\n>> > -zero or more blank lines\n>>\n>> Isn't it zero or _one_ blank line? Why more then one? would be it\n>> useful? surely is slower to parse.\n> \n> My point is just that the log message is unformatted. There is nothing\n> stopping you from\n> adding blank lines at the start of the message.\n> \n> But then I tested it, and it seems as 'git-commit' strips off any\n> leading blank lines (but git-commit-tree does not). So expecting one\n> blank _might_ be good enough.\n> \n\nOther scm's from which we can import repositories have no such \nmechanisms though. It would be nice to have this functionality in \nrev-list. I'm looking into it at the moment.\n\n> \n>>\n>> > -zero or more blank lines\n>>\n>> Why? this is necessary only to disambiguate muti (non blank) lines\n>> titles, but as Junio pointed out distinction between log title and log\n>> message is only in the Porcelain, not encoded in git. So the Porcalain\n>> is going to show _one_line title if any an the remaining stuff in the\n>> log message.\n> \n> Well, if the porcelain chooses to do so, it probably is ok. But I\n> think the parsing/presentation of the log title/message would be more\n> correct/better formatted if the parser is more \"adaptive\" to the\n> content.\n> \n\nI disagree. If the title spans over more than one line, it's most likely \nbecause it's too long to fit in one, in which case it won't look good \n(or even be viewable in qgit / gitk) when printed anyways.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"}]}