git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] branch: add '-' to delete previous branch
@ 2020-04-29 13:01 Ivan Tham
  2020-04-29 13:37 ` Sergey Organov
  2020-04-29 18:58 ` Taylor Blau
  0 siblings, 2 replies; 25+ messages in thread
From: Ivan Tham @ 2020-04-29 13:01 UTC (permalink / raw)
  To: git; +Cc: brian m . carlson

Add support to delete previous branch from git checkout/switch to have
feature parity with git switch -.

Signed-off-by: Ivan Tham <pickfire@riseup.net>
---
 Documentation/git-branch.txt | 10 ++++++++++
 builtin/branch.c             |  6 +++++-
 t/t3200-branch.sh            |  7 +++++++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 135206ff4a..37e7cbbc52 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -265,6 +265,10 @@ start-point is either a local or remote-tracking branch.
 	The new branch name must pass all checks defined by
 	linkgit:git-check-ref-format[1].  Some of these checks
 	may restrict the characters allowed in a branch name.
++
+You can use the `@{-N}` syntax to refer to the N-th last branch checked out
+using "git checkout" operation. You may also specify `-` which is synonymous to
+`@{-1}`.
 
 <start-point>::
 	The new branch head will point to this commit.  It may be
@@ -334,6 +338,12 @@ $ git branch -D test                                    <2>
 <2> Delete the "test" branch even if the "master" branch (or whichever branch
     is currently checked out) does not have all commits from the test branch.
 
+To delete the previous branch::
++
+------------
+$ git branch -D -
+------------
+
 Listing branches from a specific remote::
 +
 ------------
diff --git a/builtin/branch.c b/builtin/branch.c
index d8297f80ff..5537f819a6 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -227,9 +227,13 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 	}
 	for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
 		char *target = NULL;
+		const char *arg = argv[i];
 		int flags = 0;
 
-		strbuf_branchname(&bname, argv[i], allowed_interpret);
+		if (!strcmp(arg, "-"))
+			arg = "@{-1}";
+
+		strbuf_branchname(&bname, arg, allowed_interpret);
 		free(name);
 		name = mkpathdup(fmt, bname.buf);
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 411a70b0ce..6819107c1d 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1387,4 +1387,11 @@ test_expect_success 'invalid sort parameter in configuration' '
 	)
 '
 
+test_expect_success 'delete previous branch' '
+	git checkout -b a &&
+	git checkout -b b &&
+	git branch -D - &&
+	test_path_is_missing .git/refs/heads/a
+'
+
 test_done
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 13:01 [PATCH] branch: add '-' to delete previous branch Ivan Tham
@ 2020-04-29 13:37 ` Sergey Organov
  2020-04-29 19:00   ` Taylor Blau
  2020-04-29 19:27   ` Junio C Hamano
  2020-04-29 18:58 ` Taylor Blau
  1 sibling, 2 replies; 25+ messages in thread
From: Sergey Organov @ 2020-04-29 13:37 UTC (permalink / raw)
  To: Ivan Tham; +Cc: git, brian m . carlson

Ivan Tham <pickfire@riseup.net> writes:

> Add support to delete previous branch from git checkout/switch to have
> feature parity with git switch -.

Maybe I'm late on this, but to me, who leaves in the Linux world,
"a_command -" strongly suggests a_command will read further input
from stdin.

[...]

> +To delete the previous branch::
> ++
> +------------
> +$ git branch -D -

... so this suggests that the command, when used like this:

$ echo "branch_name" | git branch -D -

will delete "branch_name" rather than some "previous" branch, whatever
that means.

Is this short-cut /that/ important to create yet another confusion?

