git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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?
Previous: Junio C HamanoNext: Emily Shaffer
Message 4 of 16 in “submodule: port subcommand add from shell to C”
  1. 0/3 submodule: port subcommand add from shell to CShourya Shukla, Oct 7, 2020
  2. 1/3 dir: change the scope of function 'directory_exists_in_index()'Shourya Shukla, Oct 7, 2020
  3. Junio C HamanoOct 7, 2020
  4. Shourya ShuklaOct 12, 2020
  5. Emily ShafferNov 18, 2020
  6. 2/3 submodule: port submodule subcommand 'add' from shell to CShourya Shukla, Oct 7, 2020
  7. Junio C HamanoOct 7, 2020
  8. Junio C HamanoOct 7, 2020
  9. Junio C HamanoOct 8, 2020
  10. Junio C HamanoOct 9, 2020
  11. Jonathan TanNov 18, 2020
  12. Ævar Arnfjörð BjarmasonNov 19, 2020
  13. Johannes SchindelinNov 19, 2020
  14. Junio C HamanoNov 19, 2020
  15. 3/3 t7400: add test to check 'submodule add' for tracked pathsShourya Shukla, Oct 7, 2020
  16. Josh SteadmonNov 19, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.