On Fri, Mar 30, 2018 at 2:32 PM, Johannes Schindelin
[off-list ref] wrote:

Hi,

On Thu, 29 Mar 2018, Jeff King wrote:

quoted

On Thu, Mar 29, 2018 at 11:15:33AM -0700, Stefan Beller wrote:

quoted

quoted

When calling `git config --unset abc.a` on this file, it leaves this
(invalid) config behind:

        [
        [xyz]
                key = value

The reason is that we try to search for the beginning of the line (or
for the end of the preceding section header on the same line) that
defines abc.a, but as an optimization, we subtract 2 from the offset
pointing just after the definition before we call
find_beginning_of_line(). That function, however, *also* performs that
optimization and promptly fails to find the section header correctly.

This commit message would be more convincing if we had it in test form.

I agree a test might be nice. But I don't find the commit message
unconvincing at all. It explains pretty clearly why the bug occurs, and
you can verify it by looking at find_beginning_of_line.

quoted

    [abc]a

is not written by Git, but would be written from an outside tool or person
and we barely cope with it?

Yes, I don't think git would ever write onto the same line. But clearly
we should handle anything that's syntactically valid.

I was tempted to add the test case, because it is easy to test it.

But I then decided *not* to add it. Why? Testing is a balance between "can
do" and "need to do".

Can you imagine that I did *not* run the entire test suite before
submitting this patch series, because it takes an incredible *90 minutes*
to run *on a fast Windows machine*?

What's wrong with firing up a new worktree, run the test suite there
and go back to do something else so you won't waste time just waiting
for test results and submit? Sure there is a mental overhead for
switching tasks, but at 90 minutes, I think it's worth doing.
-- 
Duy