Re: [PATCH] submodule foreach: Added in --post-order=command and adjusted code per Jens Lehmann's suggestions
Am 13.04.2013 06:04, schrieb eacousineau:
Signed-off-by: eacousineau eacousin...@gmail.com
---
I see what you meant by the extra variables, so I've fixed that so the
original flags aren't needed with recursion.
Thanks, the code is looking much better now and you nicely
described the changes you made since the last version. A few
comments though:
I think the subject line should read:
[PATCH v2] submodule foreach: Add --post-order option
We use the imperative form, also the adjustments are a normal part
of the review process and don't need to be mentioned explicitly in
the title, just show the version of your iteration by adding v2
after the word PATCH (and of course the next iteration will be
v3 ;-).
The commit message is not explaining what you did and why you did
it, please see the Describe your changes well. section in
Documentation/SubmittingPatches on how to do that.
And you'll also want to add the new option to the man page in
Documentation/git-submodule.txt.
Also updated it to not
print the entering command if there is only a post-order command.
Good idea.
Examples:
$ git submodule foreach --recursive --post-order 'echo Goodbye' echo
\'ello\
Entering 'b'
'ello
Entering 'b/d'
'ello
Leaving 'b/d'
Goodbye
Leaving 'b'
Goodbye
Entering 'c'
'ello
Leaving 'c'
Goodbye
$ git submodule foreach --recursive --post-order :
Leaving 'b/d'
Leaving 'b'
Leaving 'c'
Makes sense to me. These two examples should be getting a test
case each in t/t7407-submodule-foreach.sh.
git-submodule.sh | 31 ++-
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..e08a724 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -11,7 +11,7 @@ USAGE=[--quiet] add [-b branch] [-f|--force] [--name
name] [--reference re
or: $dashless [--quiet] deinit [-f|--force] [--] 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
or: $dashless [--quiet] sync [--recursive] [--] [path...]
OPTIONS_SPEC=
. git-sh-setup
@@ -449,6 +449,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
;;
@@ -471,7 +480,9 @@ cmd_foreach()
die_if_unmatched $mode
if test -e $sm_path/.git
then
- say $(eval_gettext Entering '\$prefix\$sm_path')
+ enter_msg=$(eval_gettext Entering
'\$prefix\$sm_path')
+ leave_msg=$(eval_gettext Leaving
'\$prefix\$sm_path')
I don't think we gain much by putting enter_msg and leave_msg into
their own variables as they are only used once, I'd prefer to see
these messages inlined.
+ die_msg=$(eval_gettext Stopping at '\$sm_path';
script returned non-zero status.)
I think there is a \$prefix missing in front of the \$sm_path here
(see enter_msg and leave_msg). As you only copied that message you
can simply say in the commit message While at it also fix a missing
prefix in the die message at the end of the last paragraph.
name=$(module_name $sm_path)
(
prefix=$prefix$sm_path/
@@ -479,13 +490,23 @@ cmd_foreach()
# we make $path available to scripts ...
path=$sm_path
cd $sm_path
- eval $@
+ if test $# -gt 0 -o -z $post_order
+ then
+ say $enter_msg
+ eval $@ || die $die_msg
+ fi
if test -n $recursive
then
- cmd_foreach --recursive $@
+ # subshell will use parent-scoped values
+ cmd_foreach $@
You should at least state that you dropped the --recursive here
on purpose, just add that to the While at it ... sentence. And I
suspect the comment above is more a reminder for yourself, we
could drop that too.
+ fi
+
[PATCH] submodule foreach: Added in --post-order=command and adjusted code per Jens Lehmann's suggestions
Signed-off-by: eacousineau eacousin...@gmail.com
---
I see what you meant by the extra variables, so I've fixed that so the
original flags aren't needed with recursion. Also updated it to not
print the entering command if there is only a post-order command.
Examples:
$ git submodule foreach --recursive --post-order 'echo Goodbye' echo \'ello\
Entering 'b'
'ello
Entering 'b/d'
'ello
Leaving 'b/d'
Goodbye
Leaving 'b'
Goodbye
Entering 'c'
'ello
Leaving 'c'
Goodbye
$ git submodule foreach --recursive --post-order :
Leaving 'b/d'
Leaving 'b'
Leaving 'c'
git-submodule.sh | 31 ++-
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..e08a724 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -11,7 +11,7 @@ USAGE=[--quiet] add [-b branch] [-f|--force] [--name
name] [--reference re
or: $dashless [--quiet] deinit [-f|--force] [--] 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
or: $dashless [--quiet] sync [--recursive] [--] [path...]
OPTIONS_SPEC=
. git-sh-setup
@@ -449,6 +449,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
;;
@@ -471,7 +480,9 @@ cmd_foreach()
die_if_unmatched $mode
if test -e $sm_path/.git
then
- say $(eval_gettext Entering '\$prefix\$sm_path')
+ enter_msg=$(eval_gettext Entering
'\$prefix\$sm_path')
+ leave_msg=$(eval_gettext Leaving
'\$prefix\$sm_path')
+ die_msg=$(eval_gettext Stopping at '\$sm_path';
script returned non-zero status.)
name=$(module_name $sm_path)
(
prefix=$prefix$sm_path/
@@ -479,13 +490,23 @@ cmd_foreach()
# we make $path available to scripts ...
path=$sm_path
cd $sm_path
- eval $@
+ if test $# -gt 0 -o -z $post_order
+ then
+ say $enter_msg
+ eval $@ || die $die_msg
+ fi
if test -n $recursive
then
- cmd_foreach --recursive $@
+ # subshell will use parent-scoped values
+ cmd_foreach $@
+ fi
+ if test -n $post_order
+ then
+ say $leave_msg
+ eval $post_order || die $die_msg
fi
) 3 3- ||
- die $(eval_gettext Stopping at '\$sm_path'; script
returned non-zero status.)
+ die $die_msg
fi
done
}
--
1.8.2.1.390.gd4ee029
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule foreach: Added in --post-order=command and adjusted code per Jens Lehmann's suggestions
Had accidentally sent this as HTML, resending as plain-text.
On Fri, Apr 12, 2013 at 11:09 PM, Eric Cousineau eacousin...@gmail.com wrote:
Oops... I tried out using git-send-email adding in the Message-Id, but forgot
to change the title as well. My bad.
This was in response to:
[PATCH/RFC] Changing submodule foreach --recursive to be depth-first,
--parent option to execute command in supermodule as well
Message-Id: 515b3c0e.9000...@web.de
- Eric
On Fri, Apr 12, 2013 at 11:04 PM, eacousineau eacousin...@gmail.com wrote:
Signed-off-by: eacousineau eacousin...@gmail.com
---
I see what you meant by the extra variables, so I've fixed that so the
original flags aren't needed with recursion. Also updated it to not
print the entering command if there is only a post-order command.
Examples:
$ git submodule foreach --recursive --post-order 'echo Goodbye' echo
\'ello\
Entering 'b'
'ello
Entering 'b/d'
'ello
Leaving 'b/d'
Goodbye
Leaving 'b'
Goodbye
Entering 'c'
'ello
Leaving 'c'
Goodbye
$ git submodule foreach --recursive --post-order :
Leaving 'b/d'
Leaving 'b'
Leaving 'c'
git-submodule.sh | 31 ++-
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..e08a724 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -11,7 +11,7 @@ USAGE=[--quiet] add [-b branch] [-f|--force] [--name
name] [--reference re
or: $dashless [--quiet] deinit [-f|--force] [--] 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
or: $dashless [--quiet] sync [--recursive] [--] [path...]
OPTIONS_SPEC=
. git-sh-setup
@@ -449,6 +449,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
;;
@@ -471,7 +480,9 @@ cmd_foreach()
die_if_unmatched $mode
if test -e $sm_path/.git
then
- say $(eval_gettext Entering '\$prefix\$sm_path')
+ enter_msg=$(eval_gettext Entering
'\$prefix\$sm_path')
+ leave_msg=$(eval_gettext Leaving
'\$prefix\$sm_path')
+ die_msg=$(eval_gettext Stopping at '\$sm_path';
script returned non-zero status.)
name=$(module_name $sm_path)
(
prefix=$prefix$sm_path/
@@ -479,13 +490,23 @@ cmd_foreach()
# we make $path available to scripts ...
path=$sm_path
cd $sm_path
- eval $@
+ if test $# -gt 0 -o -z $post_order
+ then
+ say $enter_msg
+ eval $@ || die $die_msg
+ fi
if test -n $recursive
then
- cmd_foreach --recursive $@
+ # subshell will use parent-scoped
values
+ cmd_foreach $@
+ fi
+ if test -n $post_order
+ then
+ say $leave_msg
+ eval $post_order || die $die_msg
fi
) 3 3- ||
- die $(eval_gettext Stopping at '\$sm_path'; script
returned non-zero status.)
+ die $die_msg
fi
done
}
--
1.8.2.1.390.gd4ee029
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