-- Sergey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 13:01 [PATCH] branch: add '-' to delete previous branch Ivan Tham
  2020-04-29 13:37 ` Sergey Organov
@ 2020-04-29 18:58 ` Taylor Blau
  2020-04-30 14:52   ` Ivan Tham
  1 sibling, 1 reply; 25+ messages in thread
From: Taylor Blau @ 2020-04-29 18:58 UTC (permalink / raw)
  To: Ivan Tham; +Cc: git, brian m . carlson

On Wed, Apr 29, 2020 at 09:01:33PM +0800, Ivan Tham wrote:
> Add support to delete previous branch from git checkout/switch to have
> feature parity with git switch -.
>
> Signed-off-by: Ivan Tham <pickfire@riseup.net>
> ---
>  Documentation/git-branch.txt | 10 ++++++++++
>  builtin/branch.c             |  6 +++++-
>  t/t3200-branch.sh            |  7 +++++++
>  3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 135206ff4a..37e7cbbc52 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -265,6 +265,10 @@ start-point is either a local or remote-tracking branch.
>  	The new branch name must pass all checks defined by
>  	linkgit:git-check-ref-format[1].  Some of these checks
>  	may restrict the characters allowed in a branch name.
> ++
> +You can use the `@{-N}` syntax to refer to the N-th last branch checked out
> +using "git checkout" operation. You may also specify `-` which is synonymous to
> +`@{-1}`.

Interesting; we're already using strbuf_branchname, so the first part of
this documentation was true even before this commit. Would you consider
splitting this into two patches?

The first should include the first sentence of this documentation, an
additional test in t3200 exercising an explicit 'git branch -D @{-N}'
for some 'N', but no changes in builtin/branch.c. The second patch would
then make '-' a synonym for '@{-1}'.

>  <start-point>::
>  	The new branch head will point to this commit.  It may be
> @@ -334,6 +338,12 @@ $ git branch -D test                                    <2>
>  <2> Delete the "test" branch even if the "master" branch (or whichever branch
>      is currently checked out) does not have all commits from the test branch.
>
> +To delete the previous branch::
> ++
> +------------
> +$ git branch -D -
> +------------
> +
>  Listing branches from a specific remote::
>  +
>  ------------
> diff --git a/builtin/branch.c b/builtin/branch.c
> index d8297f80ff..5537f819a6 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -227,9 +227,13 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
>  	}
>  	for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
>  		char *target = NULL;
> +		const char *arg = argv[i];
>  		int flags = 0;
>
> -		strbuf_branchname(&bname, argv[i], allowed_interpret);
> +		if (!strcmp(arg, "-"))
> +			arg = "@{-1}";
> +
> +		strbuf_branchname(&bname, arg, allowed_interpret);
>  		free(name);
>  		name = mkpathdup(fmt, bname.buf);
>
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 411a70b0ce..6819107c1d 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -1387,4 +1387,11 @@ test_expect_success 'invalid sort parameter in configuration' '
>  	)
>  '
>
> +test_expect_success 'delete previous branch' '
> +	git checkout -b a &&
> +	git checkout -b b &&
> +	git branch -D - &&
> +	test_path_is_missing .git/refs/heads/a

This assertion is a little uncommon in t3200 for this style of test.
Please use the more-standard:

  git branch >actual &&
  cat >expect <<-\EOF &&
     master
   * b
  EOF
  test_cmp expect actual

Substituting the body of the heredoc for the actual state of things.
This form makes it a little clearer what branches should and shouldn't
exist.

> +'
> +
>  test_done
> --
> 2.26.2
>

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 13:37 ` Sergey Organov
@ 2020-04-29 19:00   ` Taylor Blau
  2020-04-29 19:06     ` Eric Sunshine
  2020-04-29 19:50     ` Sergey Organov
  2020-04-29 19:27   ` Junio C Hamano
  1 sibling, 2 replies; 25+ messages in thread
From: Taylor Blau @ 2020-04-29 19:00 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Ivan Tham, git, brian m . carlson

On Wed, Apr 29, 2020 at 04:37:27PM +0300, Sergey Organov wrote:
> Ivan Tham <pickfire@riseup.net> writes:
>
> > Add support to delete previous branch from git checkout/switch to have
> > feature parity with git switch -.
>
> Maybe I'm late on this, but to me, who leaves in the Linux world,
> "a_command -" strongly suggests a_command will read further input
> from stdin.
>
> [...]
>
> > +To delete the previous branch::
> > ++
> > +------------
> > +$ git branch -D -
>
> ... so this suggests that the command, when used like this:
>
> $ echo "branch_name" | git branch -D -
>
> will delete "branch_name" rather than some "previous" branch, whatever
> that means.
>
> Is this short-cut /that/ important to create yet another confusion?

I think that it may be causing more confusion now than it would be after
Ivan's patch. 'git checkout', for example, also treats '-' as a synonym
for '@{-1}'.

In my opinion, it is fairly clear that 'git branch -D -' means "delete
the last branch", and not "delete a list of branches from stdin.

