Hi Peff,

On Thu, 29 Mar 2018, Jeff King wrote:

On Thu, Mar 29, 2018 at 05:18:30PM +0200, Johannes Schindelin wrote:

quoted

The first patch is somewhat of a "while at it" bug fix that I first
thought would be a lot more critical than it actually is: It really
only affects config files that start with a section followed
immediately (i.e. without a newline) by a one-letter boolean setting
(i.e. without a `= <value>` part). So while it is a real bug fix, I
doubt anybody ever got bitten by it.

That makes me wonder if somebody could craft a malicious config to do
something bad.

I thought about that, and could not think of anything other than social
engineering vectors. Even in that case, the error message is instructive
enough that the user should be able to fix the config without consulting
StackOverflow.

quoted

Now, to the really important part: why does this patch series not
conflict with my very early statements that we cannot simply remove
empty sections because we may end up with stale comments?

Well, the patch in question takes pains to determine *iff* there are
any comments surrounding, or included in, the section. If any are
found: previous behavior. Under the assumption that the user edited
the file, we keep it as intact as possible (see below for some
argument against this). If no comments are found, and let's face it,
this is probably *the* common case, as few people edit their config
files by hand these days (neither should they because it is too easy
to end up with an unparseable one), the now-empty section *is*
removed.

I'm not against people editing their config files by hand. But I think
what you propose here makes a lot of sense, because it works as long as
you don't intermingle hand- and auto-editing in the same section (and it
even works if you do intermingle, as long as you don't use comments,
which are probably even more rare).

So it seems like quite a sensible compromise, and I think should make
most people happy.

Thanks for confirming my line of thinking,
Dscho