From: Junio C Hamano <gitster@pobox.com> To: Shourya Shukla <shouryashukla.oo@gmail.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: Wed, 07 Oct 2020 11:05:52 -0700 Message-ID: <xmqq8sch3o8v.fsf@gitster.c.googlers.com> (raw) In-Reply-To: <20201007074538.25891-2-shouryashukla.oo@gmail.com> (Shourya Shukla's message of "Wed, 7 Oct 2020 13:15:36 +0530") 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. > +enum exist_status directory_exists_in_index(struct index_state *istate, > + const char *dirname, int len); > + > enum pattern_match_result { > UNDECIDED = -1, > NOT_MATCHED = 0,
next prev parent reply other threads:[~2020-10-07 18:06 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 [this message] 2020-10-12 10:11 ` Shourya Shukla 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 List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=xmqq8sch3o8v.fsf@gitster.c.googlers.com \ --to=gitster@pobox.com \ --cc=Johannes.Schindelin@gmx.de \ --cc=christian.couder@gmail.com \ --cc=git@vger.kernel.org \ --cc=kaartic.sivaraam@gmail.com \ --cc=liu.denton@gmail.com \ --cc=shouryashukla.oo@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
git@vger.kernel.org list mirror (unofficial, one of many) This inbox may be cloned and mirrored by anyone: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V1 git git/ https://public-inbox.org/git \ git@vger.kernel.org public-inbox-index git Example config snippet for mirrors. Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.io/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ code repositories for the project(s) associated with this inbox: https://80x24.org/mirrors/git.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git