[PATCH v2 0/4] branch -l deprecation revisited

[PATCH v2 0/4] branch -l deprecation revisited

[Jeff King]

This series replaces the contents of jk/branch-l-0-deprecation,
jk/branch-l-1-removal, and jk/branch-l-2-reincarnation.

It implements the idea discussed in the subthread in:

  https://public-inbox.org/git/xmqqzi0hety4.fsf@gitster-ct.c.googlers.com/

Namely that "branch -l" would warn about deprecation only when we're not
in list mode, and then we'll eventually jump straight to repurposing
"-l" as "--list".

This gets us to the end result faster, without the annoying "-l doesn't
mean anything" step in between, and without useless warnings on "git
branch -l". It also sidesteps any pager issues since list mode will not
print the warning.

The first 3 patches would become jk/branch-l-0-deprecate, and then the
final one would become jk/branch-l-1-repurpose or similar. No third step
required.

  [1/4]: t3200: unset core.logallrefupdates when testing reflog creation
  [2/4]: t: switch "branch -l" to "branch --create-reflog"
  [3/4]: branch: deprecate "-l" option
  [4/4]: branch: make "-l" a synonym for "--list"

 Documentation/git-branch.txt |  2 +-
 builtin/branch.c             |  4 ++--
 t/t1410-reflog.sh            |  4 ++--
 t/t3200-branch.sh            | 34 +++++++++++++++++-----------------
 4 files changed, 22 insertions(+), 22 deletions(-)

-Peff

[PATCH v2 1/4] t3200: unset core.logallrefupdates when testing reflog creation

[Jeff King]

This test checks that the "-l" option creates a reflog. But
in fact we'd create one even without it, since the default
in a non-bare repository is to do so. Let's unset the config
so we can be sure our "-l" option is kicking in.

Note that we can't do this with test_config, since that
would leave the variable unset after our test finishes,
confusing downstream tests (the helper is not not smart
enough to restore the previous value, and just always runs
test_unconfig).

Signed-off-by: Jeff King <peff@peff.net>
---
Same as before.

 t/t3200-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 08467982f6..ec56176093 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -51,7 +51,7 @@ $ZERO_OID $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	bran
 EOF
 test_expect_success 'git branch -l d/e/f should create a branch and a log' '
 	GIT_COMMITTER_DATE="2005-05-26 23:30" \
-	git branch -l d/e/f &&
+	git -c core.logallrefupdates=false branch -l d/e/f &&
 	test_path_is_file .git/refs/heads/d/e/f &&
 	test_path_is_file .git/logs/refs/heads/d/e/f &&
 	test_cmp expect .git/logs/refs/heads/d/e/f
-- 
2.18.0.539.gcf23d6d4f1

[PATCH v2 2/4] t: switch "branch -l" to "branch --create-reflog"

[Jeff King]

In preparation for deprecating "-l", let's make sure we're
using the recommended option ourselves.

This patch just mechanically converts "branch -l" to "branch
--create-reflog".  Note that with the exception of the
actual "--create-reflog" test, we could actually remove "-l"
entirely from most of these callers. That's because these
days core.logallrefupdates defaults to true in a non-bare
repository.

I've left them in place, though, since they serve to
document the expectation of the test, even if they are
technically noops.

Signed-off-by: Jeff King <peff@peff.net>
---
Same as before.

 t/t1410-reflog.sh |  4 ++--
 t/t3200-branch.sh | 34 +++++++++++++++++-----------------
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 553e26d9ce..8293131001 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -339,8 +339,8 @@ test_expect_failure 'reflog with non-commit entries displays all entries' '
 '
 
 test_expect_success 'reflog expire operates on symref not referrent' '
-	git branch -l the_symref &&
-	git branch -l referrent &&
+	git branch --create-reflog the_symref &&
+	git branch --create-reflog referrent &&
 	git update-ref referrent HEAD &&
 	git symbolic-ref refs/heads/the_symref refs/heads/referrent &&
 	test_when_finished "rm -f .git/refs/heads/referrent.lock" &&
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index ec56176093..dbca665da4 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -49,9 +49,9 @@ test_expect_success 'git branch HEAD should fail' '
 cat >expect <<EOF
 $ZERO_OID $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master
 EOF
-test_expect_success 'git branch -l d/e/f should create a branch and a log' '
+test_expect_success 'git branch --create-reflog d/e/f should create a branch and a log' '
 	GIT_COMMITTER_DATE="2005-05-26 23:30" \
