From: Stefan Beller <sbeller@google.com>
Date: Thu, 05 May 2016 19:02:31 GMT
Subject: Re: [PATCHv4] submodule deinit: require '--all' instead of '.' for all submodules
Message-ID: <CAGZ79kbxDbmeh-rTHAqykjLBZ47HAviA11Q9LLYpcwT-Wr9X0A@mail.gmail.com>
In-Reply-To: <20160504235914.GD395@google.com>

On Wed, May 4, 2016 at 4:59 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

>>>
>>> Another option would be to call 'usage' and be done.
>>
>> I had that idea as well, but I think pointing out the low level is better
>> than giving the high level again, so the user immediately sees what's wrong.
>
> 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?"  Most likely the user was trying to do
> something other than specify a path, since they also passed --all.  If
> I run something like
>
>


    $ git submodule deinit force --all
    error: unknown option `all'
    usage: git submodule--helper list [--prefix=<path>] [<path>...]

        --prefix <path>       alternative anchor for relative paths

`force` is seen as the first pathspec, so "force --all" is given to the
`submodule--helper list`, which gives a less than optimal error message

    $ git submodule deinit --all force
gettext: unrecognized option '--all and pathspec are incompatible'
Try 'gettext --help' for more information.
envsubst: unrecognized option '--all and pathspec are incompatible'
Try 'envsubst --help' for more information.
envsubst: unrecognized option '--all and pathspec are incompatible'
Try 'envsubst --help' for more information.

We should not put --all as the first thing in the error message.

    $ git submodule deinit --all force
    pathspec and --all are incompatible

With the next patch this is the error message. I think the missing
dashes for force are quite visible as force is after --all.

>
> and the output tells me that --all and pathspec are incompatible then
> I just scratch my head more.
>
> We can do
>
>         USAGE="$dashless [--quiet] deinit [-f|--force] (--all | [--] <path>...)"
>         usage
>
> to print the subcommand's usage.  git commandline tools don't
> translate any usage strings today, so not getting translation here
> wouldn't feel out of place.

We can have both? I'd prefer not rewriting the USAGE string here
as it would easily be out of sync in the future?
I tried to just grep the deinit line from the USAGe though, but that doesn't
look right as it would need some post processing. (remove the "or:")
and processing the USAGE string also doesn't sound future proof.