Re: [PATCH 13/17] refs: convert each_reflog_ent_fn to struct object_id

From: Jeff King <peff@peff.net>
Date: Jan 3, 2017, 01:32 UTC
Message-ID: <20170103013217.vctawyhpsd73frjc@sigill.intra.peff.net>
In-Reply-To: <d5fc2830-37c2-9274-77b7-97ecc5f9b763@alum.mit.edu>

On Tue, Jan 03, 2017 at 12:30:40AM +0100, Michael Haggerty wrote:

Show 16 quoted lines

> > I think
> > my next series is going to include a small sscanf-style parser to parse
> > these.  Right now, using constants here is going to leave it extremely
> > difficult to read.  Something like the following for the OIDs:
> > 
> >   strbuf_git_scanf(sb, "%h %h ", &ooid, &noid);
> > 
> > and then following up parsing the remainder.
> 
> Maybe something with an interface like skip_prefix wouldn't be too
> obnoxious:
> 
>     const char *p = sb.buf;
>     if (oid_prefix(p, &ooid, &p) &&
>         *p++ == ' ' &&
>         oid_prefix(p, &noid, &p) && ...

Yeah, I've used C code before that had a very similar interface for parsing, and when used consistently it's pretty pleasant. Something like:

  if (parse_oid(p, &oid, &p) &&
      skip_whitespace(p, &p) &&
      parse_refname(p, &refname, &p))

etc is nicer than some of the magic numbers we end up using in the various parsers (I don't think anybody needs to mass-convert, I just mean that something like parse_oid() seems like a step in a good direction).

-Peff