-	git -c core.logallrefupdates=false branch -l d/e/f &&
+	git -c core.logallrefupdates=false branch --create-reflog d/e/f &&
 	test_path_is_file .git/refs/heads/d/e/f &&
 	test_path_is_file .git/logs/refs/heads/d/e/f &&
 	test_cmp expect .git/logs/refs/heads/d/e/f
@@ -82,7 +82,7 @@ test_expect_success 'git branch -m dumps usage' '
 
 test_expect_success 'git branch -m m broken_symref should work' '
 	test_when_finished "git branch -D broken_symref" &&
-	git branch -l m &&
+	git branch --create-reflog m &&
 	git symbolic-ref refs/heads/broken_symref refs/heads/i_am_broken &&
 	git branch -m m broken_symref &&
 	git reflog exists refs/heads/broken_symref &&
@@ -90,13 +90,13 @@ test_expect_success 'git branch -m m broken_symref should work' '
 '
 
 test_expect_success 'git branch -m m m/m should work' '
-	git branch -l m &&
+	git branch --create-reflog m &&
 	git branch -m m m/m &&
 	git reflog exists refs/heads/m/m
 '
 
 test_expect_success 'git branch -m n/n n should work' '
-	git branch -l n/n &&
+	git branch --create-reflog n/n &&
 	git branch -m n/n n &&
 	git reflog exists refs/heads/n
 '
@@ -378,9 +378,9 @@ mv .git/config-saved .git/config
 git config branch.s/s.dummy Hello
 
 test_expect_success 'git branch -m s/s s should work when s/t is deleted' '
-	git branch -l s/s &&
+	git branch --create-reflog s/s &&
 	git reflog exists refs/heads/s/s &&
-	git branch -l s/t &&
+	git branch --create-reflog s/t &&
 	git reflog exists refs/heads/s/t &&
 	git branch -d s/t &&
 	git branch -m s/s s &&
@@ -444,7 +444,7 @@ test_expect_success 'git branch --copy dumps usage' '
 '
 
 test_expect_success 'git branch -c d e should work' '
-	git branch -l d &&
+	git branch --create-reflog d &&
 	git reflog exists refs/heads/d &&
 	git config branch.d.dummy Hello &&
 	git branch -c d e &&
@@ -459,7 +459,7 @@ test_expect_success 'git branch -c d e should work' '
 '
 
 test_expect_success 'git branch --copy is a synonym for -c' '
-	git branch -l copy &&
+	git branch --create-reflog copy &&
 	git reflog exists refs/heads/copy &&
 	git config branch.copy.dummy Hello &&
 	git branch --copy copy copy-to &&
@@ -486,7 +486,7 @@ test_expect_success 'git branch -c ee ef should copy ee to create branch ef' '
 '
 
 test_expect_success 'git branch -c f/f g/g should work' '
-	git branch -l f/f &&
+	git branch --create-reflog f/f &&
 	git reflog exists refs/heads/f/f &&
 	git config branch.f/f.dummy Hello &&
 	git branch -c f/f g/g &&
@@ -497,7 +497,7 @@ test_expect_success 'git branch -c f/f g/g should work' '
 '
 
 test_expect_success 'git branch -c m2 m2 should work' '
-	git branch -l m2 &&
+	git branch --create-reflog m2 &&
 	git reflog exists refs/heads/m2 &&
 	git config branch.m2.dummy Hello &&
 	git branch -c m2 m2 &&
@@ -506,18 +506,18 @@ test_expect_success 'git branch -c m2 m2 should work' '
 '
 
 test_expect_success 'git branch -c zz zz/zz should fail' '
-	git branch -l zz &&
+	git branch --create-reflog zz &&
 	git reflog exists refs/heads/zz &&
 	test_must_fail git branch -c zz zz/zz
 '
 
 test_expect_success 'git branch -c b/b b should fail' '
-	git branch -l b/b &&
+	git branch --create-reflog b/b &&
 	test_must_fail git branch -c b/b b
 '
 
 test_expect_success 'git branch -C o/q o/p should work when o/p exists' '
-	git branch -l o/q &&
+	git branch --create-reflog o/q &&
 	git reflog exists refs/heads/o/q &&
 	git reflog exists refs/heads/o/p &&
 	git branch -C o/q o/p