> -- Sergey

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 19:00   ` Taylor Blau
@ 2020-04-29 19:06     ` Eric Sunshine
  2020-04-29 19:50     ` Sergey Organov
  1 sibling, 0 replies; 25+ messages in thread
From: Eric Sunshine @ 2020-04-29 19:06 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Sergey Organov, Ivan Tham, Git List, brian m . carlson

On Wed, Apr 29, 2020 at 3:00 PM Taylor Blau <me@ttaylorr.com> wrote:
> On Wed, Apr 29, 2020 at 04:37:27PM +0300, Sergey Organov wrote:
> > Ivan Tham <pickfire@riseup.net> writes:
> > > Add support to delete previous branch from git checkout/switch to have
> > > feature parity with git switch -.
> >
> > Maybe I'm late on this, but to me, who leaves in the Linux world,
> > "a_command -" strongly suggests a_command will read further input
> > from stdin.
> >
> > Is this short-cut /that/ important to create yet another confusion?
>
> I think that it may be causing more confusion now than it would be after
> Ivan's patch. 'git checkout', for example, also treats '-' as a synonym
> for '@{-1}'.
>
> In my opinion, it is fairly clear that 'git branch -D -' means "delete
> the last branch", and not "delete a list of branches from stdin.

I can't find the discussion right now, but I have a recollection that
"git branch -D -" was explicitly rejected when "-" was being added as
an alias for @{-1} to other modes and other commands. The reason for
the rejection was that branch deletion is a destructive operation (the
ref-log, in particular, is gone after deletion), and "-" alone
provides no clues and no context that you're deleting the branch you
intend to delete. (The lack of clues and context isn't a big deal for
non-destructive commands, such as git-switch.)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 13:37 ` Sergey Organov
  2020-04-29 19:00   ` Taylor Blau
@ 2020-04-29 19:27   ` Junio C Hamano
  2020-04-29 19:31     ` Randall S. Becker
  2020-04-29 19:55     ` Sergey Organov
  1 sibling, 2 replies; 25+ messages in thread
From: Junio C Hamano @ 2020-04-29 19:27 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Ivan Tham, git, brian m . carlson

Sergey Organov <sorganov@gmail.com> writes:

> Ivan Tham <pickfire@riseup.net> writes:
>
>> Add support to delete previous branch from git checkout/switch to have
>> feature parity with git switch -.
> ...
> Is this short-cut /that/ important to create yet another confusion?

I do not think it is that important, but I do not think "cd -" is so
important, either.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 19:27   ` Junio C Hamano
@ 2020-04-29 19:31     ` Randall S. Becker
  2020-04-29 19:55     ` Sergey Organov
  1 sibling, 0 replies; 25+ messages in thread
From: Randall S. Becker @ 2020-04-29 19:31 UTC (permalink / raw)
  To: 'Junio C Hamano', 'Sergey Organov'
  Cc: 'Ivan Tham', git, 'brian m . carlson'

On April 29, 2020 3:28 PM, Junio wrote:
> Sergey Organov <sorganov@gmail.com> writes:
> > Ivan Tham <pickfire@riseup.net> writes:
> >
> >> Add support to delete previous branch from git checkout/switch to
> >> have feature parity with git switch -.
> > ...
> > Is this short-cut /that/ important to create yet another confusion?
> 
> I do not think it is that important, but I do not think "cd -" is so
important,
> either.

I wouldn't mind a switch - to the previously switched-out branch rather than
supporting this for delete. Alternatively, what about a switch --push and a
switch --pop like something out of bash pushd/popd.

Regards,
Randall




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 19:00   ` Taylor Blau
  2020-04-29 19:06     ` Eric Sunshine
@ 2020-04-29 19:50     ` Sergey Organov
  2020-04-29 19:57       ` Taylor Blau
  2020-04-30  3:43       ` Ivan Tham
  1 sibling, 2 replies; 25+ messages in thread
From: Sergey Organov @ 2020-04-29 19:50 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Ivan Tham, git, brian m . carlson

Taylor Blau <me@ttaylorr.com> writes:

[...]

> In my opinion, it is fairly clear that 'git branch -D -' means "delete
> the last branch", and not "delete a list of branches from stdin.

Honestly, I'd never guess it'd "delete the last branch". No way.

"-" standing by itself in a command means stdin, stdout, or otherwise a
typo. Using it for any other meaning is a blasphemy. Sure, nobody will
die because of this, but it's /extremely/ confusing!

BTW, what about mistyping:

$ git branch -d - f my_branch

for

$ git branch -d -f my_branch

or some such?

No, it still doesn't look like a good idea to use isolated '-' as
suggested by the patch.

OTOH, for otherwise unusual @{-1}, @{-}, or @- I'd immediately realize I
must consult the manual, so these would be fine with me.

Thanks,
-- Sergey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 19:27   ` Junio C Hamano
  2020-04-29 19:31     ` Randall S. Becker
@ 2020-04-29 19:55     ` Sergey Organov
  1 sibling, 0 replies; 25+ messages in thread
From: Sergey Organov @ 2020-04-29 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ivan Tham, git, brian m . carlson

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> Ivan Tham <pickfire@riseup.net> writes:
>>
>>> Add support to delete previous branch from git checkout/switch to have
>>> feature parity with git switch -.
>> ...
>> Is this short-cut /that/ important to create yet another confusion?
>
> I do not think it is that important, but I do not think "cd -" is so
> important, either.

Sure, and git shouldn't need to repeat other's mistakes.

-- Sergey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 19:50     ` Sergey Organov
@ 2020-04-29 19:57       ` Taylor Blau
  2020-04-29 20:22         ` Junio C Hamano
  2020-04-29 20:26         ` Sergey Organov
  2020-04-30  3:43       ` Ivan Tham
  1 sibling, 2 replies; 25+ messages in thread
