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

From: Junio C Hamano <gitster@pobox.com>
Date: May 4, 2016, 21:49 UTC
Message-ID: <xmqqshxxeboj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqlh3pft91.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 18 quoted lines

>> +	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.