@@ -570,10 +570,10 @@ test_expect_success 'git branch -C master5 master5 should work when master is ch
 '
 
 test_expect_success 'git branch -C ab cd should overwrite existing config for cd' '
-	git branch -l cd &&
+	git branch --create-reflog cd &&
 	git reflog exists refs/heads/cd &&
 	git config branch.cd.dummy CD &&
-	git branch -l ab &&
+	git branch --create-reflog ab &&
 	git reflog exists refs/heads/ab &&
 	git config branch.ab.dummy AB &&
 	git branch -C ab cd &&
@@ -685,7 +685,7 @@ test_expect_success 'renaming a symref is not allowed' '
 '
 
 test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
-	git branch -l u &&
+	git branch --create-reflog u &&
 	mv .git/logs/refs/heads/u real-u &&
 	ln -s real-u .git/logs/refs/heads/u &&
 	test_must_fail git branch -m u v
-- 
2.18.0.539.gcf23d6d4f1

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

[Jeff King]

The "-l" option is short for "--create-reflog". This has
caused much confusion over the years. Most people expect it
to work as "--list", because that would match the other
"mode" options like -d/--delete and -m/--move, as well as
the similar -l/--list option of git-tag.

Adding to the confusion, using "-l" _appears_ to work as
"--list" in some cases:

  $ git branch -l
  * master

because the branch command defaults to listing (so even
trying to specify --list in the command above is redundant).
But that may bite the user later when they add a pattern,
like:

  $ git branch -l foo

which does not return an empty list, but in fact creates a
new branch (with a reflog, naturally) called "foo".

It's also probably quite uncommon for people to actually use
"-l" to create a reflog. Since 0bee591869 (Enable reflogs by
default in any repository with a working directory.,
2006-12-14), this is the default in non-bare repositories.
So it's rather unfortunate that the feature squats on the
short-and-sweet "-l" (which was only added in 3a4b3f269c
(Create/delete branch ref logs., 2006-05-19), meaning there
were only 7 months where it was actually useful).

Let's deprecate "-l" in hopes of eventually re-purposing it
to "--list".

Note that we issue the warning only when we're not in list
mode. This means that people for whom it works as a happy
accident, namely:

  $ git branch -l
  master

won't see the warning at all. And when we eventually switch
to it meaning "--list", that will just continue to work.

We do the issue the warning for these important cases:

  - when we are actually creating a branch, in case the user
    really did mean it as "--create-reflog"

  - when we are in some _other_ mode, like deletion. There
    the "-l" is a noop for now, but it will eventually
    conflict with any other mode request, and the user
    should be told that this is changing.

Signed-off-by: Jeff King <peff@peff.net>
---
This one has the interesting changes in this re-roll.

 Documentation/git-branch.txt |  3 ++-
 builtin/branch.c             | 22 +++++++++++++++++++++-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 02eccbb931..1072ca0eb6 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -91,7 +91,6 @@ OPTIONS
 -D::
 	Shortcut for `--delete --force`.
 
--l::
 --create-reflog::
 	Create the branch's reflog.  This activates recording of
 	all changes made to the branch ref, enabling use of date
@@ -101,6 +100,8 @@ OPTIONS
 	The negated form `--no-create-reflog` only overrides an earlier
 	`--create-reflog`, but currently does not negate the setting of
 	`core.logAllRefUpdates`.
++
+The `-l` option is a deprecated synonym for `--create-reflog`.
 
 -f::
 --force::
diff --git a/builtin/branch.c b/builtin/branch.c
index 5217ba3bde..ed4c093747 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -36,6 +36,7 @@ static const char * const builtin_branch_usage[] = {
 
 static const char *head;
 static struct object_id head_oid;
+static int used_deprecated_reflog_option;
 
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
@@ -573,6 +574,14 @@ static int edit_branch_description(const char *branch_name)
 	return 0;
 }
 
+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;
+}
+
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
@@ -615,7 +624,13 @@ 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")),
+		{
+			OPTION_CALLBACK, 'l', NULL, &reflog, NULL,
+			N_("deprecated synonym for --create-reflog"),
+			PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
+			deprecated_reflog_option_cb
+		},
 		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 +703,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (list)
 		setup_auto_pager("branch", 1);
 