From: Taylor Blau @ 2020-04-29 19:57 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Taylor Blau, Ivan Tham, git, brian m . carlson

On Wed, Apr 29, 2020 at 10:50:41PM +0300, Sergey Organov wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> [...]
>
> > In my opinion, it is fairly clear that 'git branch -D -' means "delete
> > the last branch", and not "delete a list of branches from stdin.
>
> Honestly, I'd never guess it'd "delete the last branch". No way.

I'm having trouble understanding why. This is how 'git checkout -'
behaves, so I have no idea why 'git branch' wouldn't work the same way.

> "-" standing by itself in a command means stdin, stdout, or otherwise a
> typo. Using it for any other meaning is a blasphemy. Sure, nobody will
> die because of this, but it's /extremely/ confusing!

Again, not sure that this is always the case. This *is* how 'git
checkout' works.

> BTW, what about mistyping:
>
> $ git branch -d - f my_branch
>
> for
>
> $ git branch -d -f my_branch
>
> or some such?
>
> No, it still doesn't look like a good idea to use isolated '-' as
> suggested by the patch.

Frankly, I do not find this compelling. Does that mean that '/' as a
directory separator is dangerous, too, because you can accidentally
write 'rm -rf / foo/bar/baz'?

> OTOH, for otherwise unusual @{-1}, @{-}, or @- I'd immediately realize I
> must consult the manual, so these would be fine with me.
>
> Thanks,
> -- Sergey

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 19:57       ` Taylor Blau
@ 2020-04-29 20:22         ` Junio C Hamano
  2020-04-29 20:35           ` Taylor Blau
  2020-04-29 20:37           ` Junio C Hamano
  2020-04-29 20:26         ` Sergey Organov
  1 sibling, 2 replies; 25+ messages in thread
From: Junio C Hamano @ 2020-04-29 20:22 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Sergey Organov, Ivan Tham, git, brian m . carlson

Taylor Blau <me@ttaylorr.com> writes:

> Again, not sure that this is always the case. This *is* how 'git
> checkout' works.

To be honest, I am somewhat sympathetic to those who find "-" ==
"@{-1}" unless it is used as an argument to "git checkout/switch".
The use of "-" in "checkout" is the exception, not the norm, and it
was sort of justifiable due to similarity to "cd -".  Both are
commands to the computer you give to "go to the previous place".

"git merge -", "git branch -d -" etc. are not about *going* to the
previous place, and declaring the "-" is "previous place" is taking
it a bit too far, at least to my taste.

Oh, I do not like those who advocate "@" as a synonym for "HEAD",
either.  If there is one simple thing I want to get rid of from the
system, that's it ;-).

Anyway...


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 19:57       ` Taylor Blau
  2020-04-29 20:22         ` Junio C Hamano
@ 2020-04-29 20:26         ` Sergey Organov
  2020-04-30 16:27           ` Konstantin Khomoutov
  1 sibling, 1 reply; 25+ messages in thread
From: Sergey Organov @ 2020-04-29 20:26 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Ivan Tham, git, brian m . carlson

Taylor Blau <me@ttaylorr.com> writes:

> On Wed, Apr 29, 2020 at 10:50:41PM +0300, Sergey Organov wrote:
>> Taylor Blau <me@ttaylorr.com> writes:
>>
>> [...]
>>
>> > In my opinion, it is fairly clear that 'git branch -D -' means "delete
>> > the last branch", and not "delete a list of branches from stdin.
>>
>> Honestly, I'd never guess it'd "delete the last branch". No way.
>
> I'm having trouble understanding why. This is how 'git checkout -'
> behaves, so I have no idea why 'git branch' wouldn't work the same
> way.

Well, if I knew 'git checkout -' does this, then yes, it'd be obvious.
The problem is that I didn't. Well, then, as I said in my original
reaction, I'm probably too late on this.

BTW, it was not that easy to find it in the "git help checkout" even
when I was specifically looking for it.

Thanks,
-- Sergey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 20:22         ` Junio C Hamano
@ 2020-04-29 20:35           ` Taylor Blau
  2020-04-29 20:39             ` Junio C Hamano
  2020-04-29 20:37           ` Junio C Hamano
  1 sibling, 1 reply; 25+ messages in thread
From: Taylor Blau @ 2020-04-29 20:35 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Taylor Blau, Sergey Organov, Ivan Tham, git, brian m . carlson

On Wed, Apr 29, 2020 at 01:22:36PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > Again, not sure that this is always the case. This *is* how 'git
> > checkout' works.
>
> To be honest, I am somewhat sympathetic to those who find "-" ==
> "@{-1}" unless it is used as an argument to "git checkout/switch".
> The use of "-" in "checkout" is the exception, not the norm, and it
> was sort of justifiable due to similarity to "cd -".  Both are
> commands to the computer you give to "go to the previous place".
>
> "git merge -", "git branch -d -" etc. are not about *going* to the
> previous place, and declaring the "-" is "previous place" is taking
> it a bit too far, at least to my taste.

OK, I could sympathize with that as well. I still think that my
suggestion from earlier about documenting the fact that 'git branch -D'
already understands '@{-N}' as a separate first patch is valid.

If I were the author, I'd cut that as a first patch, and discard the
remainder if it sounds like we don't want to go with 'git branch -D -',
which is fine by me. (I don't really care either way, and I can
understand the arguments in both directions).

> Oh, I do not like those who advocate "@" as a synonym for "HEAD",
> either.  If there is one simple thing I want to get rid of from the
> system, that's it ;-).
>
> Anyway...

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 20:22         ` Junio C Hamano
  2020-04-29 20:35           ` Taylor Blau
@ 2020-04-29 20:37           ` Junio C Hamano
  1 sibling, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2020-04-29 20:37 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Sergey Organov, Ivan Tham, git, brian m . carlson

