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

[Stefan Beller <sbeller@google.com>]

On Thu, May 5, 2016 at 12:20 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> I mean low level as in implementation detail.  The human user would
>>> wonder "what is incompatible about them?  Why are you stopping me from
>>> what I am trying to do?"
>>
>> Maybe s/incompatible/inconsistent/ is what you are after?  Why are
>> you stopping me from what I am trying to do?  Because you are asking
>> to do two contradicting things.  Do you want to nuke everything, or
>> do you want to keep everything outside what you specified?
>>
>> After being saved countless times from a stupid mistake
>>
>>     $ git commit -a -s foo.c
>>
>> that was caused by habitually typing "-a", when I do want to limit
>> the changes to record to a specific path (or two) with similar
>> safety, I do not think "what is incompatible about them" is a valid
>> question.
>
> Yes, 'inconsistent' works better than 'incompatible'.  Stepping back,
> what I meant is that when I pass an invalid set of command line
> arguments, it's difficult to give advice back because it's not obvious
> what I intended to do.
>
> When I say "git submodule deinit --all -- foo/", I'm most likely trying
> to deinit all submodules within the foo subdirectory.  That's a
> perfectly consistent thing to want --- it just doesn't match what the
> command is expecting.  It is more helpful for the command to tell me
> what it is expecting me to do instead instead of just telling me that what
> I gave it is wrong.
>
> The ideal would be something like "git check-attr"'s error_with_usage.
> It tells in a targetted way where my error was and also gives a guide
> of what to do instead.
>
>         $ git submodule deinit --all lib/
>         error: paths with --all do not make sense
>         usage: git submodule deinit [-f | --force] (--all | [--] <path>...)
>
> Thanks,
> Jonathan
>
> -- >8 --
> Subject: git-sh-setup: add error_with_usage helper
>
> When given an impossible set of options, this allows a command to
> print a targetted message followed by a short usage string that tells
> the user both (1) what part of their command wasn't supported (what
> went wrong) and (2) what correct usage looks like (what to do
> instead).
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks for this patch, I can take that as a preparation for the patch
under discussion.

> ---
>  git-sh-setup.sh | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index c48139a..2b56636 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -63,6 +63,11 @@ say () {
>         fi
>  }
>
> +error_with_usage () {

As usage is the last thing (it's die(...) essentially),
I'd rename it to die_with_usage ?

> +       printf >&2 'error: %s\n' "$*"
> +       usage
> +}
> +
>  if test -n "$OPTIONS_SPEC"; then
>         usage() {
>                 "$0" -h
> --