Re: [PATCHv3] submodule deinit: require '--all' instead of '.' for all submodules

From: Stefan Beller <sbeller@google.com>
Date: May 4, 2016, 21:57 UTC
Message-ID: <CAGZ79kbUJjqYbY3hC1W0Aw=gR49i4Q0BjY+qM9aiWG5ME89Ruw@mail.gmail.com>
In-Reply-To: <xmqqshxxeboj.fsf@gitster.mtv.corp.google.com>

On Wed, May 4, 2016 at 2:49 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 41 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>>> +    if test -n "$deinit_all" && test "$#" -ne 0
>>> +    then
>>> +            die "$(eval_gettext "usage: $dashless [--quiet] deinit [-f|--force] (--all | [--] <path>...)")"
>>
>> I doubt that "usage:" wants to go thru l10n.
>>
>> I suspect that it is more friendly to the user to say that in prose,
>> i.e.e.g. "--all and pathspec cannot be given at the same time", than
>> forcing them to grok the (alternative|possibilities).
>>
>>> +    fi
>>> +    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
>>
>> This is good.
>
> By the way, while it is a very good idea to die upon
>
>         $ git submodule deinit --all no-only-this-one
>
> it may not be too bad if we demoted the output to "info" with clean
> no-op exit when the user said
>
>         $ git submodule deinit
>
> IOW, the latter part _might_ be better if it were
>
>         if test $# = 0 && test -z "$deinit_all"
>         then
>                 echo >&2 "info: not deinitializing anything."
>                 echo >&2 "info: Use --all to deinitialize all submodules."
>                 exit 0
>         fi
>
> given that this is really about preventing mistakes from doing mass
> destruction.

Demote to a new class in a class of its own?

* grep -r "info:" gives no match for user facing code, so it's a
prefix you made up now.
* I assume we want to translate it?
* This would not respect the --quiet option?
* returning 0 as in "everyting is fine", while nothing is fine.
   There are two cases:
     - Either the user knows what they are doing (See Per complaining
about this)
       A very deliberate "I want it all gone or error out if you
cannot remove it all"
     - The user has no idea what they are doing. Exiting non zero
doesn't do any harm.

Maybe:

> -             die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
> +            usage()

instead?