Junio C Hamano <gitster@pobox.com> writes:

> Taylor Blau <me@ttaylorr.com> writes:
>
>> Again, not sure that this is always the case. This *is* how 'git
>> checkout' works.
>
> To be honest, I am somewhat sympathetic to those who find "-" ==
> "@{-1}" unless it is used as an argument to "git checkout/switch".

Insert "confusing" before "unless".  Sorry about the typo.


> The use of "-" in "checkout" is the exception, not the norm, and it
> was sort of justifiable due to similarity to "cd -".  Both are
> commands to the computer you give to "go to the previous place".
>
> "git merge -", "git branch -d -" etc. are not about *going* to the
> previous place, and declaring the "-" is "previous place" is taking
> it a bit too far, at least to my taste.
>
> Oh, I do not like those who advocate "@" as a synonym for "HEAD",
> either.  If there is one simple thing I want to get rid of from the
> system, that's it ;-).
>
> Anyway...

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 20:35           ` Taylor Blau
@ 2020-04-29 20:39             ` Junio C Hamano
  0 siblings, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2020-04-29 20:39 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Sergey Organov, Ivan Tham, git, brian m . carlson

Taylor Blau <me@ttaylorr.com> writes:

> OK, I could sympathize with that as well. I still think that my
> suggestion from earlier about documenting the fact that 'git branch -D'
> already understands '@{-N}' as a separate first patch is valid.

No question about that ;-)

> If I were the author, I'd cut that as a first patch, and discard the
> remainder if it sounds like we don't want to go with 'git branch -D -',
> which is fine by me. (I don't really care either way, and I can
> understand the arguments in both directions).

Yup, that would be what I would do, too, whether I wanted "-d -" or not.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 19:50     ` Sergey Organov
  2020-04-29 19:57       ` Taylor Blau
@ 2020-04-30  3:43       ` Ivan Tham
  1 sibling, 0 replies; 25+ messages in thread
From: Ivan Tham @ 2020-04-30  3:43 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Taylor Blau, git, brian m . carlson

On Wed, Apr 29, 2020 at 10:50:41PM +0300, Sergey Organov wrote:
>Taylor Blau <me@ttaylorr.com> writes:
>
>> In my opinion, it is fairly clear that 'git branch -D -' means "delete
>> the last branch", and not "delete a list of branches from stdin.
>
>Honestly, I'd never guess it'd "delete the last branch". No way.
>
>"-" standing by itself in a command means stdin, stdout, or otherwise a
>typo. Using it for any other meaning is a blasphemy. Sure, nobody will
>die because of this, but it's /extremely/ confusing!
>
>BTW, what about mistyping:
>
>$ git branch -d - f my_branch
>
>for
>
>$ git branch -d -f my_branch
>
>or some such?

I already knew `git checkout -` and `git switch -` exists and have been
using them quite frequently as my workflow, but when I wanted to go back
to my original branch and delete the branch, I tried `git branch -D -`
quite a few times and I am surprised it does not work as expected.

Yes, that typo would have deleted a branch but it could be restored from
reflog at the very least.

>No, it still doesn't look like a good idea to use isolated '-' as
>suggested by the patch.
>
>OTOH, for otherwise unusual @{-1}, @{-}, or @- I'd immediately realize I
>must consult the manual, so these would be fine with me.

But yes, I didn't know @{-1} or @{-} or @- exists before I was sending
this patch, I only know I can use - which is very simple.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 18:58 ` Taylor Blau
@ 2020-04-30 14:52   ` Ivan Tham
  2020-04-30 15:59     ` Taylor Blau
  0 siblings, 1 reply; 25+ messages in thread
