Re: [PATCH v2 3/4] branch: deprecate "-l" option

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 22, 2018, 22:34 UTC
Message-ID: <20180622223428.GA23440@flurp.local>
In-Reply-To: <20180622092414.GC13573@sigill.intra.peff.net>

On Fri, Jun 22, 2018 at 05:24:14AM -0400, Jeff King wrote:

Show 25 quoted lines

> Let's deprecate "-l" in hopes of eventually re-purposing it
> to "--list".
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> diff --git a/builtin/branch.c b/builtin/branch.c
> @@ -36,6 +36,7 @@ static const char * const builtin_branch_usage[] = {
> +static int used_deprecated_reflog_option;
> @@ -573,6 +574,14 @@ static int edit_branch_description(const char *branch_nam> +static int deprecated_reflog_option_cb(const struct option *opt,
> +				       const char *arg, int unset)
> +{
> +	used_deprecated_reflog_option = 1;
> +	*(int *)opt->value = !unset;
> +	return 0;
> +}
> @@ -615,7 +624,13 @@ int cmd_branch(int argc, const char **argv, const char *p> +		OPT_BOOL(0, "create-reflog", &reflog, N_("create the branch's reflog")),
> +		{
> +			OPTION_CALLBACK, 'l', NULL, &reflog, NULL,
> +			N_("deprecated synonym for --create-reflog"),
> +			PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
> +			deprecated_reflog_option_cb
> @@ -688,6 +703,11 @@ int cmd_branch(int argc, const char **argv, const char *p> +	if (used_deprecated_reflog_option && !list) {
> +		warning("the '-l' alias for '--create-reflog' is deprecated;");
> +		warning("it will be removed in a future version of Git");
> +	}

I wonder if it would be better and cleaner to limit the visibility of this change to cmd_branch(), rather than spreading it across a global variable, a callback function, and cmd_branch(). Perhaps, like this:

--- >8 ---

diff --git a/builtin/branch.c b/builtin/branch.c
index 5217ba3bde..893e5f481a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -584,6 +584,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	int icase = 0;
 	static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 	struct ref_format format = REF_FORMAT_INIT;
+	int deprecated_reflog_option = 0;
 
 	struct option options[] = {
 		OPT_GROUP(N_("Generic options")),
@@ -615,7 +616,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('c', "copy", &copy, N_("copy a branch and its reflog"), 1),
 		OPT_BIT('C', NULL, &copy, N_("copy a branch, even if target exists"), 2),
 		OPT_BOOL(0, "list", &list, N_("list branch names")),
-		OPT_BOOL('l', "create-reflog", &reflog, N_("create the branch's reflog")),
+		OPT_BOOL(0, "create-reflog", &reflog, N_("create the branch's reflog")),
+		OPT_HIDDEN_BOOL('l', NULL, &deprecated_reflog_option,
+				N_("deprecated synonym for --create-reflog")),
 		OPT_BOOL(0, "edit-description", &edit_description,
 			 N_("edit the description for the branch")),
 		OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
@@ -688,6 +691,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (list)
 		setup_auto_pager("branch", 1);
 
+	if (deprecated_reflog_option && !list) {
+		reflog = 1;
+		warning("the '-l' alias for '--create-reflog' is deprecated;");
+		warning("it will be removed in a future version of Git");
+	}
+
 	if (delete) {
 		if (!argc)
 			die(_("branch name required"));
--- >8 ---