On Fri, Mar 30, 2018 at 2:32 PM, Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > Hi, > > On Thu, 29 Mar 2018, Jeff King wrote: > >> On Thu, Mar 29, 2018 at 11:15:33AM -0700, Stefan Beller wrote: >> >> > > 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. >> >> > [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
