From: Elijah Newren Date: Mon, 05 Feb 2018 22:18:35 GMT Subject: Re: [PATCH v7 28/31] merge-recursive: fix remaining directory rename + dirty overwrite cases Message-ID: In-Reply-To: On Mon, Feb 5, 2018 at 1:52 PM, Stefan Beller wrote: > On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren wrote: >> + /* >> + * Stupid double negatives in remove_file; it somehow manages >> + * to repeatedly mess me up. So, just for myself: >> + * 1) update_wd iff !ren_src_was_dirty. >> + * 2) no_wd iff !update_wd >> + * 3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty >> + */ > > Not sure iff this comment is at the right place and is a good addition to > the code base. Fair enough, and I should apologize for letting my frustration come through there. However, what if I replaced the first two lines of the comment with: "Because the double negatives somehow keep confusing me..." so that it reads: /* * Because the double negatives somehow keep confusing me... * 1) update_wd iff !ren_src_was_dirty. * 2) no_wd iff !update_wd * 3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty */ Even if my wording was suboptimal, the rest of the comment did seem pretty important because I messed up the line after the comment multiple times. (You'd think that the odds of getting it right should be 50/50 and that a simple inversion would fix it, so one could only mess the line up once, but I'm apparently special). And then I came back to look at it later and was still confused. For some reason, I seem to need the longer explanation. > However it hints at the underlying issue of a bad API that is provided > by remove_file ? I'd definitely agree with that.