Re: [PATCH] split_ident: parse timestamp from end of line
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 14, 2013, 22:25 UTC
- Message-ID
- <xmqqwqlfebhi.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <20131014202734.GA7007@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> You could take this concept further and try to do something clever with > the email when we notice the extra ">". But I think that is where this > crosses from "easily and simply covers a class of errors" into "losing > proposition trying to tweak heuristics around various breakages".
True.
Show 6 quoted lines
> The only thing that gives me pause here is that parsing from the right > would close the door to ever adding any new information on the end of an > ident line. I'd be surprised if that door wasn't already closed by the > existing parsers, but I feel like the topic might have come up sometime > in the past year or two (but I can't seem to find anything in the > archive).
I do not recall any, either.
The approach to parse from the right-end feels like the simplest and the clearest one to get the piece of information that matters in the presence of breakages like the ones you mentioned.
Show 13 quoted lines
> + /* > + * Look from the end-of-line to find the trailing ">" of the mail > + * address, even though we should already know it as split->mail_end. > + * This can help in cases of broken idents with an extra ">" somewhere > + * in the email address. Note that we are assuming the timestamp will > + * never have a ">" in it. > + * > + * Note also that this memchr can never return NULL, as we would > + * always find at least the split->mail_end closing bracket. > + */ > + cp = memrchr(split->mail_end, '>', len - (split->mail_end - line)); > + for (cp = cp + 1; cp < line + len && isspace(*cp); cp++) > ;
"git grep" tells me this is the first use of memrchr(), which, unlike memchr(), is _GNU_SOURCE-only if I am not mistaken, so we may need a fallback definition in the compat/ and NEEDS_MEMRCHR in the Makefile, I think.