On 1/30/2020 11:20 AM, Elijah Newren wrote:

On Thu, Jan 30, 2020 at 8:10 AM Derrick Stolee [off-list ref] wrote:

quoted

diff --git a/dir.c b/dir.c
index b460211e61..0989558ae6 100644
--- a/dir.c
+++ b/dir.c

@@ -1660,29 +1660,26 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
        const struct pathspec *pathspec)
 {
        int nested_repo = 0;
-
        /* The "len-1" is to strip the final '/' */
-       switch (directory_exists_in_index(istate, dirname, len-1)) {
-       case index_directory:
-               return path_recurse;
+       enum exist_status status = directory_exists_in_index(istate, dirname, len-1);

-       case index_gitdir:
+       if (status == index_directory)
+               return path_recurse;
+       if (status == index_gitdir)
                return path_none;

I think right here we should add:

        if (status != index_nonexistent):
                BUG("Unhandled value for directory_exists_in_index:
%d\n", status);

for future-proofing, since both you and I had to look up what
possibilities existed as a return status from
directory_exists_in_index(), and I'd rather a large warning was thrown
if someone ever adds a fourth option to that function rather than
assume treat_directory() is fine and only needs to special case two
choices.

Or we could add an assert or a code comment, just so long as we
document to future readers that the remainder of the code is assuming
status==index_nonexistent.

I'm happy if you squash this into the commit. Thanks!