From: Ivan Tham @ 2020-04-30 14:52 UTC (permalink / raw)
  To: Taylor Blau; +Cc: git, brian m . carlson

On Wed, Apr 29, 2020 at 12:58:51PM -0600, Taylor Blau wrote:
>On Wed, Apr 29, 2020 at 09:01:33PM +0800, Ivan Tham wrote:
>> Add support to delete previous branch from git checkout/switch to have
>> feature parity with git switch -.
>>
>> Signed-off-by: Ivan Tham <pickfire@riseup.net>
>> ---
>>  Documentation/git-branch.txt | 10 ++++++++++
>>  builtin/branch.c             |  6 +++++-
>>  t/t3200-branch.sh            |  7 +++++++
>>  3 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
>> index 135206ff4a..37e7cbbc52 100644
>> --- a/Documentation/git-branch.txt
>> +++ b/Documentation/git-branch.txt
>> @@ -265,6 +265,10 @@ start-point is either a local or remote-tracking branch.
>>  	The new branch name must pass all checks defined by
>>  	linkgit:git-check-ref-format[1].  Some of these checks
>>  	may restrict the characters allowed in a branch name.
>> ++
>> +You can use the `@{-N}` syntax to refer to the N-th last branch checked out
>> +using "git checkout" operation. You may also specify `-` which is synonymous to
>> +`@{-1}`.
>
>Interesting; we're already using strbuf_branchname, so the first part of
>this documentation was true even before this commit. Would you consider
>splitting this into two patches?
>
>The first should include the first sentence of this documentation, an
>additional test in t3200 exercising an explicit 'git branch -D @{-N}'
>for some 'N', but no changes in builtin/branch.c. The second patch would
>then make '-' a synonym for '@{-1}'.

Is not this already true? Why do we need to split it? Currently using
'@{-N}' to delete branch already works.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-30 14:52   ` Ivan Tham
@ 2020-04-30 15:59     ` Taylor Blau
  2020-04-30 19:51       ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Taylor Blau @ 2020-04-30 15:59 UTC (permalink / raw)
  To: Ivan Tham; +Cc: Taylor Blau, git, brian m . carlson

Hi Ivan,

On Thu, Apr 30, 2020 at 10:52:21PM +0800, Ivan Tham wrote:
> On Wed, Apr 29, 2020 at 12:58:51PM -0600, Taylor Blau wrote:
> > On Wed, Apr 29, 2020 at 09:01:33PM +0800, Ivan Tham wrote:
> > > Add support to delete previous branch from git checkout/switch to have
> > > feature parity with git switch -.
> > >
> > > Signed-off-by: Ivan Tham <pickfire@riseup.net>
> > > ---
> > >  Documentation/git-branch.txt | 10 ++++++++++
> > >  builtin/branch.c             |  6 +++++-
> > >  t/t3200-branch.sh            |  7 +++++++
> > >  3 files changed, 22 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> > > index 135206ff4a..37e7cbbc52 100644
> > > --- a/Documentation/git-branch.txt
> > > +++ b/Documentation/git-branch.txt
> > > @@ -265,6 +265,10 @@ start-point is either a local or remote-tracking branch.
> > >  	The new branch name must pass all checks defined by
> > >  	linkgit:git-check-ref-format[1].  Some of these checks
> > >  	may restrict the characters allowed in a branch name.
> > > ++
> > > +You can use the `@{-N}` syntax to refer to the N-th last branch checked out
> > > +using "git checkout" operation. You may also specify `-` which is synonymous to
> > > +`@{-1}`.
> >
> > Interesting; we're already using strbuf_branchname, so the first part of
> > this documentation was true even before this commit. Would you consider
> > splitting this into two patches?
> >
> > The first should include the first sentence of this documentation, an
> > additional test in t3200 exercising an explicit 'git branch -D @{-N}'
> > for some 'N', but no changes in builtin/branch.c. The second patch would
> > then make '-' a synonym for '@{-1}'.
>
> Is not this already true? Why do we need to split it? Currently using
> '@{-N}' to delete branch already works.

It is the case that 'git branch -D @{-N}' already works, which was
surprising to me. I am suggesting that you make documenting the existent
behavior a new first patch.

Of course, that might be the only patch that you end up sending in v2,
since it seems that consensus has formed around not supposing '-' as a
shorthand for '@{-1}' with 'git branch -D'.

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-29 20:26         ` Sergey Organov
@ 2020-04-30 16:27           ` Konstantin Khomoutov
  2020-04-30 19:52             ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Konstantin Khomoutov @ 2020-04-30 16:27 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Taylor Blau, Ivan Tham, git, brian m . carlson

On Wed, Apr 29, 2020 at 11:26:59PM +0300, Sergey Organov wrote:

