Re: [PATCH 7/9] git config --unset: remove empty sections (in normal situations)

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 30, 2018, 13:00 UTC
Message-ID: <nycvar.QRO.7.76.6.1803301454400.5026@qfpub.tvgsbejvaqbjf.bet>
In-Reply-To: <20180329213229.GG2939@sigill.intra.peff.net>

Hi Peff,

On Thu, 29 Mar 2018, Jeff King wrote:

Show 11 quoted lines

> On Thu, Mar 29, 2018 at 05:19:00PM +0200, Johannes Schindelin wrote:
> 
> > Let's generalize this observation to this conservative strategy: if we
> > are removing the last entry from a section, and there are no comments
> > inside that section nor surrounding it, then remove the entire section.
> > Otherwise behave as before: leave the now-empty section (including those
> > comments, even the one about the now-deleted entry).
> 
> Yep, as I said earlier, this makes a ton of sense to me.
> 
> [... thorough review ...]

Thank you for taking the time (and figuring out my off-by-ones, am I not the king of those?).

Your in-depth analysis of the backtracking approach also makes sense, in particular the awful bug that looks very, very similar to what 1/9 fixes elsewhere.

I'll take some time to go over your comments in detail, but there is one suggestion that I think I'll want to pursue first:

Show 5 quoted lines

> I guess the holy grail would be a parser which reports _all_ syntactic
> events (section names, keys, comments, whitespace, etc) as a stream
> without storing anything. And then the normal reader could just discard
> the non-key events, and the writer here could build the tree from those
> events.

I already changed the do_config_from_file()/do_config_from() code path to allow for handing back section headers. And I *think* that approach should be easily extended to allow for an optional callback for these syntactic events (and we do not need more than that, as the parsed "tree" really is a list: there is nothing nested about ini files, so we really only have a linear list of blocks (event type, offset range)).

I'll think about this a little bit, and hopefully come back with v2 in a while that uses that approach.

Thank you so much for that suggestion, Dscho