Subject: Re: [PATCH v2 1/3] dir: change the scope of function 'directory_exists_in_index()'

From: Shourya Shukla <[email-masked]>
Date: Mon, 12 Oct 2020 15:41:40 +0530

On 07/10 11:05, Junio C Hamano wrote:
> Shourya Shukla <[email-masked]> writes:
> 
> > diff --git a/dir.h b/dir.h
> > index a3c40dec51..e46f240528 100644
> > --- a/dir.h
> > +++ b/dir.h
> > @@ -370,6 +370,15 @@ int read_directory(struct dir_struct *, struct index_state *istate,
> >  		   const char *path, int len,
> >  		   const struct pathspec *pathspec);
> >  
> > +enum exist_status {
> > +	index_nonexistent = 0,
> > +	index_directory,
> > +	index_gitdir
> > +};
> 
> These were adequate as private names used within the wall of dir.c,
> but I doubt that they are named specific enough to stand out as
> public symbols.
> 
> Unlike say "index_state" (the name of a struct type), whose nature
> is quite global to any code that wants to access the in-core index,
> "index_directory" is *NOT* such a name.  It is only of interest to
> those who want to see "I have a directory name---does it appear as a
> directory in the index?".  It is not even interesting to those who
> want to ask similar and related questions like "I have this
> pathname---does it appear as anything in the index, and if so what
> type of entry is it?".  A worse part of this is that even if such a
> helper function file_exists_in_index() were to be written, the
> "exist_status" enum won't be usable to return the answer that
> question, but yet the enum squats on a perfectly good name to
> express "status" for the whole class that it does not represent.
> 
> So, NAK.  We need to come up with a better name for these symbols if
> we were to expose them to the outside world.  The only good name
> this patch makes public is "directory_exists_in_index()", which is
> specific enough.

Understandable. So, how would it be if the function I wrote in
'submodule--helper.c' could instead be written here (in dir.c) and
would help to check if the path is there in the index or not. It could be anything
ranging from a filename to a SM. Since the return type will be an
integer, this PATCH 1/3 can be eliminated and we won't need to change
the scope of the 'directory_exists_in_index()' function and the enum can
stay visible only in dir.c

What are your thoughts on this?