[...]
> >> > In my opinion, it is fairly clear that 'git branch -D -' means "delete
> >> > the last branch", and not "delete a list of branches from stdin.
> >>
> >> Honestly, I'd never guess it'd "delete the last branch". No way.
> >
> > I'm having trouble understanding why. This is how 'git checkout -'
> > behaves, so I have no idea why 'git branch' wouldn't work the same
> > way.
> 
> Well, if I knew 'git checkout -' does this, then yes, it'd be obvious.
> The problem is that I didn't. Well, then, as I said in my original
> reaction, I'm probably too late on this.
> 
> BTW, it was not that easy to find it in the "git help checkout" even
> when I was specifically looking for it.

I would speculate that `git checkout -` may have learned about "-"
simply from the `cd -` form of the standard Unix shell (also codified by
POSIX [1]) which behaved that way - change the current directory to the
previous one - since forever. ;-)

1. https://pubs.opengroup.org/onlinepubs/9699919799/utilities/cd.html


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-30 15:59     ` Taylor Blau
@ 2020-04-30 19:51       ` Junio C Hamano
  0 siblings, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2020-04-30 19:51 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Ivan Tham, git, brian m . carlson

Taylor Blau <me@ttaylorr.com> writes:

> It is the case that 'git branch -D @{-N}' already works, which was
> surprising to me. I am suggesting that you make documenting the existent
> behavior a new first patch.
>
> Of course, that might be the only patch that you end up sending in v2,
> since it seems that consensus has formed around not supposing '-' as a
> shorthand for '@{-1}' with 'git branch -D'.

Thanks for concisely summarizing the current status of the
discussion.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-30 16:27           ` Konstantin Khomoutov
@ 2020-04-30 19:52             ` Junio C Hamano
  2020-05-01  9:18               ` Sergey Organov
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2020-04-30 19:52 UTC (permalink / raw)
  To: Konstantin Khomoutov
  Cc: Sergey Organov, Taylor Blau, Ivan Tham, git, brian m . carlson

Konstantin Khomoutov <kostix@bswap.ru> writes:

> I would speculate that `git checkout -` may have learned about "-"
> simply from the `cd -` ...

You do not have to speculate.  You only need to read what has been
already said in the thread ;-).


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-04-30 19:52             ` Junio C Hamano
@ 2020-05-01  9:18               ` Sergey Organov
  2020-05-01  9:44                 ` Konstantin Khomoutov
  0 siblings, 1 reply; 25+ messages in thread
From: Sergey Organov @ 2020-05-01  9:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Konstantin Khomoutov, Taylor Blau, Ivan Tham, git,
	brian m . carlson

Junio C Hamano <gitster@pobox.com> writes:

> Konstantin Khomoutov <kostix@bswap.ru> writes:
>
>> I would speculate that `git checkout -` may have learned about "-"
>> simply from the `cd -` ...
>
> You do not have to speculate.  You only need to read what has been
> already said in the thread ;-).

Should I expect "git checkout ~" to get me to my "home" branch then? ;-)

Actually, that could be a good idea. I mean, to have "home" branch
notion and a short-cut for it.

-- Sergey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-05-01  9:18               ` Sergey Organov
@ 2020-05-01  9:44                 ` Konstantin Khomoutov
  2020-05-01 10:22                   ` Sergey Organov
  0 siblings, 1 reply; 25+ messages in thread
From: Konstantin Khomoutov @ 2020-05-01  9:44 UTC (permalink / raw)
  To: Sergey Organov
  Cc: Junio C Hamano, Konstantin Khomoutov, Taylor Blau, Ivan Tham, git,
	brian m . carlson

On Fri, May 01, 2020 at 12:18:31PM +0300, Sergey Organov wrote:

[...]
> >> I would speculate that `git checkout -` may have learned about "-"
> >> simply from the `cd -` ...
> >
> > You do not have to speculate.  You only need to read what has been
> > already said in the thread ;-).
> 
> Should I expect "git checkout ~" to get me to my "home" branch then? ;-)

Heh, no you shouldn't: the '~' is expanded by the shell itself according
to its parameter expansion rules, so `cd` never sees the bare tilde
in this case ;-)

> Actually, that could be a good idea. I mean, to have "home" branch
> notion and a short-cut for it.

I have no say for the idea per se but you would have hard time using
bare tilda character for that precisely because of it being special to
Unix shells - you'd need to escape it all the time (though something
like unquoted @{~} would work just OK).


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-05-01  9:44                 ` Konstantin Khomoutov
@ 2020-05-01 10:22                   ` Sergey Organov
  2020-05-01 22:22                     ` Taylor Blau
  0 siblings, 1 reply; 25+ messages in thread
From: Sergey Organov @ 2020-05-01 10:22 UTC (permalink / raw)
  To: Konstantin Khomoutov
  Cc: Junio C Hamano, Taylor Blau, Ivan Tham, git, brian m . carlson

