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

2018-08-30 Thread Æ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 " 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.


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

2018-08-30 Thread 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 " 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"

2018-08-30 Thread Junio C Hamano
Jeff King  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 " 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 
> ---
>  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) []]
>   [--contains []]
>   [--points-at ] [--format=] [...]
> -'git branch' [--track | --no-track] [-l] [-f]  []
> +'git branch' [--track | --no-track] [-f]  []
>  'git branch' (--set-upstream-to= | -u ) []
>  'git branch' --unset-upstream []
>  'git branch' (-m | -M) [] 
> @@ -159,10 +159,6 @@ This option is only applicable in non-verbose mode.
>   List branches.  With optional `...`, e.g. `git
>   branch --list 'maint-*'`, list only the branches that match
>   the pattern(s).
> -+
> -This should not be confused with `git branch -l `,
> -which creates a branch named `` with a reflog.
> -See `--create-reflog` above for details.
>  
>  -v::
>  -vv::


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

2018-08-30 Thread 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 " 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 
---
 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) []]
[--contains []]
[--points-at ] [--format=] [...]
-'git branch' [--track | --no-track] [-l] [-f]  []
+'git branch' [--track | --no-track] [-f]  []
 'git branch' (--set-upstream-to= | -u ) []
 'git branch' --unset-upstream []
 'git branch' (-m | -M) [] 
@@ -159,10 +159,6 @@ This option is only applicable in non-verbose mode.
List branches.  With optional `...`, e.g. `git
branch --list 'maint-*'`, list only the branches that match
the pattern(s).
-+
-This should not be confused with `git branch -l `,
-which creates a branch named `` 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"

2018-08-30 Thread Ævar Arnfjörð Bjarmason


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

> Ævar Arnfjörð Bjarmason  writes:
>
>>> +-l::
>>>  --list::
>>> List branches.  With optional `...`, 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 `,
>>  which creates a branch named `` 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 ",
> 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 " 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"

2018-08-30 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason  writes:

>> +-l::
>>  --list::
>>  List branches.  With optional `...`, 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 `,
>  which creates a branch named `` 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 ",
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"

2018-08-30 Thread Æ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 `...`, 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 `,
 which creates a branch named `` 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.


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

2018-06-22 Thread 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 
---
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 `...`, 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, , N_("move/rename a branch, even if 
target exists"), 2),
OPT_BIT('c', "copy", , N_("copy a branch and its reflog"), 
1),
OPT_BIT('C', NULL, , N_("copy a branch, even if target 
exists"), 2),
-   OPT_BOOL(0, "list", , N_("list branch names")),
+   OPT_BOOL('l', "list", , N_("list branch names")),
OPT_BOOL(0, "create-reflog", , N_("create the branch's 
reflog")),
-   {
-   OPTION_CALLBACK, 'l', NULL, , NULL,
-   N_("deprecated synonym for --create-reflog"),
-   PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
-   deprecated_reflog_option_cb
-   },
OPT_BOOL(0, "edit-description", _description,
 N_("edit the description for the branch")),
OPT__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