+	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");
+	}
+
 	if (delete) {
 		if (!argc)
 			die(_("branch name required"));
-- 
2.18.0.539.gcf23d6d4f1

[PATCH v2 4/4] branch: make "-l" a synonym for "--list"

[Jeff King]

The other "mode" options of git-branch have short-option
aliases that are easy to type (e.g., "-d" and "-m"). Let's
give "--list" the same treatment.

This also makes it consistent with the similar "git tag -l"
option.

We didn't do this originally because "--create-reflog" was
squatting on the "-l" option. Now that we've deprecated that
use for long enough, we can make the switch.

Signed-off-by: Jeff King <peff@peff.net>
---
This one is mostly a squash of the remove/reincarnate steps from the
previous iteration. I also noticed that my original forgot to update the
documentation, which this patch does.

 Documentation/git-branch.txt |  3 +--
 builtin/branch.c             | 22 +---------------------
 2 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 1072ca0eb6..fc88e984e1 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -100,8 +100,6 @@ OPTIONS
 	The negated form `--no-create-reflog` only overrides an earlier
 	`--create-reflog`, but currently does not negate the setting of
 	`core.logAllRefUpdates`.
-+
-The `-l` option is a deprecated synonym for `--create-reflog`.
 
 -f::
 --force::
@@ -156,6 +154,7 @@ This option is only applicable in non-verbose mode.
 --all::
 	List both remote-tracking branches and local branches.
 
+-l::
 --list::
 	List branches.  With optional `<pattern>...`, e.g. `git
 	branch --list 'maint-*'`, list only the branches that match
diff --git a/builtin/branch.c b/builtin/branch.c
index ed4c093747..c1662e3db3 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -36,7 +36,6 @@ static const char * const builtin_branch_usage[] = {
 
 static const char *head;
 static struct object_id head_oid;
-static int used_deprecated_reflog_option;
 
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
@@ -574,14 +573,6 @@ static int edit_branch_description(const char *branch_name)
 	return 0;
 }
 
-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;
-}
-
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
@@ -623,14 +614,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('M', NULL, &rename, N_("move/rename a branch, even if target exists"), 2),
 		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', "list", &list, N_("list branch names")),
 		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
-		},
 		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),
@@ -703,11 +688,6 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (list)
 		setup_auto_pager("branch", 1);
 
-	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");
-	}
-
 	if (delete) {
 		if (!argc)
 			die(_("branch name required"));
-- 
2.18.0.539.gcf23d6d4f1

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

[Eric Sunshine]

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 ---

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

[Jeff King]

On Fri, Jun 22, 2018 at 06:34:28PM -0400, Eric Sunshine wrote:

> 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:

I'd prefer that, too, but...

> @@ -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")),

Now that "-l" processing is delayed, it interacts in a funny way with
--create-reflog. For instance:

  git branch -l --no-create-reflog

currently cancels itself out, but after your patch would enable reflogs.

This is a pretty niche corner case, but I think it's important not to
change any behavior during the deprecation period. You'd have to do
something more like:

  reflog = -1;

  ... parse options ...

  if (deprecated_reflog_option && !list)
    warning(...);
  if (reflog < 0 && deprecated_reflog_option)
    reflog = 1;

I think that probably works in all cases, but I actually think the
existing callback/global is less invasive.

-Peff

Re: [PATCH v2 4/4] branch: make "-l" a synonym for "--list"

[Ævar Arnfjörð Bjarmason]

On Fri, Jun 22 2018, Jeff King wrote:

> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 1072ca0eb6..fc88e984e1 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -100,8 +100,6 @@ OPTIONS
>  	The negated form `--no-create-reflog` only overrides an earlier
>  	`--create-reflog`, but currently does not negate the setting of
>  	`core.logAllRefUpdates`.
> -+
> -The `-l` option is a deprecated synonym for `--create-reflog`.
>
>  -f::
>  --force::
> @@ -156,6 +154,7 @@ This option is only applicable in non-verbose mode.
>  --all::
>  	List both remote-tracking branches and local branches.
>
> +-l::
>  --list::
>  	List branches.  With optional `<pattern>...`, e.g. `git
>  	branch --list 'maint-*'`, list only the branches that match

I think it's better to have something like this on top:

    diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
    index 5552dfcec3..a03cb1ebc9 100644
    --- a/Documentation/git-branch.txt
    +++ b/Documentation/git-branch.txt
    @@ -163,6 +163,11 @@ This option is only applicable in non-verbose mode.
     This should not be confused with `git branch -l <branchname>`,
     which creates a branch named `<branchname>` with a reflog.
     See `--create-reflog` above for details.
    ++
    +
    +Until Git version 2.20 `-l` was the short form of
    +`--create-reflog`. As of version 2.19 using it would warn about a
    +future deprecation.

     -v::
     -vv::

We're about to release 2.19 with the deprecation (but it still means
--create-reflog), this patch is sitting in next.

