git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jens Lehmann <Jens.Lehmann@web.de>
Cc: Eric Cousineau <eacousineau@gmail.com>, 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: Mon, 04 Mar 2013 15:00:45 -0800	[thread overview]
Message-ID: <7vhakqwz1e.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <51351CF5.7010308@web.de> (Jens Lehmann's message of "Mon, 04 Mar 2013 23:15:17 +0100")

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Please don't attach your patches, see Documentation/SubmittingPatches on
> how to post patches to this list.
>
> Am 04.03.2013 09:41, schrieb Eric Cousineau:
>> In this patch, foreach --recursive acts depth-first, much like the default
>> behavior described in the patch by Imram Yousuf in this
>> post <http://marc.info/?l=git&m=121066084508631&w=2>.
>> Changes were made so that the submodule "Entering ..." message was right
>> next to the output generated by the command too.
>> It also adds the --parent option for executing the command in the
>> supermodule as well.
>
> From reading the linked pages I assume a valid use case you have is:
>
>    git submodule foreach --recursive 'git add -A && git commit ...'
>
> This will currently not work because the depth first algorithm of foreach
> will execute the command /before/ recursing deeper. You'd need it to
> execute the command /after/ returning from the deeper level (which is what
> your patch seems to be about).
> ...
> What we currently get from your example is:
>   Entering 'a'
>   Entering 'a/b'
>   Entering 'a/b/d'
>   ...
>   Entering 'c'
>   Entering 'd'
> Me thinks this is what most users would expect of a recursion, enter each
> level before descending into the next.
>
> For your use case you'd need to have:
>   Entering 'a/b/d'
>   Entering 'a/b'
>   Entering 'a/c'
>   ...
>   Entering 'c'
>   Entering 'd'
> (Please note that this is still depth-first)
>
> I won't object to adding an option to foreach that will execute the command
> after recursing (but I'm not convinced --parent is a very good name for that).

Are you comparing pre-order vs post-order traversal?

Both can be useful depending on what you are trying to achieve.  You
need a pre-order traversal (i.e. you "visit" and perform some action
on the node and then descend into its children) if you need to do
some preparation before you visit deeper levels; you need a
post-order traversal (i.e. you "visit" and perform some action on
the node after you have done all its children) if you know you will
be readly only after you are done with all your children.

You can throw in in-order traversal to the mix (i.e. you "visit" and
perform some action on the node after visiting some but not all of
your children and then continue visiting the remainder of your
children), but I do not know what practical value you would get out
of it.

So if you want a single boolean to toggle between the current
behaviour and the other one, it would be --post-order.  But you may
at least want to consider pros and cons of allowing users to give
two separate commands, one for the pre-order visitation (which is
the current "command") and the other for the post-order
visitation. Being able to run both might turn out to be useful.

  reply	other threads:[~2013-03-04 23:01 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 [this message]
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
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=7vhakqwz1e.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=Jens.Lehmann@web.de \
    --cc=eacousineau@gmail.com \
    --cc=git@vger.kernel.org \
    /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).