git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Git List <git@vger.kernel.org>
Cc: Alban Gruin <alban.gruin@gmail.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re* [PATCH v1] git-clone.txt: add the --recursive option
Date: Tue, 14 Sep 2021 13:21:53 -0700	[thread overview]
Message-ID: <xmqq1r5q29i6.fsf_-_@gitster.g> (raw)
In-Reply-To: <xmqqtuin1019.fsf@gitster.g> (Junio C. Hamano's message of "Tue, 14 Sep 2021 11:31:46 -0700")

Subject: parse-options: allow hidden aliases

When OPT_ALIAS() was introduced to mark one option is a mere synonym
for another option, we forgot to add support for a use case where an
option is made an alias with an intention to deprecat and eventually
remove it in the future, which usually means "git cmd -h" hides the
deprecated alias while "git cmd --help-all" shows it.

The "--recursive" option of "git clone" and the "--mailmap" option
of "git log" use the OPT_ALIAS mechansim to mark themselves as an
alias of another.  The former has been deprecated but "git clone -h"
still shows it.

Introduce OPT_HIDDEN_ALIAS() that hides the entry from "git cmd -h"
output and use it for "git clone --recursive".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * So here is to add support for hidden aliases and application of
   it on "git clone".  Perhaps everything except for the part that
   applies to "builtin/clone.c" should become [1/2] of a two-patch
   series, while the change to "builtin/clone.c", plus documentation
   updates to mention "--recursive" as a deprecated synonym, should
   become [2/2].

   But I do not have time to go that last mile right now ;-)

>>>  * adding the PARSE_OPT_HIDDEN bit to the OPT_ALIAS() element for
>>>    the deprecated "recurse" option.
>>
>> I was going to suggest this as a possible way forward to address
>> Alban's most recent response to my response. The lack of
>> PARSE_OPT_HIDDEN on OPT_ALIAS() almost seems like an oversight.
>
> You may have an alias with no intention to deprecate either, so it
> would make it cumbersome if OPT_ALIAS() always meant HIDDEN, just
> like it currently is cumbersome for an alias that is deprecated.

 builtin/clone.c               |  2 +-
 parse-options.c               |  4 +++-
 parse-options.h               |  3 +++
 t/helper/test-parse-options.c |  1 +
 t/t0040-parse-options.sh      | 13 ++++++++++++-
 5 files changed, 20 insertions(+), 3 deletions(-)