Similarly to your 2/4 we'll have some scripts in the wild using -l,
let's at least give them a headsup that this changed in the docs, as
well as anyone on >=2.20 (or whenever we plan to merge this down from
next) a warning that if they're writing some script they can't rely on
`-l` for older clients.

Re: [PATCH v2 4/4] branch: make "-l" a synonym for "--list"

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> +-l::
>>  --list::
>>  	List branches.  With optional `<pattern>...`, e.g. `git
>>  	branch --list 'maint-*'`, list only the branches that match
>
> I think it's better to have something like this on top:
>
>     diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
>     index 5552dfcec3..a03cb1ebc9 100644
>     --- a/Documentation/git-branch.txt
>     +++ b/Documentation/git-branch.txt
>     @@ -163,6 +163,11 @@ This option is only applicable in non-verbose mode.
>      This should not be confused with `git branch -l <branchname>`,
>      which creates a branch named `<branchname>` with a reflog.
>      See `--create-reflog` above for details.
>     ++
>     +
>     +Until Git version 2.20 `-l` was the short form of
>     +`--create-reflog`. As of version 2.19 using it would warn about a
>     +future deprecation.

Doesn't your patch show a more grave issue with the current state of
'next'?

The sentence in the pre-context in your suggested patch says that
"--list" should not be confused with "git branch -l <branchname>",
but --list and -l are now synonyms in the new world order.

Worse yet, '-l' is no longer a way to create the branch with a
reflog; in the new world, you would say "--create-reflog" if you
want to do so.  "git branch -l foo" would list branches that match
that pattern 'foo'.

In the SYNOPSIS section we still see "[-l]" listed; that also must
be replaced with "--create-reflog", or just dropped, as that is the
default.

I do not know if the documentation that is shipped in 2.20 should
talk about how the old world looked like, though.  `-l` was a short
for `--create-reflog` is worth saying, but I do not see much value
in talking about the warning given in 2.19.

Thanks.

Re: [PATCH v2 4/4] branch: make "-l" a synonym for "--list"

[Ævar Arnfjörð Bjarmason]

On Thu, Aug 30 2018, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> +-l::
>>>  --list::
>>>  	List branches.  With optional `<pattern>...`, e.g. `git
>>>  	branch --list 'maint-*'`, list only the branches that match
>>
>> I think it's better to have something like this on top:
>>
>>     diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
>>     index 5552dfcec3..a03cb1ebc9 100644
>>     --- a/Documentation/git-branch.txt
>>     +++ b/Documentation/git-branch.txt
>>     @@ -163,6 +163,11 @@ This option is only applicable in non-verbose mode.
>>      This should not be confused with `git branch -l <branchname>`,
>>      which creates a branch named `<branchname>` with a reflog.
>>      See `--create-reflog` above for details.
>>     ++
>>     +
>>     +Until Git version 2.20 `-l` was the short form of
>>     +`--create-reflog`. As of version 2.19 using it would warn about a
>>     +future deprecation.
>
> Doesn't your patch show a more grave issue with the current state of
> 'next'?
>
> The sentence in the pre-context in your suggested patch says that
> "--list" should not be confused with "git branch -l <branchname>",
> but --list and -l are now synonyms in the new world order.
>
> Worse yet, '-l' is no longer a way to create the branch with a
> reflog; in the new world, you would say "--create-reflog" if you
> want to do so.  "git branch -l foo" would list branches that match
> that pattern 'foo'.
>
> In the SYNOPSIS section we still see "[-l]" listed; that also must
> be replaced with "--create-reflog", or just dropped, as that is the
> default.

Oh yes, it seems all of the doc indeed wasn't updated!

> I do not know if the documentation that is shipped in 2.20 should
> talk about how the old world looked like, though.  `-l` was a short
> for `--create-reflog` is worth saying, but I do not see much value
> in talking about the warning given in 2.19.

I'm anticipating that there will be users in the wild with similar -l
invocations, noting this helps them, because they'll be wondering what
some script that does "git branch -l <name>" is trying to do while
reading our docs.

