Re: [PATCH v2 1/3] dir: change the scope of function 'directory_exists_in_index()'
- From
Shourya Shukla <shouryashukla.oo@gmail.com>
- Date
- Oct 12, 2020, 10:11 UTC
- Message-ID
- <20201012101140.GA12637@konoha>
- In-Reply-To
- <xmqq8sch3o8v.fsf@gitster.c.googlers.com>
On 07/10 11:05, Junio C Hamano wrote:
Show 37 quoted lines
> Shourya Shukla <shouryashukla.oo@gmail.com> 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?