From: Glen Choo <chooglen@google.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>, git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Jonas Bernoulli" <jonas@bernoul.li>, "Jeff King" <peff@peff.net>,
"Emily Shaffer" <emilyshaffer@google.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH 04/10] git-submodule.sh: dispatch "foreach" to helper
Date: Thu, 20 Oct 2022 14:14:39 -0700 [thread overview]
Message-ID: <kl6ly1taxkn4.fsf@chooglen-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <patch-04.10-db6a09ee42a-20221017T115544Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Dispatch the "git submodule foreach" command directly to "git
> submodule--helper foreach". This case requires the addition of the
> PARSE_OPT_STOP_AT_NON_OPTION flag, since the shellscript was
> unconditionally adding "--" to the "git submodule--helper"
> command-line.
The commands in the previous patch also unconditionally add "--", so
this wasn't clear to me on first read.
It might be clearer to include some extra context, which is that "git
submodule foreach" is different because everything after "foreach"
should be treated like a single comand to run even if they are passed as
separate strings, e.g. from t/t7407-submodule-foreach.sh.21, these two
should be equivalent:
git submodule foreach "echo be --quiet"
vs
git submodule foreach echo be --quiet
So PARSE_OPT_STOP_AT_NON_OPTION is necessary in order to stop us from
intepreting option-like args as args to "git submodule foreach" instead
of as part of the command to run. Without PARSE_OPT_STOP_AT_NON_OPTION,
git submodule foreach echo be --quiet
becomes more like
git submodule foreach --quiet "echo be"
There could have been a subtle change in behavior, since "git submodule
foreach -- --foo" will be parsed as if "--foo" is the command, but
PARSE_OPT_STOP_AT_NON_OPTION will happily accept "--foo" as an option.
But we could never get into this situation anyway since we used to emit
usage on the first option-like arg...
> -# Execute an arbitrary command sequence in each checked out
> -# submodule
> -#
> -# $@ = command to execute
> -#
> -cmd_foreach()
> -{
> - # parse $args after "submodule ... foreach".
> - while test $# -ne 0
> - do
> - case "$1" in
> - -q|--quiet)
> - quiet=1
> - ;;
> - --recursive)
> - recursive=1
> - ;;
> - -*)
> - usage
> - ;;
i.e. here.
So this patch looks safe.
next prev parent reply other threads:[~2022-10-20 21:14 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-17 12:09 [PATCH 00/10] submodule: make it a built-in, remove git-submodule.sh Ævar Arnfjörð Bjarmason
2022-10-17 12:09 ` [PATCH 01/10] git-submodule.sh: create a "case" dispatch statement Ævar Arnfjörð Bjarmason
2022-10-17 12:09 ` [PATCH 02/10] git-submodule.sh: dispatch "sync" to helper Ævar Arnfjörð Bjarmason
2022-10-20 20:42 ` Glen Choo
2022-10-17 12:09 ` [PATCH 03/10] git-submodule.sh: dispatch directly " Ævar Arnfjörð Bjarmason
2022-10-17 12:09 ` [PATCH 04/10] git-submodule.sh: dispatch "foreach" " Ævar Arnfjörð Bjarmason
2022-10-20 21:14 ` Glen Choo [this message]
2022-10-17 12:09 ` [PATCH 05/10] git-submodule.sh: dispatch "update" " Ævar Arnfjörð Bjarmason
2022-10-20 21:50 ` Glen Choo
2022-10-17 12:09 ` [PATCH 06/10] git-submodule.sh: don't support top-level "--cached" Ævar Arnfjörð Bjarmason
2022-10-20 22:14 ` Glen Choo
2022-10-17 12:09 ` [PATCH 07/10] submodule: make it a built-in, remove git-submodule.sh Ævar Arnfjörð Bjarmason
2022-10-20 22:49 ` Glen Choo
2022-10-17 12:09 ` [PATCH 08/10] submodule: support "--" with no other arguments Ævar Arnfjörð Bjarmason
2022-10-17 12:09 ` [PATCH 09/10] submodule: support sub-command-less "--recursive" option Ævar Arnfjörð Bjarmason
2022-10-20 23:05 ` Glen Choo
2022-10-17 12:09 ` [PATCH 10/10] submodule: don't use a subprocess to invoke "submodule--helper" Ævar Arnfjörð Bjarmason
2022-10-20 23:18 ` Glen Choo
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=kl6ly1taxkn4.fsf@chooglen-macbookpro.roam.corp.google.com \
--to=chooglen@google.com \
--cc=avarab@gmail.com \
--cc=emilyshaffer@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jonas@bernoul.li \
--cc=peff@peff.net \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).