Hi Junio, On Mon, 11 Sep 2017, Junio C Hamano wrote:
> Stepping back a bit, I am not sure if it is sane or even valid for the > end-user to modify paths outside sparse-checkout area, but that is > probably a separate tangent. That is not at all the scenario that Kevin fixed. Just have a quick look at the regression test: in a sparse checkout, the user checked out a branch, then called `reset` to switch to a different commit. No file was touched by the user outside the sparse checkout. Yet without Kevin's fix, `git status` would report that the user *deleted files outside the sparse checkout*. That is such an obvious bug, and Kevin's fix is such an obvious improvement over the current upstream Git version, that I would think the only thing worth discussing is whether the patch goes about it in a way of which you approve. For example, you mentioned that you would want to move the declaration of `two` and `was_missing` into the conditional code block. That is a valid suggestion for `was_missing` (but of course not for `two`, which is used in the condition of the code block). That suggestion is more about code style (and of course easily fixed by Kevin using Edit>Refactor>Move Definition Location in VS), though, than about the correctness of the post image. Much more interesting would be a review of the conditional code block. And I am not talking about the camelCasing of `ceBefore` (which will be fixed as easily by Edit>Refactor>Rename). I am talking about the stuff where tools cannot help, but where your experience is necessary: is it correct to use make_cache_entry()/checkout_entry() in this case? Are the parameters passed to those functions correct? Is the call to cache_name_pos() followed by ce_skip_worktree() the best way to find out whether the file that is absent was not actually deleted by the user, or is there a less CPU-intensive way, seeing as we are already guaranteed to iterate over the queue diff in alphabetical order? I understand that those latter questions are a lot harder to answer, sorry about that. Ciao, Dscho
