Jens Lehmann <jens.lehm...@web.de> writes:
> Here we go, changes to v4 are:
>
> - I decided to do the proposed special casing for "."; no messages
> about uninitialized submodules will show up anymore (this is also
> tested)
> - The spelling fixes and better 'uninitialized' message Phil proposed
> - Added the missing quotation for $sm_path in output strings
> - "deinit" is added to the submodule completion list
> - Added two missing "&&" in t7400
Thanks for being thorough. I honestly did not expect this topic to
take this many cycles before becoming 'next-ready'.
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 004c034..0fb6ee0 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -547,6 +548,82 @@ cmd_init()
> }
>
> #
> +# Unregister submodules from .git/config and remove their work tree
> +#
> +# $@ = requested paths (use '.' to deinit all submodules)
> +#
> +cmd_deinit()
> +{
> + # parse $args after "submodule ... init".
> + while test $# -ne 0
> + do
> ..
> + done
> +
> + if test $# = 0
> + then
> + die "$(eval_gettext "Use '.' if you really want to deinitialize
> all submodules")"
I do not think I saw anybody mentioned this so far, but how is
"deinit" supposed to work inside a subdirectory of a superproject?
If the answer is to work on submodules appear in that subdirectory,
'.' should probably not mean "all in the superproject" I think?
> + module_list "$@" |
> + while read mode sha1 stage sm_path
> + do
> + die_if_unmatched "$mode"
> + name=$(module_name "$sm_path") || exit
> + url=$(git config submodule."$name".url)
> + if test -z "$url"
> + then
> + test $# -ne 1 || test "$@" = "." ||
> + say "$(eval_gettext "Submodule '\$name' is not
> initialized for path '\$sm_path'")"
> + continue
> + fi
This 'test "$@" = "."' makes readers feel uneasy. This particular
invocation happens to be safe only because it is protected with
"test $# -ne 1 ||", but for all other values of $# this will result
in a syntax error. 'test "$1" = "."' would make the intention of
the check more clear.
But stepping back a bit, is the condition this test is trying to
warn against really worth warning?
It seems that this "warn if user told us to deinitialize a submodule
that hasn't been initialized" is from the very initial round of this
series, and not something other people asked for during the review.
If somebody did
git submodule deinit foo bar
and then later said:
git submodule deinit foo
would it a mistake that the user wants to be warned about?
Perhaps the user did not mean to deinitialize foo (e.g. wanted to
*initialize* foo instead, or wanted to deinitialize *foz* instead)
and that is worth warning about? I am not sure, but I have a
feeling that we can do without this check.
Also the value of submodule.$name.url is not used in the later
codepath to ensure that the named submodule is in the pristine state
in the superproject's working tree (i.e. no submodule.$name section
in the local configuration, no working tree for that submodule), so
it may be even a good change to remove the "does submodule.$name.url
exist" check and always do the "deinitialize" process. That would
give the users a way to recover from a state where a submodule is
only half initialized for some reason safely, no?
--
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
