* [PATCH] - Added command synopsis in code and edited them in manual
@ 2008-03-06 7:33 imyousuf
2008-03-06 7:33 ` [PATCH] - Added 'recurse' subcommand to git submodule imyousuf
2008-03-06 10:42 ` [PATCH] - Added command synopsis in code and edited them in manual Junio C Hamano
0 siblings, 2 replies; 8+ messages in thread
From: imyousuf @ 2008-03-06 7:33 UTC (permalink / raw)
To: git; +Cc: gitster, Imran M Yousuf
From: Imran M Yousuf <imyousuf@smartitengineering.com>
Added the command synopsis so that they are available for
any future command additions.
Quiet can also be specified using -q and it was missing in
the usage in the code and man page.
In the init/update command synopsis either of them is required
command as is add in its synopsis, so removed the square brackets
around them from the documentation
Signed-off-by: Imran M Yousuf <imyousuf@smartitengineering.com>
---
Documentation/git-submodule.txt | 6 +++---
git-submodule.sh | 6 +++++-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index e818e6e..595918e 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -9,9 +9,9 @@ git-submodule - Initialize, update or inspect submodules
SYNOPSIS
--------
[verse]
-'git-submodule' [--quiet] add [-b branch] [--] <repository> [<path>]
-'git-submodule' [--quiet] status [--cached] [--] [<path>...]
-'git-submodule' [--quiet] [init|update] [--] [<path>...]
+'git-submodule' [-q|--quiet] add [-b branch] [--] <repository> [<path>]
+'git-submodule' [-q|--quiet] [status] [--cached] [--] [<path>...]
+'git-submodule' [-q|--quiet] init|update [--] [<path>...]
COMMANDS
diff --git a/git-submodule.sh b/git-submodule.sh
index 67d3224..257be4c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -4,7 +4,11 @@
#
# Copyright (c) 2007 Lars Hjemli
-USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'
+# Synopsis of this commands are as follows
+# git-submodule [-q|--quiet] add [-b|--branch branch] <repository> [<path>]
+# git-submodule [-q|--quiet] [status] [-c|--cached] [--] [<path>...]
+# git-submodule [-q|--quiet] init|update [--] [<path>...]
+USAGE='[-q|--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'
OPTIONS_SPEC=
. git-sh-setup
require_work_tree
--
1.5.4.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] - Added 'recurse' subcommand to git submodule
2008-03-06 7:33 [PATCH] - Added command synopsis in code and edited them in manual imyousuf
@ 2008-03-06 7:33 ` imyousuf
2008-03-06 7:33 ` [PATCH] - Added pre command and custom argument support to git submodule recurse command imyousuf
2008-03-06 10:42 ` [PATCH] - Added 'recurse' subcommand to git submodule Junio C Hamano
2008-03-06 10:42 ` [PATCH] - Added command synopsis in code and edited them in manual Junio C Hamano
1 sibling, 2 replies; 8+ messages in thread
From: imyousuf @ 2008-03-06 7:33 UTC (permalink / raw)
To: git; +Cc: gitster, Imran M Yousuf
From: Imran M Yousuf <imyousuf@smartitengineering.com>
The purpose of the recurse command in the git submodule is to recurse
a command in its submodule. For example if one wants to do a diff on its
project with submodules at once, one can simply do
git-submodule recurse diff HEAD
and would see the diff for all the modules it contains.
The recurse commands behavior can be customized with several arguments
that it accepts. The synopsis for the recurse command is:
git-submodule [-q|--quiet] recurse [-i|--initialize]
[-e|--exit-after-error] [-d|--depth <recursion depth>]
[-df|--depth-first] [-ca|--customized-argument] [-p|--pre-command]
<command> [<arguments> ...]
When traversing modules, a module could be uninitialized that is git
submodule init and update has not been called for it; if [-i|--initialize]
option is specified, it will initialize any module that is not initialized;
else if the module is not initialized it will simply skip it.
There are commands that can fail for a certain submodule but succeed for
others; if one wants to stop execution once the top level module's execution
fails, one can specify [-e|--exit-after-error]. It will ensure that once
execution of git <command> fails in the top level module it will not recurse
into its submodules.
If the project has submodule hierarchy upto n depth and we want to restrict
recursion to (n-p) depth; we can use the [-d|--depth <recursion depth>] option.
Value has to be greater than 0 and command will at least recurse into the first
depth. If depth is specified to p than all depths <= p will be recursed over.
While discussion on the recurse command one thing which was put forward
in several occassions is that there might be scenario where a command should be
executed over the child module before the parent module. For such scenario
[-df|--depth-first] option can be used; one use case in particualar presented
as an example is git commit; where almost everybody mentioned that they prefer
to commit the child module before the parent and -df will enable just that.
E.g. p -> a, b, c, e; a ->d is a module structure. If the following command is
used,
git submodule recurse -df commit -a
it will execute git commit -a in the following sequence - d, a, b, c, e, p.
Signed-off-by: Imran M Yousuf <imyousuf@smartitengineering.com>
---
git-submodule.sh | 162 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 160 insertions(+), 2 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 257be4c..ee3c928 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,7 +8,8 @@
# git-submodule [-q|--quiet] add [-b|--branch branch] <repository> [<path>]
# git-submodule [-q|--quiet] [status] [-c|--cached] [--] [<path>...]
# git-submodule [-q|--quiet] init|update [--] [<path>...]
-USAGE='[-q|--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'
+# git-submodule [-q|--quiet] recurse [-i|--initialize] [-e|--exit-after-error] [-d|--depth <recursion depth>] [-df|--depth-first] <command> [<arguments> ...]
+USAGE='[-q|--quiet] [[[add [-b|--branch branch] <repo>]|[[[status [-c|--cached]]|init|update] [--]]] [<path>...]]|[recurse [-i|--initialize] [-e|--exit-after-error] [-d|--depth <recursion depth>] [-df|--depth-first] <command> [<arguments> ...]]'
OPTIONS_SPEC=
. git-sh-setup
require_work_tree
@@ -17,6 +18,11 @@ command=
branch=
quiet=
cached=
+depth=0
+current_depth=0
+auto_initialize=
+depth_first=
+on_error=
#
# print stuff on stdout unless -q was specified
@@ -386,6 +392,157 @@ cmd_status()
done
}
+# Initializes the submodule if already not initialized
+# and auto initialize is enabled
+initialize_sub_module()
+{
+ if test ! -d "$1"/.git &&
+ test -n "$auto_initialize"
+ then
+ say "Initializing and updating $1"
+ git-submodule init "$1" &&
+ git-submodule update "$1" &&
+ return 0
+ # Returns true if module is already initialized
+ elif test -d "$1"/.git
+ then
+ return 0
+ fi
+ say "Module $1 is not initialized and skipped"
+ return 1
+}
+
+# This module simply checks whether the depth is traverseable
+# in terms of depth and if so then it sequentially traverses
+# its submodules
+traverse_submodules()
+{
+ # If current depth is the range specified than it will continue
+ # else return with success
+ if test "$depth" -gt 0 &&
+ test "$current_depth" -ge "$depth"
+ then
+ return 0;
+ fi
+ # If submodules exists than it will traverse over them
+ if test -f .gitmodules
+ then
+ # Incrementing the depth for the next level of submodules
+ current_depth=$(($current_depth + 1))
+ for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
+ traverse_module "$mod_path" "$@"
+ done
+ # Decremented the depth to bring it back to the depth of
+ # the current module
+ current_depth=$(($current_depth - 1))
+ fi
+}
+
+# This actually traverses a module; checks
+# whether the module is initialized or not.
+# if not initialized, then tries to do so
+# based on the user preference and then the
+# intended command is evaluated in the
+# traversal manner requested - breadth first
+# or depth first. Then it# recursively goes
+# into it modules.
+traverse_module()
+{
+ # Will work in the module if and only if the module is initialized
+ initialize_sub_module "$1" &&
+ (
+ submod_path="$1"
+ shift
+ cd "$submod_path"
+ # If depth-first is specified in that case submodules are
+ # are traversed before executing the command on this module
+ test -n "$depth_first" && traverse_submodules "$@"
+ # pwd is mentioned in order to enable the ser to distinguish
+ # between same name modules, e.g. a/lib and b/lib.
+ say "Working in mod $submod_path" @ `pwd` "with $@ ($#)"
+ cmd_status=
+ git "$@" || cmd_status=1
+ # if exit on error is specifed than script will exit if any
+ # command fails. As there is no transaction there will be
+ # no rollback either
+ # TODO - If possible facilitate transaction
+ if test -n "$cmd_status" && test -n "$on_error"
+ then
+ die "git $@ failed in module $submod_path @ $(pwd)"
+ fi
+ # If depth-first is not specified in that case submodules are
+ # are traversed after executing the command on this module
+ test -z "$depth_first" && traverse_submodules "$@"
+ )
+}
+
+# Propagates or recurses over all the submodules at any
+# depth with any git command, e.g. git-clone, git-status,
+# git-commit etc., with the arguments supplied exactly as
+# it would have been supplied to the command otherwise.
+# This actually starts the recursive propagation
+cmd_recurse() {
+ while :
+ do
+ case "$1" in
+ -q|--quiet)
+ quiet=1
+ ;;
+ -d|--depth)
+ shift
+ if test -z "$1"
+ then
+ echo "No <recursion depth> specified"
+ usage
+ # Arithmatic operation will give an error if depth is not number
+ # thus chose to check intergerness with regular expression
+ elif test "$(expr $1 : '[1-9][0-9]*')" -eq "$(expr $1 : '.*')"
+ then
+ depth="$1"
+ else
+ echo "<recursion depth> not an integer"
+ usage
+ fi
+ ;;
+ -df|--depth-first)
+ depth_first=1
+ ;;
+ -e|--exit-after-error)
+ on_error=1
+ ;;
+ -i|--initialize)
+ auto_initialize=1
+ ;;
+ -p|--pre-command)
+ pre_cmd=1
+ ;;
+ -ca|--customized-argument)
+ use_custom_args=1
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+ test "$#" -le 0 && die "No git command specified"
+ project_home="$(pwd)"
+ if test "$depth" -gt 0
+ then
+ say Command will recurse upto "$depth" depth
+ fi
+ if test -d "$project_home"/.git/
+ then
+ say "Command to recurse: git $@"
+ traverse_module . "$@"
+ else
+ die "$project_home not a git repo thus exiting"
+ fi
+}
+
# This loop parses the command line arguments to find the
# subcommand name to dispatch. Parsing of the subcommand specific
# options are primarily done by the subcommand implementations.
@@ -395,7 +552,7 @@ cmd_status()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | init | update | status)
+ add | init | update | status | recurse)
command=$1
;;
-q|--quiet)
@@ -441,3 +598,4 @@ then
fi
"cmd_$command" "$@"
+
--
1.5.4.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] - Added pre command and custom argument support to git submodule recurse command
2008-03-06 7:33 ` [PATCH] - Added 'recurse' subcommand to git submodule imyousuf
@ 2008-03-06 7:33 ` imyousuf
2008-03-06 7:33 ` imyousuf
2008-03-06 10:42 ` Junio C Hamano
2008-03-06 10:42 ` [PATCH] - Added 'recurse' subcommand to git submodule Junio C Hamano
1 sibling, 2 replies; 8+ messages in thread
From: imyousuf @ 2008-03-06 7:33 UTC (permalink / raw)
To: git; +Cc: gitster, Imran M Yousuf
From: Imran M Yousuf <imyousuf@smartitengineering.com>
There is one scenario that has been put forward several times in
discussion over the recurse command - it is that commands can have
different arguments for different modules. For example for the same example
mentioned above, one wants to check a_1 for submdoule a, while it wants to
checkout d_2 for d. It can be achieved by using [-ca|--customized-argument].
This results the script to prompt for user input, which will be passed as
argument to the command for that module.
git submodule recurse -ca checkout
Working in mod a .......
Please provide arguments for this module: a_1
Working in mod d .......
Please provide arguments for this module: a_1
It is usually helpful that when typing a command, being able to see some options
come in handy. For example if I can see the available branches before checking
out a branch that would be useful, IOW, if one could git branch before git
checkout; it is now possible using the [-p|--pre-command] option. Using this
command you can actually execute other git commands before specifying the
arguments to the original command. E.g. if the above command is changed to,
git submodule recurse -ca -p checkout
it will prompt the user for the pre command until one is satisfied and later
the user can actually use them in the argument.
As these two options get along well together it made sense to me
to group them together.
Signed-off-by: Imran M Yousuf <imyousuf@smartitengineering.com>
---
git-submodule.sh | 33 +++++++++++++++++++++++++++++----
1 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index ee3c928..05fd1d2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,8 +8,8 @@
# git-submodule [-q|--quiet] add [-b|--branch branch] <repository> [<path>]
# git-submodule [-q|--quiet] [status] [-c|--cached] [--] [<path>...]
# git-submodule [-q|--quiet] init|update [--] [<path>...]
-# git-submodule [-q|--quiet] recurse [-i|--initialize] [-e|--exit-after-error] [-d|--depth <recursion depth>] [-df|--depth-first] <command> [<arguments> ...]
-USAGE='[-q|--quiet] [[[add [-b|--branch branch] <repo>]|[[[status [-c|--cached]]|init|update] [--]]] [<path>...]]|[recurse [-i|--initialize] [-e|--exit-after-error] [-d|--depth <recursion depth>] [-df|--depth-first] <command> [<arguments> ...]]'
+# git-submodule [-q|--quiet] recurse [-i|--initialize] [-e|--exit-after-error] [-d|--depth <recursion depth>] [-df|--depth-first] [-ca|--customized-argument] [-p|--pre-command] <command> [<arguments> ...]
+USAGE='[-q|--quiet] [[[add [-b|--branch branch] <repo>]|[[[status [-c|--cached]]|init|update] [--]]] [<path>...]]|[recurse [-i|--initialize] [-e|--exit-after-error] [-d|--depth <recursion depth>] [-df|--depth-first] [-ca|--customized-argument] [-p|--pre-command] <command> [<arguments> ...]]'
OPTIONS_SPEC=
. git-sh-setup
require_work_tree
@@ -392,6 +392,30 @@ cmd_status()
done
}
+do_pre_command()
+{
+ say "Starting pre-comamnd execution!"
+ while :
+ do
+ (
+ read -p "Please provide a git command: " pre_command
+ test -z "$pre_command" || git "$pre_command"
+ )
+ read -p "Press y to continue with another git command... " keypress
+ if test "$keypress" != "y" &&
+ test "$keypress" != "Y"
+ then
+ break
+ fi
+ done
+}
+
+# Take arguments from user to pass as custom arguments
+get_custom_args()
+{
+ read -p "Please provide arguments for this module: " custom_args
+}
+
# Initializes the submodule if already not initialized
# and auto initialize is enabled
initialize_sub_module()
@@ -460,8 +484,10 @@ traverse_module()
# pwd is mentioned in order to enable the ser to distinguish
# between same name modules, e.g. a/lib and b/lib.
say "Working in mod $submod_path" @ `pwd` "with $@ ($#)"
+ test -n "$pre_cmd" && do_pre_command
+ test -n "$use_custom_args" && get_custom_args
cmd_status=
- git "$@" || cmd_status=1
+ git "$@" "$custom_args" || cmd_status=1
# if exit on error is specifed than script will exit if any
# command fails. As there is no transaction there will be
# no rollback either
@@ -598,4 +624,3 @@ then
fi
"cmd_$command" "$@"
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] - Added pre command and custom argument support to git submodule recurse command
2008-03-06 7:33 ` [PATCH] - Added pre command and custom argument support to git submodule recurse command imyousuf
@ 2008-03-06 7:33 ` imyousuf
2008-03-06 7:33 ` imyousuf
2008-03-06 10:42 ` Junio C Hamano
1 sibling, 1 reply; 8+ messages in thread
From: imyousuf @ 2008-03-06 7:33 UTC (permalink / raw)
To: git; +Cc: gitster
A noteworthy change is that modules_list() is now known as
cmd_status(). There is no "submodule list" command.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* We could probably do something like this. This first part is
about making the command dispatcher maintainable.
Note that I haven't seriously tested this series. This and
the next one are primarily to illustrate what I think the fix
you are trying should look like.
git-submodule.sh | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index ad9fe62..3c104e3 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -86,9 +86,9 @@ module_name()
#
# Clone a submodule
#
-# Prior to calling, modules_update checks that a possibly existing
+# Prior to calling, cmd_update checks that a possibly existing
# path is not a git repository.
-# Likewise, module_add checks that path does not exist at all,
+# Likewise, cmd_add checks that path does not exist at all,
# since it is the location of a new submodule.
#
module_clone()
@@ -121,7 +121,7 @@ module_clone()
#
# optional branch is stored in global branch variable
#
-module_add()
+cmd_add()
{
repo=$1
path=$2
@@ -174,7 +174,7 @@ module_add()
#
# $@ = requested paths (default to all)
#
-modules_init()
+cmd_init()
{
git ls-files --stage -- "$@" | grep -e '^160000 ' |
while read mode sha1 stage path
@@ -207,7 +207,7 @@ modules_init()
#
# $@ = requested paths (default to all)
#
-modules_update()
+cmd_update()
{
git ls-files --stage -- "$@" | grep -e '^160000 ' |
while read mode sha1 stage path
@@ -266,7 +266,7 @@ set_name_rev () {
#
# $@ = requested paths (default to all)
#
-modules_list()
+cmd_status()
{
git ls-files --stage -- "$@" | grep -e '^160000 ' |
while read mode sha1 stage path
@@ -347,16 +347,16 @@ esac
case "$add,$init,$update,$status,$cached" in
1,,,,)
- module_add "$@"
+ cmd_add "$@"
;;
,1,,,)
- modules_init "$@"
+ cmd_init "$@"
;;
,,1,,)
- modules_update "$@"
+ cmd_update "$@"
;;
,,,*,*)
- modules_list "$@"
+ cmd_status "$@"
;;
*)
usage
--
1.5.3.7
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] - Added pre command and custom argument support to git submodule recurse command
2008-03-06 7:33 ` imyousuf
@ 2008-03-06 7:33 ` imyousuf
0 siblings, 0 replies; 8+ messages in thread
From: imyousuf @ 2008-03-06 7:33 UTC (permalink / raw)
To: git; +Cc: gitster
$ git submodule add init update
which is meant to add a submodule called 'init' at path 'update'
was misinterpreted as a request to invoke more than one mutually
incompatible subcommands and incorrectly rejected.
This patch fixes the issue by stopping the subcommand parsing at
the first subcommand word, to allow the sample command line
above to work as expected.
It also introduces the usual -- option disambiguator, so that a
submodule at path '-foo' can be updated with
$ git submodule update -- -foo
without triggering an "unrecognized option -foo" error.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* And this is the second part to fix the real issue.
git-submodule.sh | 157 ++++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 116 insertions(+), 41 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 3c104e3..a6aaf40 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -9,11 +9,8 @@ OPTIONS_SPEC=
. git-sh-setup
require_work_tree
-add=
+command=
branch=
-init=
-update=
-status=
quiet=
cached=
@@ -123,6 +120,32 @@ module_clone()
#
cmd_add()
{
+ # parse $args after "submodule ... add".
+ while test $# -ne 0
+ do
+ case "$1" in
+ -b | --branch)
+ case "$2" in '') usage ;; esac
+ branch=$2
+ shift
+ ;;
+ -q|--quiet)
+ quiet=1
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
repo=$1
path=$2
@@ -176,6 +199,27 @@ cmd_add()
#
cmd_init()
{
+ # parse $args after "submodule ... init".
+ while test $# -ne 0
+ do
+ case "$1" in
+ -q|--quiet)
+ quiet=1
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
git ls-files --stage -- "$@" | grep -e '^160000 ' |
while read mode sha1 stage path
do
@@ -209,6 +253,27 @@ cmd_init()
#
cmd_update()
{
+ # parse $args after "submodule ... update".
+ while test $# -ne 0
+ do
+ case "$1" in
+ -q|--quiet)
+ quiet=1
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
git ls-files --stage -- "$@" | grep -e '^160000 ' |
while read mode sha1 stage path
do
@@ -268,6 +333,30 @@ set_name_rev () {
#
cmd_status()
{
+ # parse $args after "submodule ... status".
+ while test $# -ne 0
+ do
+ case "$1" in
+ -q|--quiet)
+ quiet=1
+ ;;
+ --cached)
+ cached=1
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
git ls-files --stage -- "$@" | grep -e '^160000 ' |
while read mode sha1 stage path
do
@@ -293,20 +382,17 @@ cmd_status()
done
}
-while test $# != 0
+# This loop parses the command line arguments to find the
+# subcommand name to dispatch. Parsing of the subcommand specific
+# options are primarily done by the subcommand implementations.
+# Subcommand specific options such as --branch and --cached are
+# parsed here as well, for backward compatibility.
+
+while test $# != 0 && test -z "$command"
do
case "$1" in
- add)
- add=1
- ;;
- init)
- init=1
- ;;
- update)
- update=1
- ;;
- status)
- status=1
+ add | init | update | status)
+ command=$1
;;
-q|--quiet)
quiet=1
@@ -335,30 +421,19 @@ do
shift
done
-case "$add,$branch" in
-1,*)
- ;;
-,)
- ;;
-,*)
+# No command word defaults to "status"
+test -n "$command" || command=status
+
+# "-b branch" is accepted only by "add"
+if test -n "$branch" && test "$command" != add
+then
usage
- ;;
-esac
-
-case "$add,$init,$update,$status,$cached" in
-1,,,,)
- cmd_add "$@"
- ;;
-,1,,,)
- cmd_init "$@"
- ;;
-,,1,,)
- cmd_update "$@"
- ;;
-,,,*,*)
- cmd_status "$@"
- ;;
-*)
+fi
+
+# "--cached" is accepted only by "status"
+if test -n "$cached" && test "$command" != status
+then
usage
- ;;
-esac
+fi
+
+"cmd_$command" "$@"
--
1.5.4.rc3.11.g4e67
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] - Added 'recurse' subcommand to git submodule
2008-03-06 7:33 ` [PATCH] - Added 'recurse' subcommand to git submodule imyousuf
2008-03-06 7:33 ` [PATCH] - Added pre command and custom argument support to git submodule recurse command imyousuf
@ 2008-03-06 10:42 ` Junio C Hamano
1 sibling, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2008-03-06 10:42 UTC (permalink / raw)
To: imyousuf; +Cc: git, Imran M Yousuf
imyousuf@gmail.com writes:
> The purpose of the recurse command in the git submodule is to recurse
s/command/sub&/;
> a command in its submodule. For example if one wants to do a diff on its
> project with submodules at once, one can simply do
> git-submodule recurse diff HEAD
> and would see the diff for all the modules it contains.
Can we please have a blank line around the example command line to make it
visually stand out more?
> The recurse commands behavior can be customized with several arguments
> that it accepts. The synopsis for the recurse command is:
>
> git-submodule [-q|--quiet] recurse [-i|--initialize]
> [-e|--exit-after-error] [-d|--depth <recursion depth>]
> [-df|--depth-first] [-ca|--customized-argument] [-p|--pre-command]
> <command> [<arguments> ...]
>
> When traversing modules, a module could be uninitialized that is git
> submodule init and update has not been called for it; if [-i|--initialize]
> option is specified, it will initialize any module that is not initialized;
> else if the module is not initialized it will simply skip it.
I really do not think the -i option should exist. "init" is a conscious
action and should not be a side effect of something else. (Why doesn't
"git submodule status -i" exist? ;-)
I do not mind "git submodule recurse init", though. "git submodule
recurse update" might also be a natural thing to do.
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 257be4c..ee3c928 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -8,7 +8,8 @@
> # git-submodule [-q|--quiet] add [-b|--branch branch] <repository> [<path>]
> # git-submodule [-q|--quiet] [status] [-c|--cached] [--] [<path>...]
> # git-submodule [-q|--quiet] init|update [--] [<path>...]
> -USAGE='[-q|--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'
> +# git-submodule [-q|--quiet] recurse [-i|--initialize] [-e|--exit-after-error] [-d|--depth <recursion depth>] [-df|--depth-first] <command> [<arguments> ...]
> +USAGE='[-q|--quiet] [[[add [-b|--branch branch] <repo>]|[[[status [-c|--cached]]|init|update] [--]]] [<path>...]]|[recurse [-i|--initialize] [-e|--exit-after-error] [-d|--depth <recursion depth>] [-df|--depth-first] <command> [<arguments> ...]]'
I commented on the overlong USAGE and also not-so-useful comments that
would only hurt maintainability in the previous message. The comment you
have above would look reasonably good in LONG_USAGE without leading "#",
don't you think?
> +# This module simply checks whether the depth is traverseable
s/module/function/; I wouldn't nitpick like this, but in the context of
"git-submodule", saying "module" when you do not mean the _module_ you are
working on is misleading.
> +# This actually traverses a module; checks
> +# whether the module is initialized or not.
> +# if not initialized, then tries to do so
> +# based on the user preference and then the
> +# intended command is evaluated in the
> +# traversal manner requested - breadth first
> +# or depth first. Then it# recursively goes
> +# into it modules.
Overnarrow lines are harder to read as well as overlong ones...
> +traverse_module()
> +{
> + # Will work in the module if and only if the module is initialized
> + initialize_sub_module "$1" &&
> + (
> + submod_path="$1"
> + shift
> + cd "$submod_path"
> + # If depth-first is specified in that case submodules are
> + # are traversed before executing the command on this module
> + test -n "$depth_first" && traverse_submodules "$@"
> + # pwd is mentioned in order to enable the ser to distinguish
> + # between same name modules, e.g. a/lib and b/lib.
> + say "Working in mod $submod_path" @ `pwd` "with $@ ($#)"
This feels more like a debug message than progress report useful for the
end users. Perhaps:
say "git submodule recurse $submod_path $*"
(which is modeled after how "diff -r" repeats itself) would be enough.
By the way, please make it the habit of using $@ only when you are asking
for the magic field splitting with "$@"; interpolating all params inside a
single string should be done with "$*". say() happens to take multiple
parameters, so your code happens to work Ok, but it is error prone; I do
not think you deliberately tried to send multiple parameters to the above
"say" by using "$@", knowing what this piece of code would do:
frotz () {
echo $#; for i; do echo $i; done
}
set b c d
frotz "a $@ e"
> + cmd_status=
> + git "$@" || cmd_status=1
> + # if exit on error is specifed than script will exit if any
> + # command fails. As there is no transaction there will be
> + # no rollback either
> + # TODO - If possible facilitate transaction
You can test $? here without $cmd_status.
> + if test -n "$cmd_status" && test -n "$on_error"
Excess SP between "if test".
> + then
> + die "git $@ failed in module $submod_path @ $(pwd)"
Same issue with $@ vs $*, and excess $submod_path vs the remainder cruft.
If the issue you wanted to solve with $(pwd) was that $submod_path is a
local path within the current submodule, a better way to solve it would be
to pass another "full path from the top" around when recursing into a new
sublevel.
> +# Propagates or recurses over all the submodules at any
> +# depth with any git command, e.g. git-clone, git-status,
> +# git-commit etc., with the arguments supplied exactly as
> +# it would have been supplied to the command otherwise.
> +# This actually starts the recursive propagation
> +cmd_recurse() {
> + while :
> + do
> ...
> + elif test "$(expr $1 : '[1-9][0-9]*')" -eq "$(expr $1 : '.*')"
What is this doing? $1 is underquoted here, by the way.
> + -df|--depth-first)
> + depth_first=1
> + ;;
Single dash followed by two letters is a somewhat unconventional option
flag.
> + -ca|--customized-argument)
> + use_custom_args=1
> + ;;
Who uses this and other options? The series seems to be split
incorrectly.
> + project_home="$(pwd)"
> + if test "$depth" -gt 0
> + then
> + say Command will recurse upto "$depth" depth
> + fi
> + if test -d "$project_home"/.git/
> + then
> + say "Command to recurse: git $@"
> + traverse_module . "$@"
These "say" are too noisy, compared to other existing uses. It feels as
if the command is being run with --debug option.
> @@ -441,3 +598,4 @@ then
> fi
>
> "cmd_$command" "$@"
> +
Adds trailing blank line.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] - Added pre command and custom argument support to git submodule recurse command
2008-03-06 7:33 ` [PATCH] - Added pre command and custom argument support to git submodule recurse command imyousuf
2008-03-06 7:33 ` imyousuf
@ 2008-03-06 10:42 ` Junio C Hamano
1 sibling, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2008-03-06 10:42 UTC (permalink / raw)
To: imyousuf; +Cc: git, Imran M Yousuf
imyousuf@gmail.com writes:
> From: Imran M Yousuf <imyousuf@smartitengineering.com>
>
> There is one scenario that has been put forward several times in
> discussion over the recurse command - it is that commands can have
> different arguments for different modules. For example for the same example
> mentioned above, one wants to check a_1 for submdoule a, while it wants to
> checkout d_2 for d. It can be achieved by using [-ca|--customized-argument].
Read this again, notice "for the same example mentioned above", and go
"Huh?"
> This results the script to prompt for user input, which will be passed as
> argument to the command for that module.
> git submodule recurse -ca checkout
> Working in mod a .......
> Please provide arguments for this module: a_1
> Working in mod d .......
> Please provide arguments for this module: a_1
Again, a blank line before the displayed script like this would make it
easier to read.
A single dash with two letters for an option name is somewhat
unconventional. Shouldn't this be called interactive arguments, by the
way?
> It is usually helpful that when typing a command, being able to see some options
> come in handy. For example if I can see the available branches before checking
> out a branch that would be useful, IOW, if one could git branch before git
> checkout; it is now possible using the [-p|--pre-command] option. Using this
> command you can actually execute other git commands before specifying the
> arguments to the original command. E.g. if the above command is changed to,
> git submodule recurse -ca -p checkout
> it will prompt the user for the pre command until one is satisfied and later
> the user can actually use them in the argument.
Btw, can we please try to keep commit log messages readable?
The above "blob of text" could/should have more structure than being just
one big block, and could have been structured as a few shorter paragraphs
to make it easier to read.
I don't know about you guys, but I read a *lot* of emails (and commit
messages), and I hate seeing big blobs of text without structure. Give it
a few breaks to make it easier to read, like just making new paragraphs,
i.e. something like:
> When typing a command, being able to see some options come in handy.
> For example, if a command asks for an option to "git checkout", being
> able to run "git branch" to see what branches exist before answering
> might help the user.
>
> "git submodule recurse" allows this with the [-p|--pre-command]
> option. With this option, you can actually execute other git commands
> before specifying the arguments to the original command. E.g. if the
> above command is changed to:
>
> git submodule recurse -ca -p checkout
>
> it will prompt the user for the pre command until one is satisfied and
> later the user can actually use them in the argument.
and now you have a bit of a breather space and some visual cues for where
you are in the text.
Yeah, maybe it's just me, but I like my whitespace. Ihaveareallyhardtime
readingtextthatdoesn'thavethepropermarkersforwhereconceptsstartandbegin,
andthatreallydoesincludetheverticalwhitespacetoo.
By the way, I do not find your example particularly convincing.
> +do_pre_command()
> +{
> + say "Starting pre-comamnd execution!"
> + while :
> + do
> + (
> + read -p "Please provide a git command: " pre_command
"read -p"? That's not even in POSIX. Please don't.
> + test -z "$pre_command" || git "$pre_command"
I am not convinced. Why do you limit it only to a git command? Why do
you limit it only to a git command that does not take any parameters? How
is this more useful over \C-z and returning to a shell, or examining the
situation in a different window/screen?
> +}
> +
> +# Take arguments from user to pass as custom arguments
> +get_custom_args()
> +{
> + read -p "Please provide arguments for this module: " custom_args
> +}
Contrary to its name, it reads a _single_ argument,...
> # Initializes the submodule if already not initialized
> # and auto initialize is enabled
> initialize_sub_module()
> @@ -460,8 +484,10 @@ traverse_module()
> # pwd is mentioned in order to enable the ser to distinguish
> # between same name modules, e.g. a/lib and b/lib.
> say "Working in mod $submod_path" @ `pwd` "with $@ ($#)"
> + test -n "$pre_cmd" && do_pre_command
> + test -n "$use_custom_args" && get_custom_args
> cmd_status=
> - git "$@" || cmd_status=1
> + git "$@" "$custom_args" || cmd_status=1
... and passes it as a single argument.
The overall structure of recursing into and running arbitrary commands
inside each submodule might be useful, but the implementation feels rather
too limiting.
Come to think of it, does it really matter that the command you run by
recursing into them is limited to "git-foo" command? I do not see you are
taking advantage of it being a git command, so it feels like an arbitrary
restriction to me, too.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] - Added command synopsis in code and edited them in manual
2008-03-06 7:33 [PATCH] - Added command synopsis in code and edited them in manual imyousuf
2008-03-06 7:33 ` [PATCH] - Added 'recurse' subcommand to git submodule imyousuf
@ 2008-03-06 10:42 ` Junio C Hamano
1 sibling, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2008-03-06 10:42 UTC (permalink / raw)
To: imyousuf; +Cc: git, Imran M Yousuf
imyousuf@gmail.com writes:
> From: Imran M Yousuf <imyousuf@smartitengineering.com>
>
> Added the command synopsis so that they are available for
> any future command additions.
I think you are talking about the comments you added at the beginning of
the script; I do not particularly see it as improvement. Rather, it
is just an additional maintenance burden that risks going out of sync with
the documentation.
It may make more sense to make your added lines into one line per
subcommand descriptions in LONG_USAGE, and shorten USAGE to just mention
"git submodule <command> <options>". The command is multi-featured enough
that it can afford to have a long on-line usage text, and I am reasonably
sure you would agree with me if you read your later patches that cram tons
of options on a single line USAGE. It simply is unreadable, no matter how
wide your terminal is.
By the way, please use imperative, e.g. "Add gostak so that doshes are
properly distimmed", instead of past tense "Added synopsis".
> In the init/update command synopsis either of them is required
> command as is add in its synopsis, so removed the square brackets
> around them from the documentation
But without grouping, the reader cannot tell where the alternation begins
and ends. Typically we use () in our documentation set.
That reminds me of the topic of marking "either this or that, you must
have one" with { this | that }, and that is more in line with other
systems' documentation, and is also consistent with what POSIX recommends.
I think the list atmosphere back then was "Yeah, {} may be more kosher,
but we have been consistently using () and that is not misleading, so
unless we convert everything consistently, using {} at only a few places
makes it even worse." I personally do not mind patches to convert
everybody to {} if we are confident that we can finish it before -rc1.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-03-06 10:43 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-06 7:33 [PATCH] - Added command synopsis in code and edited them in manual imyousuf
2008-03-06 7:33 ` [PATCH] - Added 'recurse' subcommand to git submodule imyousuf
2008-03-06 7:33 ` [PATCH] - Added pre command and custom argument support to git submodule recurse command imyousuf
2008-03-06 7:33 ` imyousuf
2008-03-06 7:33 ` imyousuf
2008-03-06 10:42 ` Junio C Hamano
2008-03-06 10:42 ` [PATCH] - Added 'recurse' subcommand to git submodule Junio C Hamano
2008-03-06 10:42 ` [PATCH] - Added command synopsis in code and edited them in manual Junio C Hamano
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).