Both our command-line options (plumbing or otherwise) and file various
formats (e.g. I had a similar mention of version differences in my
recent skipList patches) can be expected to be used across multiple git
versions, by users who most likely are only browsing the latest version
of the docs, not comparing how the manpage looked like in multiple git
versions to see if an option meant something different, or if a format
was documented as behaving differently.

Re: [PATCH v2 4/4] branch: make "-l" a synonym for "--list"

[Jeff King]

On Thu, Aug 30, 2018 at 09:53:25PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > In the SYNOPSIS section we still see "[-l]" listed; that also must
> > be replaced with "--create-reflog", or just dropped, as that is the
> > default.
> 
> Oh yes, it seems all of the doc indeed wasn't updated!

Sorry, this is my fault. Patch is below (which would go on top of
jk/branch-l-1-repurpose).

> > I do not know if the documentation that is shipped in 2.20 should
> > talk about how the old world looked like, though.  `-l` was a short
> > for `--create-reflog` is worth saying, but I do not see much value
> > in talking about the warning given in 2.19.
> 
> I'm anticipating that there will be users in the wild with similar -l
> invocations, noting this helps them, because they'll be wondering what
> some script that does "git branch -l <name>" is trying to do while
> reading our docs.

I don't have a strong opinion either way. If we do mention it, it should
probably be short ("Until Git v2.20, the `-l` option was a synonym for
`--create-reflog").

-Peff

-- >8 --
Subject: [PATCH] doc/git-branch: remove obsolete "-l" references

The previous commit switched "-l" to meaning "--list", but a
few vestiges of its prior meaning as "--create-reflog"
remained:

  - the synopsis mentioned "-l" when creating a new branch;
    we can drop this entirely, as it has been the default
    for years

  - the --list command mentions the unfortunate "-l"
    confusion, but we've now fixed that

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/git-branch.txt | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 5552dfcec3..bf5316ffa9 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -14,7 +14,7 @@ SYNOPSIS
 	[(--merged | --no-merged) [<commit>]]
 	[--contains [<commit]] [--no-contains [<commit>]]
 	[--points-at <object>] [--format=<format>] [<pattern>...]
-'git branch' [--track | --no-track] [-l] [-f] <branchname> [<start-point>]
+'git branch' [--track | --no-track] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
 'git branch' --unset-upstream [<branchname>]
 'git branch' (-m | -M) [<oldbranch>] <newbranch>
@@ -159,10 +159,6 @@ This option is only applicable in non-verbose mode.
 	List branches.  With optional `<pattern>...`, e.g. `git
 	branch --list 'maint-*'`, list only the branches that match
 	the pattern(s).
-+
-This should not be confused with `git branch -l <branchname>`,
-which creates a branch named `<branchname>` with a reflog.
-See `--create-reflog` above for details.
 
 -v::
 -vv::
-- 
2.19.0.rc1.546.g3fcb3c0d7c

Re: [PATCH v2 4/4] branch: make "-l" a synonym for "--list"

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Aug 30, 2018 at 09:53:25PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> > In the SYNOPSIS section we still see "[-l]" listed; that also must
>> > be replaced with "--create-reflog", or just dropped, as that is the
>> > default.
>> 
>> Oh yes, it seems all of the doc indeed wasn't updated!
>
> Sorry, this is my fault. Patch is below (which would go on top of
> jk/branch-l-1-repurpose).

Heh, reviewers who did not notice share the same blame.  The patch
looks good.  Thanks for a quick update.

>> > I do not know if the documentation that is shipped in 2.20 should
>> > talk about how the old world looked like, though.  `-l` was a short
>> > for `--create-reflog` is worth saying, but I do not see much value
>> > in talking about the warning given in 2.19.
>> 
>> I'm anticipating that there will be users in the wild with similar -l
>> invocations, noting this helps them, because they'll be wondering what
>> some script that does "git branch -l <name>" is trying to do while
>> reading our docs.
>
> I don't have a strong opinion either way. If we do mention it, it should
> probably be short ("Until Git v2.20, the `-l` option was a synonym for
> `--create-reflog").

I agree that the short one would of course be good.  I am on the
fence about mentioning the warning only given in 2.19.

> -- >8 --
> Subject: [PATCH] doc/git-branch: remove obsolete "-l" references
>
> The previous commit switched "-l" to meaning "--list", but a
> few vestiges of its prior meaning as "--create-reflog"
> remained:
>
>   - the synopsis mentioned "-l" when creating a new branch;
>     we can drop this entirely, as it has been the default
>     for years
>
>   - the --list command mentions the unfortunate "-l"
>     confusion, but we've now fixed that
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  Documentation/git-branch.txt | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 5552dfcec3..bf5316ffa9 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -14,7 +14,7 @@ SYNOPSIS
>  	[(--merged | --no-merged) [<commit>]]
>  	[--contains [<commit]] [--no-contains [<commit>]]
>  	[--points-at <object>] [--format=<format>] [<pattern>...]
> -'git branch' [--track | --no-track] [-l] [-f] <branchname> [<start-point>]
> +'git branch' [--track | --no-track] [-f] <branchname> [<start-point>]
>  'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
>  'git branch' --unset-upstream [<branchname>]
>  'git branch' (-m | -M) [<oldbranch>] <newbranch>
> @@ -159,10 +159,6 @@ This option is only applicable in non-verbose mode.
>  	List branches.  With optional `<pattern>...`, e.g. `git
>  	branch --list 'maint-*'`, list only the branches that match
>  	the pattern(s).
> -+
> -This should not be confused with `git branch -l <branchname>`,
> -which creates a branch named `<branchname>` with a reflog.
> -See `--create-reflog` above for details.
>  
>  -v::
>  -vv::

Re: [PATCH v2 4/4] branch: make "-l" a synonym for "--list"

[Jeff King]

On Thu, Aug 30, 2018 at 01:29:53PM -0700, Junio C Hamano wrote:

> >> > I do not know if the documentation that is shipped in 2.20 should
> >> > talk about how the old world looked like, though.  `-l` was a short
> >> > for `--create-reflog` is worth saying, but I do not see much value
> >> > in talking about the warning given in 2.19.
> >> 
> >> I'm anticipating that there will be users in the wild with similar -l
> >> invocations, noting this helps them, because they'll be wondering what
> >> some script that does "git branch -l <name>" is trying to do while
> >> reading our docs.
> >
> > I don't have a strong opinion either way. If we do mention it, it should
> > probably be short ("Until Git v2.20, the `-l` option was a synonym for
> > `--create-reflog").
> 
> I agree that the short one would of course be good.  I am on the
> fence about mentioning the warning only given in 2.19.

Yeah, I was confused about that part of the thread. Is there something
proposed to (additionally) go into v2.19? Ævar, can you elaborate?

-Peff

Re: [PATCH v2 4/4] branch: make "-l" a synonym for "--list"

[Ævar Arnfjörð Bjarmason]

On Thu, Aug 30 2018, Jeff King wrote:

> On Thu, Aug 30, 2018 at 01:29:53PM -0700, Junio C Hamano wrote:
>
>> >> > I do not know if the documentation that is shipped in 2.20 should
>> >> > talk about how the old world looked like, though.  `-l` was a short
>> >> > for `--create-reflog` is worth saying, but I do not see much value
>> >> > in talking about the warning given in 2.19.
>> >>
>> >> I'm anticipating that there will be users in the wild with similar -l
>> >> invocations, noting this helps them, because they'll be wondering what
>> >> some script that does "git branch -l <name>" is trying to do while
>> >> reading our docs.
>> >
>> > I don't have a strong opinion either way. If we do mention it, it should
>> > probably be short ("Until Git v2.20, the `-l` option was a synonym for
>> > `--create-reflog").
>>
>> I agree that the short one would of course be good.  I am on the
>> fence about mentioning the warning only given in 2.19.
>
> Yeah, I was confused about that part of the thread. Is there something
> proposed to (additionally) go into v2.19? Ævar, can you elaborate?

The patch I proposed was badly worded and on reflection I don't think
it's useful to include this, but FWIW what I meant was:

 * 1. <2.19: -l is --create-reflog
 * 2. =2.19: -l is --create-reflog, but will spew a warning to stderr about futre deprecation
 * 3. >2.19: -l is --list

I.e. should we in >2.19 docs say that -l used to mean something
different <= 2.19? Yeah, but it's probably worthless information to say
that it used to warn in that one release, since the actionable thing to
do with this information is to change it to --create-reflog, and unlike
going from >2.19 to <2.19 running =2.19 isn't silently going to treat
the -l option in a way you might not expect.