* [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 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 '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 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).