Re: [PATCH v2 08/15] config: introduce an optional event stream while parsing

[Jeff King <peff@peff.net>]

On Tue, Apr 03, 2018 at 06:28:29PM +0200, Johannes Schindelin wrote:

> This extends our config parser so that it can optionally produce an event
> stream via callback function, where it reports e.g. when a comment was
> parsed, or a section header, etc.
> 
> This parser will be used subsequently to handle the scenarios better where
> removing config entries would make sections empty, or where a new entry
> could be added to an already-existing, empty section.

Nice, it looks like this didn't end up being too bad to go in this
direction. It seems like this is an optional "also emit the events here"
function you can set. I think in the long run we could actually just
always emit the events to this function. And then we could wrap that to
provide an interface that matches the existing callbacks (just an
event-stream callback that sees EVENT_ENTRY and calls the sub-callback).

But that might end up quite a pain, since we have a zillion entry points
into the config parser, making wrapping tough. So I'm perfectly happy to
stop here for now.

> +static inline int do_event(enum config_event_t type,
> +			   struct parse_event_data *data)

I'm not sure if "inline" here is a good idea, as it seems to get called
quite a few times. If we're trying to make things fast, bloating the
instruction cache may have the opposite effect.

-Peff