Re: [PATCH 4/6] dir: move setting of nested_repo next to its actual usage

> On Thu, Jan 30, 2020 at 8:10 AM Derrick Stolee <stolee@gmail.com> wrote:
>> 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.