From: Shourya Shukla <shouryashukla.oo@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, christian.couder@gmail.com,
kaartic.sivaraam@gmail.com, Johannes.Schindelin@gmx.de,
liu.denton@gmail.com
Subject: Re: [PATCH v2 1/3] dir: change the scope of function 'directory_exists_in_index()'
Date: Mon, 12 Oct 2020 15:41:40 +0530 [thread overview]
Message-ID: <20201012101140.GA12637@konoha> (raw)
In-Reply-To: <xmqq8sch3o8v.fsf@gitster.c.googlers.com>
On 07/10 11:05, Junio C Hamano wrote:
> 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?
next prev parent reply other threads:[~2020-10-12 10:11 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-07 7:45 [PATCH v2 0/3] submodule: port subcommand add from shell to C Shourya Shukla
2020-10-07 7:45 ` [PATCH v2 1/3] dir: change the scope of function 'directory_exists_in_index()' Shourya Shukla
2020-10-07 18:05 ` Junio C Hamano
2020-10-12 10:11 ` Shourya Shukla [this message]
2020-11-18 23:25 ` Emily Shaffer
2020-10-07 7:45 ` [PATCH v2 2/3] submodule: port submodule subcommand 'add' from shell to C Shourya Shukla
2020-10-07 18:37 ` Junio C Hamano
2020-10-07 22:19 ` Junio C Hamano
2020-10-08 17:19 ` Junio C Hamano
2020-10-09 5:09 ` Junio C Hamano
2020-11-18 23:13 ` Jonathan Tan
2020-11-19 7:44 ` Ævar Arnfjörð Bjarmason
2020-11-19 12:38 ` Johannes Schindelin
2020-11-19 20:37 ` Junio C Hamano
2020-10-07 7:45 ` [PATCH v2 3/3] t7400: add test to check 'submodule add' for tracked paths Shourya Shukla
2020-11-19 0:03 ` [PATCH v2 0/3] submodule: port subcommand add from shell to C Josh Steadmon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201012101140.GA12637@konoha \
--to=shouryashukla.oo@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=kaartic.sivaraam@gmail.com \
--cc=liu.denton@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.