Re* [PATCH v1] git-clone.txt: add the --recursive option

Commit Message

Junio C Hamano Sept. 14, 2021, 8:21 p.m. UTC

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 <[e-mail]>
---

 * 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(-)

Patch

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