{"thread":{"id":"35141","subject":"[PATCH] split_ident: parse timestamp from end of line","startedAt":"2013-10-14T20:27:34Z","lastAt":"2013-10-15T18:48:47Z","messageCount":9,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"228945","messageId":"20131014202734.GA7007@sigill.intra.peff.net","threadId":"35141","inReplyTo":null,"subject":"[PATCH] split_ident: parse timestamp from end of line","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-14T20:27:34Z","receivedAt":"2013-10-14T20:27:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Split_ident currently parses left to right. Given this\ninput:\n\n Your Name 123456789 -0500\\n\n\nWe assume the name starts the line and runs until the first\n\"<\". That starts the email address, which runs until the\nfirst \">\". Everything after that is assumed to be the\ntimestamp.\n\nThis works fine in the normal case, but is easily broken by\ncorrupted ident lines that contain an extra \">\". Some\nexamples seen in the wild are:\n\n 1. Name -<> 123456789 -0500\\n\n\n 2. Name > 123456789 -0500\\n\n\n 3. Name1 , Name2 123456789 -0500\\n\n\nCurrently each of these produces some email address (which\nis not necessarily the one the user intended) and end up\nwith a NULL date (which is generally interpreted as the\nepoch by \"git log\" and friends).\n\nBut in each case we could get the correct timestamp simply\nby parsing from the right-hand side, looking backwards for\nthe final \">\", and then reading the timestamp from there.\n\nIn general, it's a losing battle to try to automatically\nguess what the user meant with their broken crud. But this\nparticular workaround is probably worth doing. One, it's\ndirt simple, and can't impact non-broken cases. Two, it\ndoesn't catch a single breakage we've seen, but rather a\nlarge class of errors (i.e., any breakage inside the email\nangle brackets may affect the email, but won't spill over\ninto the timestamp parsing). And three, the timestamp is\narguably more valuable to get right, because it can affect\ncorrectness (e.g., in --until cutoffs).\n\nThis patch implements the right-to-left scheme described\nabove. We adjust the tests in t4212, which generate a commit\nwith such a broken ident, and now gets the timestamp right.\nWe also add a test that fsck continues to detect the\nbreakage.\n\nFor reference, here are pointers to the breakages seen (as\nnumbered above):\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/221441\n\n[2] http://article.gmane.org/gmane.comp.version-control.git/222362\n\n[3] http://perl5.git.perl.org/perl.git/commit/13b79730adea97e660de84bbe67f9d7cbe344302\n\nSigned-off-by: Jeff King \n---\nYou could take this concept further and try to do something clever with\nthe email when we notice the extra \">\". But I think that is where this\ncrosses from \"easily and simply covers a class of errors\" into \"losing\nproposition trying to tweak heuristics around various breakages\".\n\nThe only thing that gives me pause here is that parsing from the right\nwould close the door to ever adding any new information on the end of an\nident line. I'd be surprised if that door wasn't already closed by the\nexisting parsers, but I feel like the topic might have come up sometime\nin the past year or two (but I can't seem to find anything in the\narchive).\n\n ident.c | 14 +++++++++++++-\n t/t4212-log-corrupt.sh | 9 +++++++--\n 2 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 1c123e6..1a4b3ad 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -233,7 +233,19 @@ int split_ident_line(struct ident_split *split, const char *line, int len)\n \tif (!split->mail_end)\n \t\treturn status;\n \n-\tfor (cp = split->mail_end + 1; cp < line + len && isspace(*cp); cp++)\n+\t/*\n+\t * Look from the end-of-line to find the trailing \">\" of the mail\n+\t * address, even though we should already know it as split->mail_end.\n+\t * This can help in cases of broken idents with an extra \">\" somewhere\n+\t * in the email address. Note that we are assuming the timestamp will\n+\t * never have a \">\" in it.\n+\t *\n+\t * Note also that this memchr can never return NULL, as we would\n+\t * always find at least the split->mail_end closing bracket.\n+\t */\n+\tcp = memrchr(split->mail_end, '>', len - (split->mail_end - line));\n+\n+\tfor (cp = cp + 1; cp < line + len && isspace(*cp); cp++)\n \t\t;\n \tif (line + len <= cp)\n \t\tgoto person_only;\ndiff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh\nindex ec5099b..93c7c36 100755\n--- a/t/t4212-log-corrupt.sh\n+++ b/t/t4212-log-corrupt.sh\n@@ -13,11 +13,16 @@ test_expect_success 'git log with broken author email' '\n \tgit update-ref refs/heads/broken_email $(cat broken_email.hash)\n '\n \n+test_expect_success 'fsck notices broken commit' '\n+\tgit fsck 2>actual &&\n+\ttest_i18ngrep invalid.author actual\n+'\n+\n test_expect_success 'git log with broken author email' '\n \t{\n \t\techo commit $(cat broken_email.hash)\n \t\techo \"Author: A U Thor \"\n-\t\techo \"Date: Thu Jan 1 00:00:00 1970 +0000\"\n+\t\techo \"Date: Thu Apr 7 15:13:13 2005 -0700\"\n \t\techo\n \t\techo \" foo\"\n \t} >expect.out &&\n@@ -30,7 +35,7 @@ test_expect_success 'git log --format with broken author email' '\n '\n \n test_expect_success 'git log --format with broken author email' '\n-\techo \"A U Thor+author@example.com+\" >expect.out &&\n+\techo \"A U Thor+author@example.com+Thu Apr 7 15:13:13 2005 -0700\" >expect.out &&\n \t: >expect.err &&\n \n \tgit log --format=\"%an+%ae+%ad\" broken_email >actual.out 2>actual.err &&\n-- \n1.8.4.1.4.gf327177\n"},{"id":"228951","messageId":"xmqqwqlfebhi.fsf@gitster.dls.corp.google.com","threadId":"35141","inReplyTo":"20131014202734.GA7007@sigill.intra.peff.net","subject":"Re: [PATCH] split_ident: parse timestamp from end of line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-14T22:25:29Z","receivedAt":"2013-10-14T22:25:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> You could take this concept further and try to do something clever with\n> the email when we notice the extra \">\". But I think that is where this\n> crosses from \"easily and simply covers a class of errors\" into \"losing\n> proposition trying to tweak heuristics around various breakages\".\n\nTrue.\n\n> The only thing that gives me pause here is that parsing from the right\n> would close the door to ever adding any new information on the end of an\n> ident line. I'd be surprised if that door wasn't already closed by the\n> existing parsers, but I feel like the topic might have come up sometime\n> in the past year or two (but I can't seem to find anything in the\n> archive).\n\nI do not recall any, either.\n\nThe approach to parse from the right-end feels like the simplest and\nthe clearest one to get the piece of information that matters in the\npresence of breakages like the ones you mentioned.\n\n> +\t/*\n> +\t * Look from the end-of-line to find the trailing \">\" of the mail\n> +\t * address, even though we should already know it as split->mail_end.\n> +\t * This can help in cases of broken idents with an extra \">\" somewhere\n> +\t * in the email address. Note that we are assuming the timestamp will\n> +\t * never have a \">\" in it.\n> +\t *\n> +\t * Note also that this memchr can never return NULL, as we would\n> +\t * always find at least the split->mail_end closing bracket.\n> +\t */\n> +\tcp = memrchr(split->mail_end, '>', len - (split->mail_end - line));\n> +\tfor (cp = cp + 1; cp < line + len && isspace(*cp); cp++)\n> \t\t;\n\n\"git grep\" tells me this is the first use of memrchr(), which,\nunlike memchr(), is _GNU_SOURCE-only if I am not mistaken, so we may\nneed a fallback definition in the compat/ and NEEDS_MEMRCHR in the\nMakefile, I think.\n"},{"id":"228953","messageId":"20131014223137.GA12744@sigill.intra.peff.net","threadId":"35141","inReplyTo":"xmqqwqlfebhi.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] split_ident: parse timestamp from end of line","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-14T22:31:37Z","receivedAt":"2013-10-14T22:31:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 14, 2013 at 03:25:29PM -0700, Junio C Hamano wrote:\n\n> > +\t/*\n> > +\t * Look from the end-of-line to find the trailing \">\" of the mail\n> > +\t * address, even though we should already know it as split->mail_end.\n> > +\t * This can help in cases of broken idents with an extra \">\" somewhere\n> > +\t * in the email address. Note that we are assuming the timestamp will\n> > +\t * never have a \">\" in it.\n> > +\t *\n> > +\t * Note also that this memchr can never return NULL, as we would\n> > +\t * always find at least the split->mail_end closing bracket.\n> > +\t */\n> > +\tcp = memrchr(split->mail_end, '>', len - (split->mail_end - line));\n> > +\tfor (cp = cp + 1; cp < line + len && isspace(*cp); cp++)\n> > \t\t;\n> \n> \"git grep\" tells me this is the first use of memrchr(), which,\n> unlike memchr(), is _GNU_SOURCE-only if I am not mistaken, so we may\n> need a fallback definition in the compat/ and NEEDS_MEMRCHR in the\n> Makefile, I think.\n\nYeah, you are right[1]. I'm happy to re-roll. I wonder if we even need\nto worry about a compatibility wrapper. We are already doing pointer\nmanipulations, and it is probably just as readable to roll the loop by\nhand.\n\n-Peff\n\n[1] I even looked at \"man memrchr\" on my glibc system and was surprised\n to see it mentioned above the \"#define _GNU_SOURCE\" fold. But that\n \"fold\" is used only sometimes (e.g., strchrnul), and not others (in\n memrchr, the portability bits are listed at the end of the\n synopsis). Grr.\n"},{"id":"228970","messageId":"20131014224500.GA10279@sigill.intra.peff.net","threadId":"35141","inReplyTo":"20131014223137.GA12744@sigill.intra.peff.net","subject":"Re: [PATCH] split_ident: parse timestamp from end of line","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-14T22:45:00Z","receivedAt":"2013-10-14T22:45:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 14, 2013 at 06:31:37PM -0400, Jeff King wrote:\n\n> > \"git grep\" tells me this is the first use of memrchr(), which,\n> > unlike memchr(), is _GNU_SOURCE-only if I am not mistaken, so we may\n> > need a fallback definition in the compat/ and NEEDS_MEMRCHR in the\n> > Makefile, I think.\n> \n> Yeah, you are right[1]. I'm happy to re-roll. I wonder if we even need\n> to worry about a compatibility wrapper. We are already doing pointer\n> manipulations, and it is probably just as readable to roll the loop by\n> hand.\n\nHere is that re-roll, which does:\n\n -\tcp = memrchr(split->mail_end, '>', len - (split->mail_end - line));\n +\tfor (cp = line + len - 1; *cp != '>'; cp--)\n +\t\t;\n\n-- >8 --\nSubject: split_ident: parse timestamp from end of line\n\nSplit_ident currently parses left to right. Given this\ninput:\n\n Your Name 123456789 -0500\\n\n\nWe assume the name starts the line and runs until the first\n\"<\". That starts the email address, which runs until the\nfirst \">\". Everything after that is assumed to be the\ntimestamp.\n\nThis works fine in the normal case, but is easily broken by\ncorrupted ident lines that contain an extra \">\". Some\nexamples seen in the wild are:\n\n 1. Name -<> 123456789 -0500\\n\n\n 2. Name > 123456789 -0500\\n\n\n 3. Name1 , Name2 123456789 -0500\\n\n\nCurrently each of these produces some email address (which\nis not necessarily the one the user intended) and end up\nwith a NULL date (which is generally interpreted as the\nepoch by \"git log\" and friends).\n\nBut in each case we could get the correct timestamp simply\nby parsing from the right-hand side, looking backwards for\nthe final \">\", and then reading the timestamp from there.\n\nIn general, it's a losing battle to try to automatically\nguess what the user meant with their broken crud. But this\nparticular workaround is probably worth doing. One, it's\ndirt simple, and can't impact non-broken cases. Two, it\ndoesn't catch a single breakage we've seen, but rather a\nlarge class of errors (i.e., any breakage inside the email\nangle brackets may affect the email, but won't spill over\ninto the timestamp parsing). And three, the timestamp is\narguably more valuable to get right, because it can affect\ncorrectness (e.g., in --until cutoffs).\n\nThis patch implements the right-to-left scheme described\nabove. We adjust the tests in t4212, which generate a commit\nwith such a broken ident, and now gets the timestamp right.\nWe also add a test that fsck continues to detect the\nbreakage.\n\nFor reference, here are pointers to the breakages seen (as\nnumbered above):\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/221441\n\n[2] http://article.gmane.org/gmane.comp.version-control.git/222362\n\n[3] http://perl5.git.perl.org/perl.git/commit/13b79730adea97e660de84bbe67f9d7cbe344302\n\nSigned-off-by: Jeff King \n---\n ident.c | 16 +++++++++++++++-\n t/t4212-log-corrupt.sh | 9 +++++++--\n 2 files changed, 22 insertions(+), 3 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 1c123e6..7d1c79c 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -233,7 +233,21 @@ int split_ident_line(struct ident_split *split, const char *line, int len)\n \tif (!split->mail_end)\n \t\treturn status;\n \n-\tfor (cp = split->mail_end + 1; cp < line + len && isspace(*cp); cp++)\n+\t/*\n+\t * Look from the end-of-line to find the trailing \">\" of the mail\n+\t * address, even though we should already know it as split->mail_end.\n+\t * This can help in cases of broken idents with an extra \">\" somewhere\n+\t * in the email address. Note that we are assuming the timestamp will\n+\t * never have a \">\" in it.\n+\t *\n+\t * Note that we will always find some \">\" before going off the front of\n+\t * the string, because will always hit the split->mail_end closing\n+\t * bracket.\n+\t */\n+\tfor (cp = line + len - 1; *cp != '>'; cp--)\n+\t\t;\n+\n+\tfor (cp = cp + 1; cp < line + len && isspace(*cp); cp++)\n \t\t;\n \tif (line + len <= cp)\n \t\tgoto person_only;\ndiff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh\nindex ec5099b..93c7c36 100755\n--- a/t/t4212-log-corrupt.sh\n+++ b/t/t4212-log-corrupt.sh\n@@ -13,11 +13,16 @@ test_expect_success 'git log with broken author email' '\n \tgit update-ref refs/heads/broken_email $(cat broken_email.hash)\n '\n \n+test_expect_success 'fsck notices broken commit' '\n+\tgit fsck 2>actual &&\n+\ttest_i18ngrep invalid.author actual\n+'\n+\n test_expect_success 'git log with broken author email' '\n \t{\n \t\techo commit $(cat broken_email.hash)\n \t\techo \"Author: A U Thor \"\n-\t\techo \"Date: Thu Jan 1 00:00:00 1970 +0000\"\n+\t\techo \"Date: Thu Apr 7 15:13:13 2005 -0700\"\n \t\techo\n \t\techo \" foo\"\n \t} >expect.out &&\n@@ -30,7 +35,7 @@ test_expect_success 'git log --format with broken author email' '\n '\n \n test_expect_success 'git log --format with broken author email' '\n-\techo \"A U Thor+author@example.com+\" >expect.out &&\n+\techo \"A U Thor+author@example.com+Thu Apr 7 15:13:13 2005 -0700\" >expect.out &&\n \t: >expect.err &&\n \n \tgit log --format=\"%an+%ae+%ad\" broken_email >actual.out 2>actual.err &&\n-- \n1.8.4.1.4.gf327177\n"},{"id":"228971","messageId":"xmqqsiw3eajt.fsf@gitster.dls.corp.google.com","threadId":"35141","inReplyTo":"20131014223137.GA12744@sigill.intra.peff.net","subject":"Re: [PATCH] split_ident: parse timestamp from end of line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-14T22:45:42Z","receivedAt":"2013-10-14T22:45:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Yeah, you are right[1]. I'm happy to re-roll. I wonder if we even need\n> to worry about a compatibility wrapper. We are already doing pointer\n> manipulations, and it is probably just as readable to roll the loop by\n> hand.\n\nYeah, unrolling the loop is probably better. You may even be able\nto do so in a single pass with an extra \"last > seen\" pointer\nvariable without too much additional code complexity, I would think.\n"},{"id":"228976","messageId":"20131014232949.GA10415@sigill.intra.peff.net","threadId":"35141","inReplyTo":"xmqqsiw3eajt.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] split_ident: parse timestamp from end of line","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-14T23:29:49Z","receivedAt":"2013-10-14T23:29:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 14, 2013 at 03:45:42PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Yeah, you are right[1]. I'm happy to re-roll. I wonder if we even need\n> > to worry about a compatibility wrapper. We are already doing pointer\n> > manipulations, and it is probably just as readable to roll the loop by\n> > hand.\n> \n> Yeah, unrolling the loop is probably better. You may even be able\n> to do so in a single pass with an extra \"last > seen\" pointer\n> variable without too much additional code complexity, I would think.\n\nI'm not sure what you mean here.\n\nIf you mean doing a single pass to find the final \">\", that is easy,\nbecause we know the length of the line already and can jump past and\nstart from the back.\n\nIf you mean rolling it into the loop directly below, where we jump past\nthe whitespace, I think it's a bit more complicated. We would not want\nto stop when we see something date-like, because parsing:\n\n Name < 5678> 1234 -0500\n\nyou would want to find \"1234\" as the date. You can, while you are\nscanning right, keep track of the end of the whitespace after \">\", but I\ndo not think the complication is worth much. There should typically only\nbe one space, so you are only saving looking at a single character.\n\n-Peff\n"},{"id":"229015","messageId":"xmqqob6qe808.fsf@gitster.dls.corp.google.com","threadId":"35141","inReplyTo":"20131014232949.GA10415@sigill.intra.peff.net","subject":"Re: [PATCH] split_ident: parse timestamp from end of line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-15T17:52:55Z","receivedAt":"2013-10-15T17:52:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> Yeah, unrolling the loop is probably better. You may even be able\n>> to do so in a single pass with an extra \"last > seen\" pointer\n>> variable without too much additional code complexity, I would think.\n>\n> I'm not sure what you mean here.\n\n> If you mean doing a single pass to find the final \">\", that is easy,\n> because we know the length of the line already and can jump past and\n> start from the back.\n\nI meant a single forward pass, like this.\n\n ident.c | 29 +++++++++++------------------\n 1 file changed, 11 insertions(+), 18 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 7d1c79c..ff29779 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -200,7 +200,7 @@ static void strbuf_addstr_without_crud(struct strbuf *sb, const char *src)\n */\n int split_ident_line(struct ident_split *split, const char *line, int len)\n {\n-\tconst char *cp;\n+\tconst char *cp, *last_ket;\n \tsize_t span;\n \tint status = -1;\n \n@@ -225,29 +225,22 @@ int split_ident_line(struct ident_split *split, const char *line, int len)\n \t\tsplit->name_end = split->name_begin;\n \t}\n \n-\tfor (cp = split->mail_begin; cp < line + len; cp++)\n-\t\tif (*cp == '>') {\n+\tfor (cp = split->mail_begin, last_ket = NULL; cp < line + len; cp++) {\n+\t\tif (*cp != '>')\n+\t\t\tcontinue;\n+\t\tif (!last_ket)\n \t\t\tsplit->mail_end = cp;\n-\t\t\tbreak;\n-\t\t}\n+\t\tlast_ket = cp;\n+\t}\n \tif (!split->mail_end)\n \t\treturn status;\n \n \t/*\n-\t * Look from the end-of-line to find the trailing \">\" of the mail\n-\t * address, even though we should already know it as split->mail_end.\n-\t * This can help in cases of broken idents with an extra \">\" somewhere\n-\t * in the email address. Note that we are assuming the timestamp will\n-\t * never have a \">\" in it.\n-\t *\n-\t * Note that we will always find some \">\" before going off the front of\n-\t * the string, because will always hit the split->mail_end closing\n-\t * bracket.\n+\t * Typically, last_ket is the same as split_mail_end, but with\n+\t * a broken identity line, there may be multiple closing ket '>';\n+\t * read the timestamp after the last one.\n \t */\n-\tfor (cp = line + len - 1; *cp != '>'; cp--)\n-\t\t;\n-\n-\tfor (cp = cp + 1; cp < line + len && isspace(*cp); cp++)\n+\tfor (cp = last_ket + 1; cp < line + len && isspace(*cp); cp++)\n \t\t;\n \tif (line + len <= cp)\n \t\tgoto person_only;\n"},{"id":"229016","messageId":"20131015180312.GA26845@sigill.intra.peff.net","threadId":"35141","inReplyTo":"xmqqob6qe808.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] split_ident: parse timestamp from end of line","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-15T18:03:12Z","receivedAt":"2013-10-15T18:03:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 15, 2013 at 10:52:55AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> >> Yeah, unrolling the loop is probably better. You may even be able\n> >> to do so in a single pass with an extra \"last > seen\" pointer\n> >> variable without too much additional code complexity, I would think.\n> >\n> > I'm not sure what you mean here.\n> \n> > If you mean doing a single pass to find the final \">\", that is easy,\n> > because we know the length of the line already and can jump past and\n> > start from the back.\n> \n> I meant a single forward pass, like this.\n\nAh, I see. You are combining with the pass before, not the pass after.\n\nI do not think this is any more (nor less) efficient than what I posted.\nWe still pass over the space after split->mail_end one additional time\nsearching for the closing bracket. Mine is _slightly_ more efficient in\nthat by going backwards we can stop when we see the first '>', avoiding\nlooking at the space between \"mail_end\" and \"last_ket\". But that space\nis 0 in the normal case, and even if it is not, we are talking about\ntens of bytes at most. So I doubt it would ever matter.\n\nMy version seems a little clearer to me, but that is probably because I\nwrote it. If you strongly prefer the other, feel free to mark up my\npatch.\n\n-Peff\n\nPS I learned a new term, \"ket\". I always called it \"closing angle\n bracket\".\n"},{"id":"229018","messageId":"xmqqeh7me5f4.fsf@gitster.dls.corp.google.com","threadId":"35141","inReplyTo":"20131015180312.GA26845@sigill.intra.peff.net","subject":"Re: [PATCH] split_ident: parse timestamp from end of line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-15T18:48:47Z","receivedAt":"2013-10-15T18:48:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> My version seems a little clearer to me, but that is probably because I\n> wrote it. If you strongly prefer the other, feel free to mark up my\n> patch.\n\nI do not have strong preference either way. Just that I thought two\nloops would be shorter and easier to understand than three, that's\nall.\n"}]}