All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Carlo Arenas" <carenas@gmail.com>,
	"Paul-Sebastian Ungureanu" <ungureanupaulsebastian@gmail.com>,
	"Alexandr Miloslavskiy" <alexandr.miloslavskiy@syntevo.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH] stash: print the correct usage on "git stash [push] --invalid-option"
Date: Wed,  1 Sep 2021 13:19:54 +0200	[thread overview]
Message-ID: <patch-1.1-47c582f1218-20210901T111930Z-avarab@gmail.com> (raw)

Change the usage message emitted by "git stash --invalid-option" to
emit usage information for "git stash" in general, and not just for
the "push" command.

That we emitted the usage for just "push" in the case of the
subcommand not being explicitly specified was an unintentional
side-effect of how it was implemented. When it was converted to C in
d553f538b8a (stash: convert push to builtin, 2019-02-25) the pattern
of having per-subcommand usage information was rightly continued. The
Git-stash.sh shellscript did not have that, and always printed the
equivalent of "git_stash_usage".

But in doing so the case of push being implicit and explicit was
conflated. A variable was added to track this in 8c3713cede7 (stash:
eliminate crude option parsing, 2020-02-17), but it did not update the
usage output accordingly. Let's do that.

This still leaves e.g. "git stash push -h" emitting the
"git_stash_usage" output, instead of "git_stash_push_usage". That
should be fixed, but is a much deeper misbehavior in parse_options()
not being aware of subcommands at all. I.e. in how
PARSE_OPT_KEEP_UNKNOWN and PARSE_OPT_NO_INTERNAL_HELP combine in
commands such as "git stash".

Perhaps PARSE_OPT_KEEP_UNKNOWN should imply
PARSE_OPT_NO_INTERNAL_HELP, or better yet parse_options() should be
extended to fully handle these subcommand cases that we handle
manually in "git stash", "git commit-graph", "git multi-pack-index"
etc. All of those musings would be a much bigger change than this
isolated fix though, so let's leave that for some other time.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/stash.c  |  1 +
 t/t3903-stash.sh | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/builtin/stash.c b/builtin/stash.c
index 8f42360ca91..cf62c3b3cd8 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1608,6 +1608,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,
 	if (argc) {
 		force_assume = !strcmp(argv[0], "-p");
 		argc = parse_options(argc, argv, prefix, options,
+				     push_assumed ? git_stash_usage :
 				     git_stash_push_usage,
 				     PARSE_OPT_KEEP_DASHDASH);
 	}
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 873aa56e359..ba9582bdee5 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -10,6 +10,17 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+test_expect_success 'usage' '
+	test_expect_code 129 git stash -h >out &&
+	grep "or: git stash" out &&
+
+	test_expect_code 129 git stash --invalid-option >out 2>err &&
+	grep "or: git stash" err &&
+
+	test_expect_code 129 git stash push --invalid-option >out 2>err &&
+	! grep "or: git stash" err
+'
+
 diff_cmp () {
 	for i in "$1" "$2"
 	do
-- 
2.33.0.807.gf14ecf9c2e9


             reply	other threads:[~2021-09-01 11:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01 11:19 Ævar Arnfjörð Bjarmason [this message]
2021-09-21 13:46 ` [PATCH v2] stash: print the correct usage on "git stash [push] --invalid-option" Ævar Arnfjörð Bjarmason
2021-12-16 12:54   ` [PATCH v3] stash: don't show "git stash push" usage on bad "git stash" usage Ævar Arnfjörð Bjarmason
2021-12-16 22:03     ` Junio C Hamano
2021-12-16 22:05     ` 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=patch-1.1-47c582f1218-20210901T111930Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=alexandr.miloslavskiy@syntevo.com \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=ungureanupaulsebastian@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.