All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Kaartic Sivaraam" <kaartic.sivaraam@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH v2 3/4] branch: deprecate "-l" option
Date: Fri, 22 Jun 2018 18:34:28 -0400	[thread overview]
Message-ID: <20180622223428.GA23440@flurp.local> (raw)
In-Reply-To: <20180622092414.GC13573@sigill.intra.peff.net>

On Fri, Jun 22, 2018 at 05:24:14AM -0400, Jeff King wrote:
> 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 ---

  reply	other threads:[~2018-06-22 22:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-22  9:23 [PATCH v2 0/4] branch -l deprecation revisited Jeff King
2018-06-22  9:23 ` [PATCH v2 1/4] t3200: unset core.logallrefupdates when testing reflog creation Jeff King
2018-06-22  9:23 ` [PATCH v2 2/4] t: switch "branch -l" to "branch --create-reflog" Jeff King
2018-06-22  9:24 ` [PATCH v2 3/4] branch: deprecate "-l" option Jeff King
2018-06-22 22:34   ` Eric Sunshine [this message]
2018-06-22 22:43     ` Jeff King
2018-06-22  9:24 ` [PATCH v2 4/4] branch: make "-l" a synonym for "--list" Jeff King
2018-08-30  8:58   ` Ævar Arnfjörð Bjarmason
2018-08-30 18:48     ` Junio C Hamano
2018-08-30 19:53       ` Ævar Arnfjörð Bjarmason
2018-08-30 20:04         ` Jeff King
2018-08-30 20:29           ` Junio C Hamano
2018-08-30 20:50             ` Jeff King
2018-08-30 21:07               ` Ævar Arnfjörð Bjarmason

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=20180622223428.GA23440@flurp.local \
    --to=sunshine@sunshineco.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kaartic.sivaraam@gmail.com \
    --cc=peff@peff.net \
    /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.