Re: [PATCHv4] submodule deinit: require '--all' instead of '.' for all submodules
- From
Stefan Beller <sbeller@google.com>
- Date
- May 5, 2016, 19:35 UTC
- Message-ID
- <CAGZ79kbsgddnrF+oibSrpRu9e3PP2DTWMesqsdHCDHdySnhTyQ@mail.gmail.com>
- In-Reply-To
- <20160505192036.GA22726@google.com>
On Thu, May 5, 2016 at 12:20 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 54 quoted lines
> 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.
Show 13 quoted lines
> ---
> 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 ?
Show 8 quoted lines
> + printf >&2 'error: %s\n' "$*"
> + usage
> +}
> +
> if test -n "$OPTIONS_SPEC"; then
> usage() {
> "$0" -h
> --