From: Shaoxuan Yuan Date: Thu, 21 Jul 2022 13:58:23 GMT Subject: Re: [PATCH v1 2/7] mv: add documentation for check_dir_in_index() Message-ID: <0af9c29a-2071-6aa3-28ce-9b9127789644@gmail.com> In-Reply-To: <228ad533-477c-f16e-220d-61e52d9aee26@github.com> On 7/20/2022 1:43 AM, Derrick Stolee wrote: >> + * >> + * Note: *always* check the directory is not on-disk before this function >> + * (i.e. using lstat()); >> + * otherwise it may return a false positive for a partially sparsified >> + * directory. > > I'm not sure what you mean by a "false positive" in this case. > The directory exists in the index, which is what the method is > defined as checking. This does not say anything about the > worktree. > > Perhaps that's the real problem? Someone might interpret this > as meaning the directory does not exist in the worktree? That > would mean that this doc update needs to be changed significantly > to say "Note that this does not imply anything about the state > of the worktree" or something. This method assumes that the directory being checking does not exist in the working tree, but the method itself does not check this. And if the user does not make sure the directory is absent from the worktree, this method may return a success for a partially sparsified directory, which is not intended. > But I think I'd rather just see this patch be dropped, unless I > am missing something important. I found Victoria's paraphrase [1] makes my point much clearer. -- Thanks, Shaoxuan