diff --git c/builtin/clone.c w/builtin/clone.c
index 66fe66679c..6fd4b41eb3 100644
--- c/builtin/clone.c
+++ w/builtin/clone.c
@@ -110,7 +110,7 @@ static struct option builtin_clone_options[] = {
 	{ OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
 	  N_("pathspec"), N_("initialize submodules in the clone"),
 	  PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },
-	OPT_ALIAS(0, "recursive", "recurse-submodules"),
+	OPT_HIDDEN_ALIAS(0, "recursive", "recurse-submodules"),
 	OPT_INTEGER('j', "jobs", &max_jobs,
 		    N_("number of submodules cloned in parallel")),
 	OPT_STRING(0, "template", &option_template, N_("template-directory"),
diff --git c/parse-options.c w/parse-options.c
index 2abff136a1..46af4eacdf 100644
--- c/parse-options.c
+++ w/parse-options.c
@@ -653,6 +653,7 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
 		int short_name;
 		const char *long_name;
 		const char *source;
+		int flags;
 		struct strbuf help = STRBUF_INIT;
 		int j;
 
@@ -662,6 +663,7 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
 		short_name = newopt[i].short_name;
 		long_name = newopt[i].long_name;
 		source = newopt[i].value;
+		flags = newopt[i].flags;
 
 		if (!long_name)
 			BUG("An alias must have long option name");
@@ -680,7 +682,7 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
 			newopt[i].short_name = short_name;
 			newopt[i].long_name = long_name;
 			newopt[i].help = strbuf_detach(&help, NULL);
-			newopt[i].flags |= PARSE_OPT_FROM_ALIAS;
+			newopt[i].flags |= PARSE_OPT_FROM_ALIAS | flags;
 			break;
 		}
 
diff --git c/parse-options.h w/parse-options.h
index a845a9d952..8ba72c7916 100644
--- c/parse-options.h
+++ w/parse-options.h
@@ -201,6 +201,9 @@ struct option {
 #define OPT_ALIAS(s, l, source_long_name) \
 	{ OPTION_ALIAS, (s), (l), (source_long_name) }
 
+#define OPT_HIDDEN_ALIAS(s, l, source_long_name)		\
+	{ OPTION_ALIAS, (s), (l), (source_long_name), NULL, NULL, PARSE_OPT_HIDDEN }
+
 /*
  * parse_options() will filter out the processed options and leave the
  * non-option arguments in argv[]. argv0 is assumed program name and
diff --git c/t/helper/test-parse-options.c w/t/helper/test-parse-options.c
index 2051ce57db..86c3eb1a29 100644
--- c/t/helper/test-parse-options.c
+++ w/t/helper/test-parse-options.c
@@ -154,6 +154,7 @@ int cmd__parse_options(int argc, const char **argv)
 		OPT_GROUP("Alias"),
 		OPT_STRING('A', "alias-source", &string, "string", "get a string"),
 		OPT_ALIAS('Z', "alias-target", "alias-source"),
+		OPT_HIDDEN_ALIAS(0, "hidden-alias", "alias-source"),
 		OPT_END(),
 	};
 	int i;
diff --git c/t/t0040-parse-options.sh w/t/t0040-parse-options.sh
index ad4746d899..4d31367b07 100755
--- c/t/t0040-parse-options.sh
+++ w/t/t0040-parse-options.sh
@@ -7,7 +7,7 @@ test_description='our own option parser'
 
 . ./test-lib.sh
 
-cat >expect <<\EOF
+cat >help-all.in <<\EOF
 usage: test-tool parse-options <options>
 
     A helper function for the parse-options API.
@@ -34,6 +34,7 @@ String options
     --string2 <str>       get another string
     --st <st>             get another string (pervert ordering)
     -o <str>              get another string
+#    --obsolete            no-op (backward compatibility)
     --list <str>          add str to list
 
 Magic arguments
@@ -55,10 +56,20 @@ Alias
                           get a string
     -Z, --alias-target <string>
                           alias of --alias-source
+#    --hidden-alias <string>
+#                          alias of --alias-source
 
 EOF
 
+test_expect_success 'hidden alias in test help' '
+	sed -e "s/^#//" help-all.in >expect &&
+	test_must_fail test-tool parse-options --help-all >output 2>output.err && 
+	test_must_be_empty output.err &&
+	test_cmp expect output
+'
+
 test_expect_success 'test help' '
+	sed -e "/^#/d" help-all.in >expect &&
 	test_must_fail test-tool parse-options -h >output 2>output.err &&
 	test_must_be_empty output.err &&
 	test_cmp expect output

  reply	other threads:[~2021-09-14 20:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-13 18:59 [PATCH v1] git-clone.txt: add the --recursive option Alban Gruin
2021-09-13 19:26 ` Eric Sunshine
2021-09-13 20:42   ` Alban Gruin
2021-09-13 21:57     ` Eric Sunshine
2021-09-14 10:27       ` Alban Gruin
2021-09-14 17:46         ` Junio C Hamano
2021-09-14 17:53           ` Eric Sunshine
2021-09-14 18:31             ` Junio C Hamano
2021-09-14 20:21               ` Junio C Hamano [this message]
2021-09-13 21:43   ` 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqq1r5q29i6.fsf_-_@gitster.g \
    --to=gitster@pobox.com \
    --cc=alban.gruin@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=sunshine@sunshineco.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).