Konstantin Khomoutov <kostix@bswap.ru> writes:

> On Fri, May 01, 2020 at 12:18:31PM +0300, Sergey Organov wrote:
>
> [...]
>> >> I would speculate that `git checkout -` may have learned about "-"
>> >> simply from the `cd -` ...
>> >
>> > You do not have to speculate.  You only need to read what has been
>> > already said in the thread ;-).
>> 
>> Should I expect "git checkout ~" to get me to my "home" branch then? ;-)
>
> Heh, no you shouldn't: the '~' is expanded by the shell itself according
> to its parameter expansion rules, so `cd` never sees the bare tilde
> in this case ;-)

Yeah, but then git could compare its argument to $HOME and "do the right
thing" ;-) Just kidding, obviously.

That said, bare "cd" also changes to home directory, so bare "git
checkout" could become as useful.

>
>> Actually, that could be a good idea. I mean, to have "home" branch
>> notion and a short-cut for it.
>
> I have no say for the idea per se but you would have hard time using
> bare tilda character for that precisely because of it being special to
> Unix shells - you'd need to escape it all the time (though something
> like unquoted @{~} would work just OK).

Yes, plain '~' won't do.

-- Sergey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] branch: add '-' to delete previous branch
  2020-05-01 10:22                   ` Sergey Organov
@ 2020-05-01 22:22                     ` Taylor Blau
  0 siblings, 0 replies; 25+ messages in thread
From: Taylor Blau @ 2020-05-01 22:22 UTC (permalink / raw)
  To: Sergey Organov
  Cc: Konstantin Khomoutov, Junio C Hamano, Taylor Blau, Ivan Tham, git,
	brian m . carlson

On Fri, May 01, 2020 at 01:22:25PM +0300, Sergey Organov wrote:
> Konstantin Khomoutov <kostix@bswap.ru> writes:
>
> > On Fri, May 01, 2020 at 12:18:31PM +0300, Sergey Organov wrote:
> >
> > [...]
> >> >> I would speculate that `git checkout -` may have learned about "-"
> >> >> simply from the `cd -` ...
> >> >
> >> > You do not have to speculate.  You only need to read what has been
> >> > already said in the thread ;-).
> >>
> >> Should I expect "git checkout ~" to get me to my "home" branch then? ;-)
> >
> > Heh, no you shouldn't: the '~' is expanded by the shell itself according
> > to its parameter expansion rules, so `cd` never sees the bare tilde
> > in this case ;-)
>
> Yeah, but then git could compare its argument to $HOME and "do the right
> thing" ;-) Just kidding, obviously.
>
> That said, bare "cd" also changes to home directory, so bare "git
> checkout" could become as useful.

My qualms about the general usefulness of a bare 'git checkout' going to
some bookmarked branch aside, this won't work, since a bare 'git
checkout' already sets HEAD to the currently checked-out branch.

That said, I am not at all sold on this being a good idea.

> >
> >> Actually, that could be a good idea. I mean, to have "home" branch
> >> notion and a short-cut for it.
> >
> > I have no say for the idea per se but you would have hard time using
> > bare tilda character for that precisely because of it being special to
> > Unix shells - you'd need to escape it all the time (though something
> > like unquoted @{~} would work just OK).
>
> Yes, plain '~' won't do.
>
> -- Sergey

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2020-05-01 22:22 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-29 13:01 [PATCH] branch: add '-' to delete previous branch Ivan Tham
2020-04-29 13:37 ` Sergey Organov
2020-04-29 19:00   ` Taylor Blau
2020-04-29 19:06     ` Eric Sunshine
2020-04-29 19:50     ` Sergey Organov
2020-04-29 19:57       ` Taylor Blau
2020-04-29 20:22         ` Junio C Hamano
2020-04-29 20:35           ` Taylor Blau
2020-04-29 20:39             ` Junio C Hamano
2020-04-29 20:37           ` Junio C Hamano
2020-04-29 20:26         ` Sergey Organov
2020-04-30 16:27           ` Konstantin Khomoutov
2020-04-30 19:52             ` Junio C Hamano
2020-05-01  9:18               ` Sergey Organov
2020-05-01  9:44                 ` Konstantin Khomoutov
2020-05-01 10:22                   ` Sergey Organov
2020-05-01 22:22                     ` Taylor Blau
2020-04-30  3:43       ` Ivan Tham
2020-04-29 19:27   ` Junio C Hamano
2020-04-29 19:31     ` Randall S. Becker
2020-04-29 19:55     ` Sergey Organov
2020-04-29 18:58 ` Taylor Blau
2020-04-30 14:52   ` Ivan Tham
2020-04-30 15:59     ` Taylor Blau
2020-04-30 19:51       ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).