Thanks, just a quick review before I find some time do take a
deeper look.
Am 14.03.2013 07:30, schrieb Eric Cousineau:
> From 59fb432e17a1aae9de26bbaaca7f09cc7f03b471 Mon Sep 17 00:00:00 2001
> From: Eric Cousineau <[email protected]>
> Date: Thu, 14 Mar 2013 01:19:53 -0500
> Subject: [PATCH] submodule-foreach: Added in --post-order=<command> per Jens
> Lehmann's suggestion
>
> Signed-off-by: Eric Cousineau <[email protected]>
> ---
> Made the scope of the patch only relate to --post-order.
> Would we want to rename this to just --post=<command> ?
Hmm, while having no strong preference on that, "post order"
looks more like the correct term describing what we do here.
> Anywho, here it is running in a test setup, where the structure is:
> a
> - b
> - - d
> - c
>
> $ git submodule foreach --recursive --post-order 'echo Post $name' 'echo Pre
> $path'
> Entering 'b'
> Pre b
> Entering 'b/d'
> Pre d
> Entering 'b/d'
> Post d
> Entering 'b'
> Post b
> Entering 'c'
> Pre c
> Entering 'c'
> Post c
Looking good.
> An interesting note is that it fails with 'git submodule foreach
> --post-order', but not 'git submodule foreach --post-order=', since it simply
> interprets that as an empty command.
> If that is important, I could add in a check for $# when parsing the argument
> for --post-order=*.
>
> git-submodule.sh | 39 ++++++++++++++++++++++++++++++++++-----
> 1 file changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 004c034..9b70bc2 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -10,7 +10,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name
> <name>] [--reference <re
> or: $dashless [--quiet] init [--] [<path>...]
> or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch]
> [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive]
> [--] [<path>...]
> or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>]
> [commit] [--] [<path>...]
> - or: $dashless [--quiet] foreach [--recursive] <command>
> + or: $dashless [--quiet] foreach [--recursive] [--post-order=<command>]
> <command>
Maybe drop the '=' from the description (see --reference for an example)?
> or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
> OPTIONS_SPEC=
> . git-sh-setup
> @@ -434,6 +434,8 @@ Use -f if you really want to add it." >&2
> cmd_foreach()
> {
> # parse $args after "submodule ... foreach".
> + # Gratuitous (empty) local's to prevent recursive bleeding
> + local recursive= post_order=
Wouldn't it be sufficient to add "post_order=" to the top of the
file where "recursive" is already initialized? Or am I missing
something here?
> while test $# -ne 0
> do
> case "$1" in
> @@ -443,6 +445,15 @@ cmd_foreach()
> --recursive)
> recursive=1
> ;;
> + --post-order)
> + test "$#" = "1" && usage
> + post_order="$2"
> + shift
> + ;;
> + --post-order=*)
> + # Will skip empty commands
> + post_order=${1#*=}
> + ;;
> -*)
> usage
> ;;
> @@ -453,7 +464,7 @@ cmd_foreach()
> shift
> done
>
> - toplevel=$(pwd)
> + local toplevel=$(pwd)
Why do you have to add the "local" keyword here?
> # dup stdin so that it can be restored when running the external
> # command in the subshell (and a recursive call to this function)
> @@ -465,18 +476,36 @@ cmd_foreach()
> die_if_unmatched "$mode"
> if test -e "$sm_path"/.git
> then
> - say "$(eval_gettext "Entering '\$prefix\$sm_path'")"
> + local name prefix path message epitaph
Same here?
> + message="$(eval_gettext "Entering '\$prefix\$sm_path'")"
> + epitaph="$(eval_gettext "Stopping at '\$sm_path'; script
> returned non-zero status.")"
> name=$(module_name "$sm_path")
> (
> prefix="$prefix$sm_path/"
> clear_local_git_env
> # we make $path available to scripts ...
> path=$sm_path
> +
> + sm_eval() {
> + say "$message"
> + eval "$@" || die "$epitaph"
> + }
> +
> cd "$sm_path" &&
> - eval "$@" &&
> + sm_eval "$@" &&
> if test -n "$recursive"
> then
> - cmd_foreach "--recursive" "$@"
> + if test -n "$post_order"
> + then
> + # Tried keeping flags as a variable, but was having
> difficulty
Maybe because you set the "post_order" variable to empty at the
beginning of this function? If I read that right moving that
initialization to the top of the file could get rid of the if
here?
> + cmd_foreach --recursive --post-order "$post_order"
> "$@"
> + else
> + cmd_foreach --recursive "$@"
> + fi
> + fi &&
> + if test -n "$post_order"
> + then
> + sm_eval "$post_order"
> fi
> ) <&3 3<&- ||
> die "$(eval_gettext "Stopping at '\$sm_path'; script returned
> non-zero status.")"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
