[PATCH] ls-remote: a lone "-h" is asking for help

[PATCH] ls-remote: a lone "-h" is asking for help

[Junio C Hamano]

What should happen if you run this command?

	$ git ls-remote -h

It does not give a short-help for the command. Instead because "-h" is a
synonym for "--heads", it runs "git ls-remote --heads", and because there
is no remote specified on the command line, we run it against the default
"origin" remote, hence end up doing the same as

	$ git ls-remote --heads origin

Fix this counter-intuitive behaviour by special casing a lone "-h" that
does not have anything else on the command line and calling usage().

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/ls-remote.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 1022309..41c88a9 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -43,6 +43,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	struct transport *transport;
 	const struct ref *ref;
 
+	if (argc == 2 && !strcmp("-h", argv[1]))
+		usage(ls_remote_usage);
+
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 

Re: [PATCH] ls-remote: a lone "-h" is asking for help

[Sverre Rabbelier]

Heya,

On Fri, Sep 16, 2011 at 20:14, Junio C Hamano <gitster@pobox.com> wrote:
> It does not give a short-help for the command. Instead because "-h" is a
> synonym for "--heads", it runs "git ls-remote --heads", and because there
> is no remote specified on the command line, we run it against the default
> "origin" remote, hence end up doing the same as

Should we really have "-h" as a short for anything other than "--help"
in the first place?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] ls-remote: a lone "-h" is asking for help

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Should we really have "-h" as a short for anything other than "--help"
> in the first place?

You have been here long enough to know the answer to that question, no?

The answer would be different if you are starting a new project from
scratch and if you are talking about a project with existing userbase.

Re: [PATCH] ls-remote: a lone "-h" is asking for help

[Sverre Rabbelier]

Heya,

On Fri, Sep 16, 2011 at 21:35, Junio C Hamano <gitster@pobox.com> wrote:
> Sverre Rabbelier <srabbelier@gmail.com> writes:
>> Should we really have "-h" as a short for anything other than "--help"
>> in the first place?
>
> You have been here long enough to know the answer to that question, no?

Yeah, you're right :).

> The answer would be different if you are starting a new project from
> scratch and if you are talking about a project with existing userbase.

Does git 2.0 count?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] ls-remote: a lone "-h" is asking for help

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> On Fri, Sep 16, 2011 at 21:35, Junio C Hamano <gitster@pobox.com> wrote:
>> Sverre Rabbelier <srabbelier@gmail.com> writes:
>>> Should we really have "-h" as a short for anything other than "--help"
>>> in the first place?
> ...
> Does git 2.0 count?

I am not opposed to. We should do the usual "start from warning and then
deprecate" dance, but I do not think we would want to have a "I want the
old behaviour, please keep it" configuration, especially if we are talking
about a big version bump like 2.0.

The first step would look something like this, on top of the previous
patch.

 builtin/ls-remote.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 41c88a9..dabe21e 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -28,6 +28,12 @@ static int tail_match(const char **pattern, const char *path)
 	return 0;
 }
 
+static void warn_h_deprecation(void)
+{
+	warning("Using -h as synonym for --heads is deprecated");
+	warning("and will be removed in future versions of Git.");
+}
+
 int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
 	int i;
@@ -64,6 +70,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			}
 			if (!strcmp("--heads", arg) || !strcmp("-h", arg)) {
 				flags |= REF_HEADS;
+				if (!arg[2])
+					warn_h_deprecation();
 				continue;
 			}
 			if (!strcmp("--refs", arg)) {

Re: [PATCH] ls-remote: a lone "-h" is asking for help

[Sverre Rabbelier]

Heya,

On Fri, Sep 16, 2011 at 22:31, Junio C Hamano <gitster@pobox.com> wrote:
> I am not opposed to. We should do the usual "start from warning and then
> deprecate" dance, but I do not think we would want to have a "I want the
> old behaviour, please keep it" configuration, especially if we are talking
> about a big version bump like 2.0.
>
> The first step would look something like this, on top of the previous
> patch.

Makes sense.

I remember some sort of "this is for post 1.7.x" section in what's
cooking at some point. Should we have some way to queue patches like
this, or would someone have to resend after the appropriate release?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] ls-remote: a lone "-h" is asking for help

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> ... Should we have some way to queue patches like
> this, or would someone have to resend after the appropriate release?

For this particular case, I do not think it is worth _my_ time to keep
maintaining that patch.

It is understandable that a new person will be hurt if

	$ git ls-remote -h

does not give a short-help message and instead try to contact and show the
origin repository which may not even exist, and that is why I sent a fix.

But would anybody gets hurt if

	$ git ls-remote -h origin
        $ git ls-remote -h git://git.kernel.org/pub/git/git.git

kept working as they do today, given that we do not advertise -h as
a synonym in "git ls-remote -h" output?

That is why I said "I am not opposed to", and not "I'd volunteer to do
that". It's not worth my time, but since you brought it up, you may care
more about it.