threads / patch / 53351

patchbranch: add '-' to delete previous branch

Subject: [PATCH] branch: add '-' to delete previous branch

## tl;dr

25 messages between Apr 29, 2020 and May 1, 2020. Diffs are folded; open one to read it.

replies: 24people: 7as markdown or json

Ivan Tham· Apr 29, 2020, 13:01 UTC · lore

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(-)
Show changes to 3 files +22 −1

Documentation/git-branch.txt, builtin/branch.c, t/t3200-branch.sh

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
Sergey Organov· Apr 29, 2020, 13:37 UTC · re: Ivan Tham · lore

Re: [PATCH] branch: add '-' to delete previous branch

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
Taylor Blau· Apr 29, 2020, 19:00 UTC · re: Sergey Organov · lore

Re: [PATCH] branch: add '-' to delete previous branch

On Wed, Apr 29, 2020 at 04:37:27PM +0300, Sergey Organov wrote:
Show 24 quoted lines
> 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

Eric Sunshine· Apr 29, 2020, 19:06 UTC · re: Taylor Blau · lore

Re: [PATCH] branch: add '-' to delete previous branch

On Wed, Apr 29, 2020 at 3:00 PM Taylor Blau <me@ttaylorr.com> wrote:
Show 17 quoted lines
> 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.)

Sergey Organov· Apr 29, 2020, 19:50 UTC · re: Taylor Blau · lore

Re: [PATCH] branch: add '-' to delete previous branch

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

Taylor Blau· Apr 29, 2020, 19:57 UTC · re: Sergey Organov · lore

Re: [PATCH] branch: add '-' to delete previous branch

On Wed, Apr 29, 2020 at 10:50:41PM +0300, Sergey Organov wrote:
Show 8 quoted lines
> 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.

Show 12 quoted lines
> 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'?

Show 5 quoted lines
> 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

Junio C Hamano· Apr 29, 2020, 20:22 UTC · re: Taylor Blau · lore

Re: [PATCH] branch: add '-' to delete previous branch

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...
Taylor Blau· Apr 29, 2020, 20:35 UTC · re: Junio C Hamano · lore

Re: [PATCH] branch: add '-' to delete previous branch

On Wed, Apr 29, 2020 at 01:22:36PM -0700, Junio C Hamano wrote:
Show 14 quoted lines
> 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).

Show 5 quoted lines
> 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

Junio C Hamano· Apr 29, 2020, 20:39 UTC · re: Taylor Blau · lore

Re: [PATCH] branch: add '-' to delete previous branch

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.
Junio C Hamano· Apr 29, 2020, 20:37 UTC · re: Junio C Hamano · lore

Re: [PATCH] branch: add '-' to delete previous branch

Junio C Hamano <gitster@pobox.com> writes:
Show 7 quoted lines
> 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.
Show 13 quoted lines
> 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...
Sergey Organov· Apr 29, 2020, 20:26 UTC · re: Taylor Blau · lore

Re: [PATCH] branch: add '-' to delete previous branch

Taylor Blau <me@ttaylorr.com> writes:
Show 13 quoted lines
> 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

Konstantin Khomoutov· Apr 30, 2020, 16:27 UTC · re: Sergey Organov · lore

Re: [PATCH] branch: add '-' to delete previous branch

On Wed, Apr 29, 2020 at 11:26:59PM +0300, Sergey Organov wrote:
[...]
Show 15 quoted lines
> >> > 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
Junio C Hamano· Apr 30, 2020, 19:52 UTC · re: Konstantin Khomoutov · lore

Re: [PATCH] branch: add '-' to delete previous branch

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 ;-).

Sergey Organov· May 1, 2020, 09:18 UTC · re: Junio C Hamano · lore

Re: [PATCH] branch: add '-' to delete previous branch

Junio C Hamano <gitster@pobox.com> writes:
Show 7 quoted lines
> 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
Konstantin Khomoutov· May 1, 2020, 09:44 UTC · re: Sergey Organov · lore

Re: [PATCH] branch: add '-' to delete previous branch

On Fri, May 01, 2020 at 12:18:31PM +0300, Sergey Organov wrote:
[...]
Show 7 quoted lines
> >> 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).

Sergey Organov· May 1, 2020, 10:22 UTC · re: Konstantin Khomoutov · lore

Re: [PATCH] branch: add '-' to delete previous branch

Konstantin Khomoutov <kostix@bswap.ru> writes:
Show 14 quoted lines
> 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.

Show 8 quoted lines
>
>> 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
Taylor Blau· May 1, 2020, 22:22 UTC · re: Sergey Organov · lore

Re: [PATCH] branch: add '-' to delete previous branch

On Fri, May 01, 2020 at 01:22:25PM +0300, Sergey Organov wrote:
Show 22 quoted lines
> 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.
Show 12 quoted lines
> >
> >> 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

Ivan Tham· Apr 30, 2020, 03:43 UTC · re: Sergey Organov · lore

Re: [PATCH] branch: add '-' to delete previous branch

On Wed, Apr 29, 2020 at 10:50:41PM +0300, Sergey Organov wrote:
Show 20 quoted lines
>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.

Show 5 quoted lines
>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.

Junio C Hamano· Apr 29, 2020, 19:27 UTC · re: Sergey Organov · lore

Re: [PATCH] branch: add '-' to delete previous branch

Sergey Organov <sorganov@gmail.com> writes:
Show 6 quoted lines
> 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.

Randall S. Becker· Apr 29, 2020, 19:31 UTC · re: Junio C Hamano · lore

RE: [PATCH] branch: add '-' to delete previous branch

On April 29, 2020 3:28 PM, Junio wrote:
Show 9 quoted lines
> 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

Sergey Organov· Apr 29, 2020, 19:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] branch: add '-' to delete previous branch

Junio C Hamano <gitster@pobox.com> writes:
Show 11 quoted lines
> 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
Taylor Blau· Apr 29, 2020, 18:58 UTC · re: Ivan Tham · lore

Re: [PATCH] branch: add '-' to delete previous branch

On Wed, Apr 29, 2020 at 09:01:33PM +0800, Ivan Tham wrote:
Show 22 quoted lines
> 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}'.

Show 47 quoted lines
>  <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.

Show 6 quoted lines
> +'
> +
>  test_done
> --
> 2.26.2
>

Thanks, Taylor

Ivan Tham· Apr 30, 2020, 14:52 UTC · re: Taylor Blau · lore

Re: [PATCH] branch: add '-' to delete previous branch

On Wed, Apr 29, 2020 at 12:58:51PM -0600, Taylor Blau wrote:
Show 32 quoted lines
>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.

Taylor Blau· Apr 30, 2020, 15:59 UTC · re: Ivan Tham · lore

Re: [PATCH] branch: add '-' to delete previous branch

Hi Ivan,
On Thu, Apr 30, 2020 at 10:52:21PM +0800, Ivan Tham wrote:
Show 36 quoted lines
> 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

Junio C Hamano· Apr 30, 2020, 19:51 UTC · re: Taylor Blau · lore

Re: [PATCH] branch: add '-' to delete previous branch

Taylor Blau <me@ttaylorr.com> writes:
Show 7 quoted lines
> 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.

← back to recent threads