Re: [PATCH 2/3] submodule deinit: lose requirement for giving '.'

> Stefan Beller <sbeller@google.com> writes:
>
>> The discussion in [1] realized that '.' is a faulty suggestion as
>> there is a corner case where it fails:
>>
>>> "submodule deinit ." may have "worked" in the sense that you would
>>> have at least one path in your tree and avoided this "nothing
>>> matches" most of the time.  It would have still failed with the
>>> exactly same error if run in an empty repository, i.e.
>>>
>>>        $ E=/var/tmp/x/empty && rm -fr "$E" && mkdir -p "$E" && cd "$E"
>>>        $ git init
>>>        $ rungit v2.6.6 submodule deinit .
>>>        error: pathspec '.' did not match any file(s) known to git.
>>>        Did you forget to 'git add'?
>>>        $ >file && git add file
>>>        $ rungit v2.6.6 submodule deinit .
>>>        $ echo $?
>>>        0
>>
>> Allow no argument for `submodule deinit` to mean all submodules
>> and add a test to check for the corner case of an empty repository.
>>
>> There is no need to update the documentation as it did not describe the
>> special case '.' to remove all submodules.
>
> OK, and the reason why there is no need to update the actual code,
> other than the "do not allow" gate, is because "submodule--helper
> list" aka module_list already knows to list everything if no
> pathspec is given.  Am I reading the code (not in the patch)
> correctly?

>
>> [1] http://news.gmane.org/gmane.comp.version-control.git/289535
>
> The old discussion thread raises a good point.  The refusal to
> accept no-pathspec form for a potentially destructive "deinit" may
> have been a safety measure, even though the suggested way to tell
> the command "Yes, I know I want to deinit everything" was not a
> good one (i.e. it resulted in an error if your project did not have
> any files tracked yet).
>
> So possible ways forward may be
>
>  - to remove the safety altogether; or
>  - keep the safety, but give a better suggestion to say "Yes, deinit
>    everything".
>
> And this patch decides to take the former approach?

>
> I am wondering if this can be solved in a cleaner way to teach
> "deinit" take a new "--all" option instead, e.g. something like...
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 82e95a9..4b84116 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -405,6 +405,7 @@ cmd_init()
>  cmd_deinit()
>  {
>         # parse $args after "submodule ... deinit".
> +       deinit_all=
>         while test $# -ne 0
>         do
>                 case "$1" in
> @@ -414,6 +415,9 @@ cmd_deinit()
>                 -q|--quiet)
>                         GIT_QUIET=1
>                         ;;
> +               -a|--all)
> +                       deinit_all=t
> +                       ;;
>                 --)
>                         shift
>                         break
> @@ -428,9 +432,9 @@ cmd_deinit()
>                 shift
>         done
>
> -       if test $# = 0
> +       if test $# = 0 && test -z "$deinit_all"
>         then
> -               die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
> +               die "$(eval_gettext "Use '--all' if you really want to deinitialize all submodules")"
>         fi
>
>         git submodule--helper list --prefix "$wt_prefix" "$@" |
>
>
> That would work even in the pathological "empty directory that has
> nothing to match even '.'" case without losing the safety, no?