Re: [PATCH] split_ident: parse timestamp from end of line

From: Jeff King <peff@peff.net>
Date: Oct 14, 2013, 23:29 UTC
Message-ID: <20131014232949.GA10415@sigill.intra.peff.net>
In-Reply-To: <xmqqsiw3eajt.fsf@gitster.dls.corp.google.com>

On Mon, Oct 14, 2013 at 03:45:42PM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Yeah, you are right[1]. I'm happy to re-roll. I wonder if we even need
> > to worry about a compatibility wrapper. We are already doing pointer
> > manipulations, and it is probably just as readable to roll the loop by
> > hand.
> 
> Yeah, unrolling the loop is probably better.  You may even be able
> to do so in a single pass with an extra "last > seen" pointer
> variable without too much additional code complexity, I would think.

I'm not sure what you mean here.

If you mean doing a single pass to find the final ">", that is easy, because we know the length of the line already and can jump past and start from the back.

If you mean rolling it into the loop directly below, where we jump past the whitespace, I think it's a bit more complicated. We would not want to stop when we see something date-like, because parsing:

  Name <<bogus.email> 5678> 1234 -0500

you would want to find "1234" as the date. You can, while you are scanning right, keep track of the end of the whitespace after ">", but I do not think the complication is worth much. There should typically only be one space, so you are only saving looking at a single character.

-Peff