Re: [PATCH 1/4] git submodule: Teach add to accept --group
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 20, 2016, 21:18 UTC
- Message-ID
- <xmqq1t9c54qz.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1453260880-628-2-git-send-email-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
Show 9 quoted lines
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 6fce0dc..ab0f209 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -130,6 +130,7 @@ cmd_add()
> {
> # parse $args after "submodule ... add".
> reference_path=
> + submodule_groups=This can just be called $groups in the context of this script. I do not foresee we would be planning to deal with other kinds of groups here.
Show 11 quoted lines
> while test $# -ne 0
> do
> case "$1" in
> @@ -165,6 +166,10 @@ cmd_add()
> --depth=*)
> depth=$1
> ;;
> + -g|--group)
> + submodule_groups=${submodule_groups:+${submodule_groups};}"$2"
> + shift
> + ;;You would want to accept "--group=<name>" as well, just like existing --reference and --depth do. It won't be much more code, and when you move to C (hence parse_options) you'd get it for free anyway.
Show 8 quoted lines
> @@ -292,6 +297,16 @@ Use -f if you really want to add it." >&2 > > git config -f .gitmodules submodule."$sm_name".path "$sm_path" && > git config -f .gitmodules submodule."$sm_name".url "$repo" && > + if test -n "$submodule_groups" > + then > + OIFS=$IFS > + IFS=';'
I do not quite understand the choice of ';' here. If and only if you _must_ accept multi-word name that has spaces in between as a group name, the above construct may make sense, but I do not think we have such requirement. Why not separate with $IFS letters just like any other normal list managed in shell scripts do? Is there anything special about names of submodule groups?