From: Atharva Raykar <raykar.ath@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Git List <git@vger.kernel.org>,
Emily Shaffer <emilyshaffer@google.com>,
Jonathan Nieder <jrnieder@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
Christian Couder <christian.couder@gmail.com>,
Shourya Shukla <periperidip@gmail.com>,
Kaartic Sivaraam <kaartic.sivaraam@gmail.com>,
Prathamesh Chavan <pc44800@gmail.com>
Subject: Re: [GSoC] [PATCH] submodule--helper: run update procedures from C
Date: Wed, 04 Aug 2021 14:05:12 +0530 [thread overview]
Message-ID: <m2wnp1a9q7.fsf@gmail.com> (raw)
In-Reply-To: <9532C3EF-257E-4898-8C75-C49EA4B66A99@gmail.com>
Atharva Raykar <raykar.ath@gmail.com> writes:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>>> static void update_submodule(struct update_clone_data *ucd)
>>> {
>>> fprintf(stdout, "dummy %s %d\t%s\n",
>>> @@ -2379,6 +2552,79 @@ static int update_clone(int argc, const char **argv, const char *prefix)
>>> return update_submodules(&suc);
>>> }
>>>
>>> +static int run_update_procedure(int argc, const char **argv, const char *prefix)
>>> +{
>>> + int force = 0, quiet = 0, nofetch = 0, just_cloned = 0;
>>> + char *prefixed_path, *update = NULL;
>>> + char *sha1 = NULL, *subsha1 = NULL;
>>> + struct update_data update_data = UPDATE_DATA_INIT;
>>> +
>>> + struct option options[] = {
>>> + OPT__QUIET(&quiet, N_("suppress output for update by rebase or merge")),
>>> + OPT__FORCE(&force, N_("force checkout updates"), 0),
>>> + OPT_BOOL('N', "no-fetch", &nofetch,
>>> + N_("don't fetch new objects from the remote site")),
>>> + OPT_BOOL(0, "just-cloned", &just_cloned,
>>> + N_("overrides update mode in case the repository is a fresh clone")),
>>> + OPT_INTEGER(0, "depth", &update_data.depth, N_("depth for shallow fetch")),
>>> + OPT_STRING(0, "prefix", &prefix,
>>> + N_("path"),
>>> + N_("path into the working tree")),
>>> + OPT_STRING(0, "update", &update,
>>> + N_("string"),
>>> + N_("rebase, merge, checkout or none")),
>>> + OPT_STRING(0, "recursive-prefix", &update_data.recursive_prefix, N_("path"),
>>> + N_("path into the working tree, across nested "
>>> + "submodule boundaries")),
>>> + OPT_STRING(0, "sha1", &sha1, N_("string"),
>>> + N_("SHA1 expected by superproject")),
>>> + OPT_STRING(0, "subsha1", &subsha1, N_("string"),
>>> + N_("SHA1 of submodule's HEAD")),
>>> + OPT_END()
>>> + };
>>> +
>>> + const char *const usage[] = {
>>> + N_("git submodule--helper run-update-procedure [<options>] <path>"),
>>> + NULL
>>> + };
>>> +
>>> + argc = parse_options(argc, argv, prefix, options, usage, 0);
>>> +
>>> + if (argc != 1)
>>> + usage_with_options(usage, options);
>>> + update_data.force = !!force;
>>> + update_data.quiet = !!quiet;
>>> + update_data.nofetch = !!nofetch;
>>> + update_data.just_cloned = !!just_cloned;
>>
>> For all of these just pass the reference to the update_data variable
>> directly in the OPT_*(). No need to set an "int force", only to copy it
>> over to update_data.force. Let's just use the latter only.
>
> Hmm, I'm trying to remember why the single bit values are treated this way
> in this whole file...
>
> ...there seems to be no good reason for it. The API docs for parse options
> state that OPT_BOOL() is guaranteed to return either zero or one, so that
> double negation does look unnecessary.
I forgot to mention why I did not address this change in my v3 patch.
The reason why we are handling boolean values this way is because they
are declared as bitfields in the 'update_data' struct. Since we cannot
take the address of bitfields, we have to use a different variable to
store when using 'parse_options()'.
next prev parent reply other threads:[~2021-08-04 8:35 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-22 13:40 [GSoC] [PATCH] submodule--helper: run update procedures from C Atharva Raykar
2021-07-23 9:37 ` Ævar Arnfjörð Bjarmason
2021-07-23 16:59 ` Atharva Raykar
2021-08-04 8:35 ` Atharva Raykar [this message]
2021-08-02 13:06 ` [GSoC] [PATCH v2] " Atharva Raykar
2021-08-02 18:50 ` Shourya Shukla
2021-08-03 8:46 ` Atharva Raykar
2021-08-03 10:07 ` Atharva Raykar
2021-08-13 7:56 ` [GSoC] [PATCH v3] " Atharva Raykar
2021-08-13 18:32 ` Junio C Hamano
2021-08-24 8:58 ` Atharva Raykar
2021-08-24 14:06 ` [PATCH v4] " Atharva Raykar
2021-09-08 0:14 ` Junio C Hamano
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=m2wnp1a9q7.fsf@gmail.com \
--to=raykar.ath@gmail.com \
--cc=avarab@gmail.com \
--cc=christian.couder@gmail.com \
--cc=emilyshaffer@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=kaartic.sivaraam@gmail.com \
--cc=pc44800@gmail.com \
--cc=periperidip@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).