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

Jonathan Nieder [off-list ref] 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.


    

Junio C Hamano wrote:

Jonathan Nieder [off-list ref] writes:

quoted

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

-- 

On Thu, May 5, 2016 at 12:20 PM, Jonathan Nieder [off-list ref] wrote:

Junio C Hamano wrote:

quoted

Jonathan Nieder [off-list ref] writes:

quoted

quoted

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 <redacted>

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

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