git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Lehmann <Jens.Lehmann@web.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: Eric Cousineau <eacousineau@gmail.com>,
	Heiko Voigt <hvoigt@hvoigt.net>,
	git@vger.kernel.org
Subject: Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well
Date: Tue, 05 Mar 2013 21:51:02 +0100	[thread overview]
Message-ID: <51365AB6.2010602@web.de> (raw)
In-Reply-To: <7vboaxu23y.fsf@alter.siamese.dyndns.org>

Am 05.03.2013 19:34, schrieb Junio C Hamano:
> Eric Cousineau <eacousineau@gmail.com> writes:
> 
>> Would these be the correct behaviors of Heiko's implementation?
> 
> I do not think Heiko already has an implementation, but let's try to
> see how each example makes sense.
> 
>> git submodule foreach # Empty command, pre-order
>> git submodule foreach --pre-order # Same behavior
>> git submodule foreach --post-order # Empty command, post-order
> 
> OK.  The last one shows "I am here" output differently from the
> other two, but otherwise they are all no-op.
> 
>> git submodule foreach 'frotz' # Do 'frotz' pre-order in each submodule
> 
> OK.  And it would be the same if you said either one of:
> 
> 	git submodule foreach --pre-order 'frotz'
> 	git submodule foreach --pre-order='frotz'
> 
>> git submodule foreach --post-order 'frotz' # Do 'frotz' post-order in
>> each submodule
> 
> OK.
> 
>> git submodule foreach --pre-order='frotz' --post-order='shimmy' # Do
>> 'frotz' pre-order and 'shimmy' post-order in each submodule
> 
> OK.
> 
>> git submodule foreach --post-order='shimmy' 'frotz' # Invalid usage of
>> the command
> 
> I would expect this to behave exactly the same as:
> 
> 	git submodule foreach \
>         	--post-order=shimmy \
>                 --pre-order=frotz
> 
>> git submodule foreach --post-order --pre-order #
> 
> I expect it to behave exactly the same as:
> 
> 	git submodule foreach --post-order=: --pre-order=:

I'd favor to just drop the --pre-order option and do this:

  foreach [--recursive] [--post-order <command>] [<command>]

Me thinks pre-order is a sane default and we shouldn't add an
explicit option for that. And even with current Git you can
simply give no command at all and it'll show you all the
submodules it enters without doing anything in them, so we'd
only need to add the --post-order handling anyway (and fix the
synopsis by adding square brackets around the command while at
it, as that is optional).

>> It should not be too hard to have this functionality affect the
>> --include-super command as well.
> 
> I would assume that
> 
> 	git submodule foreach --pre-order=A --post-order=B --include-super
> 
> would be identical to running
> 
> 	A &&
>         git submodule foreach --pre-order=A --post-order=B &&
>         B
>
> I am not entirely convinced we would want --include-super in the
> first place, though.  It does not belong to "submodule foreach";
> it is doing something _outside_ the submoudules.

I totally agree with that. First, adding --include-super does not
belong into the --post-order patch at all, as that is a different
topic (even though it belongs to the same use case Eric has). Also
the reason why we are thinking about adding the --post-order option
IMO cuts the other way for --include-super: It is so easy to do
that yourself I'm not convinced we should add an extra option to
foreach for that, especially as it has nothing to do with submodules.
So I think we should just drop --include-super.

  reply	other threads:[~2013-03-05 20:51 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-04  8:41 [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well Eric Cousineau
2013-03-04 22:15 ` Jens Lehmann
2013-03-04 23:00   ` Junio C Hamano
2013-03-05  5:37     ` Eric Cousineau
2013-03-05  7:59     ` Heiko Voigt
2013-03-05 16:09       ` Junio C Hamano
2013-03-05 16:42         ` Eric Cousineau
2013-03-05 18:34           ` Junio C Hamano
2013-03-05 20:51             ` Jens Lehmann [this message]
2013-03-05 21:17               ` Phil Hord
2013-03-09 18:18                 ` Jens Lehmann
2013-03-11 16:46                   ` Heiko Voigt
2013-03-12 16:01                   ` Phil Hord
2013-03-14  6:30                     ` Eric Cousineau
2013-03-18 21:25                       ` Jens Lehmann
2013-03-26  4:03                         ` Eric Cousineau
2013-04-02 20:14                           ` Jens Lehmann
2013-04-13  4:04                             ` [PATCH] submodule foreach: Added in --post-order=<command> and adjusted code per Jens Lehmann's suggestions eacousineau
     [not found]                               ` <CA+aSAWuK9Yhvx-vO1fUteq-K=xOPgxkyeWeHG3UwZuDHsxLzAw@mail.gmail.com>
2013-04-13  4:11                                 ` Eric Cousineau
2013-04-14 18:52                               ` Jens Lehmann
2013-03-18 21:10                     ` [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well Jens Lehmann
2013-03-26  3:56                       ` Eric Cousineau
2013-03-26  4:36                         ` Eric Cousineau
2013-03-26  5:23                       ` Junio C Hamano
2013-03-26  5:25                         ` 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=51365AB6.2010602@web.de \
    --to=jens.lehmann@web.de \
    --cc=eacousineau@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hvoigt@hvoigt.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 NNTP newsgroup(s).