git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH v2] refs: avoid "too many arguments"
Date: Tue, 06 Aug 2024 10:47:16 -0700	[thread overview]
Message-ID: <xmqqed71tufv.fsf_-_@gitster.g> (raw)
In-Reply-To: <20240806003539.3292562-2-gitster@pobox.com> (Junio C. Hamano's message of "Mon, 5 Aug 2024 17:35:36 -0700")

Running "git refs migrate master main" would fail and say "too many
arguments".  By reading that message, you cannot tell if you just
should have given a single ref and made it "git refs migrate
master", or the command refuses to take any arguments.

Instead, report that "git ref migrate" takes no arguments, which is
far easier for the user to understand.

    $ git refs migrate master main
    fatal: 'git refs migrate' takes no arguments

The other side of the coin this change is covering is to remove
doubts in new users' minds when we say "git refs migrate", if it is
"git" command running with two "refs migrate" arguments, "git refs"
command running with one "migrate" argument, or "git refs migrate"
command running with no arguments.

In the same spirit, reword the existing "missing --ref-format=<format>"
message and say

    $ git refs migrate
    fatal: 'git refs migrate' needs '--ref-format=<format>'

Note that we are turning two usage() calls to die() calls.  The
former should signal that the message given is a command line that
shows the usage help of the command.  If we are giving a fatal error
message, we should not hesitate to use die().

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * This is no longer a series.  I may resurrect [2,3,4/4] later, but
   they do not share any new helper routines at this moment, so they
   can stay independent patches.  Or perhaps while looking at this
   use case deeper during the review of this patch, we might come up
   with a good way to address this issue for many other scenarios
   and make a set of helper routines, in which case those other
   steps may have to become a part of series again to take advantage
   of them.

 builtin/refs.c          |  4 ++--
 t/t1460-refs-migrate.sh | 10 ++--------
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/builtin/refs.c b/builtin/refs.c
index 46dcd150d4..a51602f84b 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -30,9 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, options, migrate_usage, 0);
 	if (argc)
-		usage(_("too many arguments"));
+		die(_("'git refs migrate' takes no arguments"));
 	if (!format_str)
-		usage(_("missing --ref-format=<format>"));
+		die(_("'git refs migrate' needs '--ref-format=<format>'"));
 
 	format = ref_storage_format_by_name(format_str);
 	if (format == REF_STORAGE_FORMAT_UNKNOWN) {
diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
index f7c0783d30..e063a98b11 100755
--- a/t/t1460-refs-migrate.sh
+++ b/t/t1460-refs-migrate.sh
@@ -31,20 +31,14 @@ test_expect_success "superfluous arguments" '
 	test_when_finished "rm -rf repo" &&
 	git init repo &&
 	test_must_fail git -C repo refs migrate foo 2>err &&
-	cat >expect <<-EOF &&
-	usage: too many arguments
-	EOF
-	test_cmp expect err
+	test_grep "takes no arguments" err
 '
 
 test_expect_success "missing ref storage format" '
 	test_when_finished "rm -rf repo" &&
 	git init repo &&
 	test_must_fail git -C repo refs migrate 2>err &&
-	cat >expect <<-EOF &&
-	usage: missing --ref-format=<format>
-	EOF
-	test_cmp expect err
+	test_grep "needs ${SQ}--ref-format=<format>${SQ}" err
 '
 
 test_expect_success "unknown ref storage format" '

Interdiff against v1:
  diff --git a/builtin/refs.c b/builtin/refs.c
  index a2aac38ceb..a51602f84b 100644
  --- a/builtin/refs.c
  +++ b/builtin/refs.c
  @@ -30,11 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix)
   
   	argc = parse_options(argc, argv, prefix, options, migrate_usage, 0);
   	if (argc)
  -		usage_msg_optf(_("unknown argument: '%s'"),
  -			       migrate_usage, options,
  -			       argv[0]);
  +		die(_("'git refs migrate' takes no arguments"));
   	if (!format_str)
  -		usage(_("missing --ref-format=<format>"));
  +		die(_("'git refs migrate' needs '--ref-format=<format>'"));
   
   	format = ref_storage_format_by_name(format_str);
   	if (format == REF_STORAGE_FORMAT_UNKNOWN) {
  diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
  index b32e740001..e063a98b11 100755
  --- a/t/t1460-refs-migrate.sh
  +++ b/t/t1460-refs-migrate.sh
  @@ -31,21 +31,14 @@ test_expect_success "superfluous arguments" '
   	test_when_finished "rm -rf repo" &&
   	git init repo &&
   	test_must_fail git -C repo refs migrate foo 2>err &&
  -	{
  -		printf "fatal: unknown argument: ${SQ}foo${SQ}\n\n" &&
  -		( git -C repo refs migrate -h || : )
  -	} >expect &&
  -	test_cmp expect err
  +	test_grep "takes no arguments" err
   '
   
   test_expect_success "missing ref storage format" '
   	test_when_finished "rm -rf repo" &&
   	git init repo &&
   	test_must_fail git -C repo refs migrate 2>err &&
  -	cat >expect <<-EOF &&
  -	usage: missing --ref-format=<format>
  -	EOF
  -	test_cmp expect err
  +	test_grep "needs ${SQ}--ref-format=<format>${SQ}" err
   '
   
   test_expect_success "unknown ref storage format" '
-- 
2.46.0-235-g968ce1ce0e


  parent reply	other threads:[~2024-08-06 17:47 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-06  0:35 [PATCH v1 0/4] make "too many arguments" a bit more useful Junio C Hamano
2024-08-06  0:35 ` [PATCH v1 1/4] refs: avoid "too many arguments" Junio C Hamano
2024-08-06  6:13   ` Patrick Steinhardt
2024-08-06 16:48     ` Junio C Hamano
2024-08-06 17:11       ` [RFC] usage_msg_opt() and _optf() must die Junio C Hamano
2024-08-06 17:38         ` Eric Sunshine
2024-08-06 20:21           ` Junio C Hamano
2024-08-07  5:01             ` Patrick Steinhardt
2024-08-06 19:09         ` Justin Tobler
2024-08-06 19:24         ` Martin Ågren
2024-08-06 17:47   ` Junio C Hamano [this message]
2024-08-06  0:35 ` [PATCH v1 2/4] cat-file: avoid "too many arguments" Junio C Hamano
2024-08-06  0:35 ` [PATCH v1 3/4] notes: " Junio C Hamano
2024-08-06  0:35 ` [PATCH v1 4/4] miscellaneous: " Junio C Hamano
2024-08-06  2:31   ` Eric Sunshine
2024-08-06 16:50     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqed71tufv.fsf_-_@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).