diff mbox series

[v1,4/4] miscellaneous: avoid "too many arguments"

Message ID 20240806003539.3292562-5-gitster@pobox.com (mailing list archive)
State New, archived
Headers show
Series make "too many arguments" a bit more useful | expand

Commit Message

Junio C Hamano Aug. 6, 2024, 12:35 a.m. UTC
Imagine seeing your command failing with "too many arguments" when
you run "git cmd foo bar baz".  Can you tell it will work if you
said "git cmd foo bar"?  Or is that trimming your command line too
much?  Too little?

Instead, if the command reports "unknown argument: 'bar'", you'd know
that "bar" and everything after it is unwanted.

Let's make it so for a few remaining commands.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/prune-packed.c | 6 +++---
 builtin/receive-pack.c | 3 ++-
 builtin/tag.c          | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

Comments

Eric Sunshine Aug. 6, 2024, 2:31 a.m. UTC | #1
On Mon, Aug 5, 2024 at 8:36 PM Junio C Hamano <gitster@pobox.com> wrote:
> Imagine seeing your command failing with "too many arguments" when
> you run "git cmd foo bar baz".  Can you tell it will work if you
> said "git cmd foo bar"?  Or is that trimming your command line too
> much?  Too little?
>
> Instead, if the command reports "unknown argument: 'bar'", you'd know
> that "bar" and everything after it is unwanted.
>
> Let's make it so for a few remaining commands.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> @@ -2503,7 +2503,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
>         if (argc > 1)
> -               usage_msg_opt(_("too many arguments"), receive_pack_usage, options);
> +               usage_msg_optf(_("unknown argument: '%s'"),
> +                              receive_pack_usage, options, argv[0]);

Is this supposed to be referencing `argv[1]` rather than `argv[0]`...

> diff --git a/builtin/tag.c b/builtin/tag.c
> @@ -641,7 +641,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>         if (argc > 2)
> -               die(_("too many arguments"));
> +               die(_("unknown argument: '%s'"), argv[2]);

...similar to how this references `argv[2]` when the condition is `argc > 2`?
Junio C Hamano Aug. 6, 2024, 4:50 p.m. UTC | #2
Eric Sunshine <sunshine@sunshineco.com> writes:

> On Mon, Aug 5, 2024 at 8:36 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Imagine seeing your command failing with "too many arguments" when
>> you run "git cmd foo bar baz".  Can you tell it will work if you
>> said "git cmd foo bar"?  Or is that trimming your command line too
>> much?  Too little?
>>
>> Instead, if the command reports "unknown argument: 'bar'", you'd know
>> that "bar" and everything after it is unwanted.
>>
>> Let's make it so for a few remaining commands.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
>> @@ -2503,7 +2503,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
>>         if (argc > 1)
>> -               usage_msg_opt(_("too many arguments"), receive_pack_usage, options);
>> +               usage_msg_optf(_("unknown argument: '%s'"),
>> +                              receive_pack_usage, options, argv[0]);
>
> Is this supposed to be referencing `argv[1]` rather than `argv[0]`...

You are right.  To the command, the first argument is acceptable,
and the unexpected ones are the second and later ones, so argv[1] is
what it should have said.

>
>> diff --git a/builtin/tag.c b/builtin/tag.c
>> @@ -641,7 +641,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>>         if (argc > 2)
>> -               die(_("too many arguments"));
>> +               die(_("unknown argument: '%s'"), argv[2]);
>
> ...similar to how this references `argv[2]` when the condition is `argc > 2`?

Yes.
diff mbox series

Patch

diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index ca3578e158..46989e12f9 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c
@@ -23,9 +23,9 @@  int cmd_prune_packed(int argc, const char **argv, const char *prefix)
 			     prune_packed_usage, 0);
 
 	if (argc > 0)
-		usage_msg_opt(_("too many arguments"),
-			      prune_packed_usage,
-			      prune_packed_options);
+		usage_msg_optf(_("unknown argument: '%s'"),
+			       prune_packed_usage, prune_packed_options,
+			       argv[0]);
 
 	prune_packed_objects(opts);
 	return 0;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 339524ae2a..e49f4ea4dd 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2503,7 +2503,8 @@  int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, receive_pack_usage, 0);
 
 	if (argc > 1)
-		usage_msg_opt(_("too many arguments"), receive_pack_usage, options);
+		usage_msg_optf(_("unknown argument: '%s'"),
+			       receive_pack_usage, options, argv[0]);
 	if (argc == 0)
 		usage_msg_opt(_("you must specify a directory"), receive_pack_usage, options);
 
diff --git a/builtin/tag.c b/builtin/tag.c
index a1fb218512..274bd0e6ce 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -641,7 +641,7 @@  int cmd_tag(int argc, const char **argv, const char *prefix)
 
 	object_ref = argc == 2 ? argv[1] : "HEAD";
 	if (argc > 2)
-		die(_("too many arguments"));
+		die(_("unknown argument: '%s'"), argv[2]);
 
 	if (repo_get_oid(the_repository, object_ref, &object))
 		die(_("Failed to resolve '%s' as a valid ref."), object_ref);