[PATCH] branch: delete now accepts '-' as branch name

[PATCH] branch: delete now accepts '-' as branch name

[Erlend Egeberg Aasland via GitGitGadget]

From: "Erlend E. Aasland" <erlend.aasland@innova.no>

This makes it easy to get rid of short-lived branches:

$ git switch -c experiment
$ git switch -
$ git branch -D -

$ gh pr checkout nnn
$ make && make test
$ git switch -
$ git branch -D -

Signed-off-by: Erlend E. Aasland <erlend.aasland@innova.no>
---
    [RFC] branch: delete now accepts '-' as branch name

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1217%2Ferlend-aasland%2Fbranch-name-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1217/erlend-aasland/branch-name-v1
Pull-Request: https://github.com/git/git/pull/1217

 builtin/branch.c  | 6 +++++-
 t/t3200-branch.sh | 8 ++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 4ce2a247542..f6c876c09d2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -236,7 +236,11 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 		char *target = NULL;
 		int flags = 0;
 
-		strbuf_branchname(&bname, argv[i], allowed_interpret);
+		if (!strcmp(argv[i], "-")) {
+			strbuf_branchname(&bname, "@{-1}", allowed_interpret);
+		} else {
+			strbuf_branchname(&bname, argv[i], allowed_interpret);
+		}
 		free(name);
 		name = mkpathdup(fmt, bname.buf);
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 1bc3795847d..6afb7150534 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -71,6 +71,14 @@ test_expect_success 'git branch -d d/e/f should delete a branch and a log' '
 	test_must_fail git reflog exists refs/heads/d/e/f
 '
 
+test_expect_success 'git branch -D - should delete branch @{-1}' '
+	git switch -c j &&
+	git switch - &&
+	git branch -D - &&
+	test_path_is_missing .git/refs/heads/j &&
+	test_must_fail git reflog exists refs/heads/j
+'
+
 test_expect_success 'git branch j/k should work after branch j has been deleted' '
 	git branch j &&
 	git branch -d j &&

base-commit: b80121027d1247a0754b3cc46897fee75c050b44
-- 
gitgitgadget

Re: [PATCH] branch: delete now accepts '-' as branch name

[Junio C Hamano]

> From: "Erlend E. Aasland" <erlend.aasland@innova.no>
>
> This makes it easy to get rid of short-lived branches:
>
> $ git switch -c experiment
> $ git switch -
> $ git branch -D -

Or you can use @{-1} directly.  Or short-lived experiment can
directly be done on HEAD without any branch ;-)

In any case, the above is sufficient demonstration.  I do not think
you'd need a second one that is identical---having two may help you
demonstrate there are multiple ways to start a short-lived branch
that you want to get rid of quickly, but it does not help convince
others how useful "branch -" is in such situations.

> $ gh pr checkout nnn
> $ make && make test
> $ git switch -
> $ git branch -D -
>
> Signed-off-by: Erlend E. Aasland <erlend.aasland@innova.no>
> ---


> diff --git a/builtin/branch.c b/builtin/branch.c
> index 4ce2a247542..f6c876c09d2 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -236,7 +236,11 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
>  		char *target = NULL;
>  		int flags = 0;
>  
> -		strbuf_branchname(&bname, argv[i], allowed_interpret);
> +		if (!strcmp(argv[i], "-")) {
> +			strbuf_branchname(&bname, "@{-1}", allowed_interpret);
> +		} else {
> +			strbuf_branchname(&bname, argv[i], allowed_interpret);
> +		}
>  		free(name);
>  		name = mkpathdup(fmt, bname.buf);

You do not need if/else here.

+	if (!strcmp(argv[i], "-"))
+		argv[i] = "@{-1}";
	strbuf_branchname(&bname, argv[i], allowed_interpret);

should be sufficient.  If you want to avoid assigning to argv[i],
if/else is OK, but then lose the unnecessary {pair of braces} around
single-statement blocks.

> +test_expect_success 'git branch -D - should delete branch @{-1}' '
> +	git switch -c j &&
> +	git switch - &&
> +	git branch -D - &&
> +	test_path_is_missing .git/refs/heads/j &&

These days we try hard *NOT* to assume how refs are stored in the
system.  Please don't peek into .git/ at filesystem level, when you
can achieve what you want to in some other way.

    test_must_fail git show-ref --verify refs/heads/j

would be a more direct way.

> +	test_must_fail git reflog exists refs/heads/j

We should try not to assume that reflog for ref X is lost when ref X
itself goes away in newer tests and update such assumptions in older
tests.  Ideally we would want to be able to say "at time X, branch
'j' was removed, and at time X+Y, branch 'j' was recreated", and
adding more of these will make such an improvement harder to make.

This script is riddled with instances of these existing problems.
Please don't make it even worse by adding on top of them.

The added ones is a positive test.  A new feature works as expected
in a situation where it should kick in.  There needs an accompanying
negative test or two, making sure it fails sensibly in a situation
where it should fail.  Off the top of my head, negative tests that
are appropriate for this new feature are:

    * In a freshly created repository, possibly creating a few
      commits on the initial branch without ever switching to a
      different branch, what should happen when you use the new
      "branch -d -" feature?  What error message, if any, should
      the user see?  Do we show that expected message?

    * Switch to a new branch, create a commit on top, switch back to
      the original branch and run "branch -d -".  This should fail
      because you'd be losing the commit created on the branch.
      What error message should the user see?  Do we show the name
      of the branch correctly?  Do we show "@{-1}", or "-"?

    * After running "branch -D -" to successfully remove the
      previous branch, run "branch -D -" again.  This should fail
      because the branch you are trying to remove no longer exists.
      What error message should the user see?

Thanks for trying to make Git better.

Re: [PATCH] branch: delete now accepts '-' as branch name

[Eric Sunshine]

On 2/16/22 11:54 AM, Junio C Hamano wrote:
> From: "Erlend E. Aasland" <erlend.aasland@innova.no>
>> This makes it easy to get rid of short-lived branches:
>>
>> $ git switch -c experiment
>> $ git switch -
>> $ git branch -D -
> 
> Or you can use @{-1} directly.  Or short-lived experiment can
> directly be done on HEAD without any branch ;-)
> 
> Thanks for trying to make Git better.

Patches implementing this behavior for `git branch -D` have been 
submitted previously but were rejected (if I recall correctly) since 
"delete" is a destructive operation, unlike other cases in which `-` is 
accepted. A relatively recent submission and ensuing discussion is at [1].

[1]: https://lore.kernel.org/git/20200501222227.GE41612@syl.local/T/

Re: [PATCH] branch: delete now accepts '-' as branch name

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On 2/16/22 11:54 AM, Junio C Hamano wrote:
>> From: "Erlend E. Aasland" <erlend.aasland@innova.no>
>>> This makes it easy to get rid of short-lived branches:
>>>
>>> $ git switch -c experiment
>>> $ git switch -
>>> $ git branch -D -
>> Or you can use @{-1} directly.  Or short-lived experiment can
>> directly be done on HEAD without any branch ;-)
>> Thanks for trying to make Git better.
>
> Patches implementing this behavior for `git branch -D` have been
> submitted previously but were rejected (if I recall correctly) since 
> "delete" is a destructive operation, unlike other cases in which `-`
> is accepted. A relatively recent submission and ensuing discussion is
> at [1].
>
> [1]: https://lore.kernel.org/git/20200501222227.GE41612@syl.local/T/

I think the thread around <vpqh944eof7.fsf@anie.imag.fr> is what you
have in mind as the origin of

 * "-" usually stands for "read from standard input" and using it to
   mean "the previous branch" is a source of confusion, and cannot
   be defended with the excuse of "'cd -' is used to go back to
   where we were" when the command is not going back to that branch
   (i.e. not "checkout -" or "switch -").

 * it is doubly dubious to make it easy to type "-" to cause
   something destructive.

 * on the other hand, since "checkout -" already makes "-" as a
   synonym to "@{-1}", not supporting it elsewhere may appear
   inconsistent.

I personally am sympathetic to the first point, but not that
sympathetic to the second one.  We made it worse since then by
accepting "merge -", which is not about going back, though.

Re: [PATCH] branch: delete now accepts '-' as branch name

[Erlend Aasland]

> On 16 Feb 2022, at 20:03, Eric Sunshine <sunshine@sunshineco.com> wrote:
> 
> Patches implementing this behavior for `git branch -D` have been submitted previously but were rejected (if I recall correctly) since "delete" is a destructive operation, unlike other cases in which `-` is accepted. A relatively recent submission and ensuing discussion is at [1].
> 
> [1]: https://lore.kernel.org/git/20200501222227.GE41612@syl.local/T/

Thanks for the heads-up regarding previous discussions, Eric.

AFAIK, switch/checkout, rebase, and merge all use "-" as a shortcut to the
"last branch", so for me, it made sense with similar semantics when deleting a
branch.  I understand the concern regarding "destructive operations", but
deleting a branch is not comparable to, for example, deleting a file.  Deleting
a branch is "soft destructive";  unless *explicitly* silenced, git kindly tells
us which ref we can use to immediately recreate our branch:

$ git branch -D -  # oops, shouldn't have done that!
Deleted branch stuff (was 580cd9ab29).
$ git switch -c stuff 580cd9ab29

Until now, I've just been using the @{-1} notation, but - is very much faster
to type ;)

There are no new arguments pro this feature, so I guess the conclusion from
2020 still stands :)


E

Re: [PATCH] branch: delete now accepts '-' as branch name

[Eric Sunshine]

On Thu, Feb 17, 2022 at 7:51 AM Erlend Aasland <Erlend-A@innova.no> wrote:
> > On 16 Feb 2022, at 20:03, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > Patches implementing this behavior for `git branch -D` have been submitted previously but were rejected (if I recall correctly) since "delete" is a destructive operation, unlike other cases in which `-` is accepted. A relatively recent submission and ensuing discussion is at [1].
> >
> > [1]: https://lore.kernel.org/git/20200501222227.GE41612@syl.local/T/
>
> Thanks for the heads-up regarding previous discussions, Eric.
>
> Until now, I've just been using the @{-1} notation, but - is very much faster
> to type ;)
>
> There are no new arguments pro this feature, so I guess the conclusion from
> 2020 still stands :)

Perhaps. Perhaps not. I may be misreading Junio's responses in this
thread, but it didn't seem like he was necessarily opposed to the
change. A documentation update as in [1] would be a good idea, though,
if resubmitted.

Re: [PATCH] branch: delete now accepts '-' as branch name

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> Perhaps. Perhaps not. I may be misreading Junio's responses in this
> thread, but it didn't seem like he was necessarily opposed to the
> change.

I do not care too much about this issue to expend my political
capital on enforcing my preference ;-)

FWIW, my preference in an ideal world would be to limit "-" as a
short-hand to go back to previous (i.e. "checkout -"), which can be
justified with similarity to "cd -", but do not add any more use.
If we could, I would even deprecate "merge -", "rebase -", etc. that
can not be justified with similarity to "cd -", but I think we came
too far for that.

"-" cannot be used as a universal "the branch we last 'git checkout'
out of" notation because some commands and people expect "-" to be
something else, like "read from the standard input".  The only two
reasons this pops up from time to time is because "checkout -"
exists and because "@{-N}" notation, which is accepted everywhere
uniformly and does not have problems "-" has, is not very well
known.

> A documentation update as in [1] would be a good idea, though,
> if resubmitted.

Yeah, [1] talked about both "@{-1}" and "-", but limiting it to the
former may make sense.  It feels a bit odd that we single out "git
branch" and describe "@{-1}" there, when the notation is universally
available, though.

    $ git grep -l '@{-' -- Documentation/ :\!Documentation/\*/\*

shows hits only in check-ref-format, checkout, switch, and worktree,
but the mention in "revisions.txt" is included in all commands in
the "log" family of commands.  If we add one to "branch", we should
at least teach "@{-1}" to the documentation of merge, rebase, and
revert.  The hits we see here

    $ git grep -l -B1 '"@{-' \*.c
    builtin/checkout.c
    builtin/merge.c
    builtin/rebase.c
    builtin/revert.c
    builtin/worktree.c

all are about replacing "-" the user typed with "@{-1}".

Continuing the "thinking aloud" a bit, I _think_ this tells us these
things:

 * @{-1} has way too many letters to type to be liked by users, who
   won't learn or remember what they do not appreciate (and do not
   blame them---it is a bad notation).

 * @{-<n>} may have been a generalized way that satisfied geeky mind
   while being implemented, but the users only need the "last one"
   and no such generalization.

If it is too late for a more easy-to-type-and-pleasant-to-eyes
notation, perhaps "@-", that does not have downsides of "-" or
"@{-1}", I have to wonder.

Thanks.

Re: [PATCH] branch: delete now accepts '-' as branch name

[Ævar Arnfjörð Bjarmason]

On Thu, Feb 17 2022, Junio C Hamano wrote:

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> Perhaps. Perhaps not. I may be misreading Junio's responses in this
>> thread, but it didn't seem like he was necessarily opposed to the
>> change.
>
> I do not care too much about this issue to expend my political
> capital on enforcing my preference ;-)
>
> FWIW, my preference in an ideal world would be to limit "-" as a
> short-hand to go back to previous (i.e. "checkout -"), which can be
> justified with similarity to "cd -", but do not add any more use.
> If we could, I would even deprecate "merge -", "rebase -", etc. that
> can not be justified with similarity to "cd -", but I think we came
> too far for that.
>
> "-" cannot be used as a universal "the branch we last 'git checkout'
> out of" notation because some commands and people expect "-" to be
> something else, like "read from the standard input".  The only two
> reasons this pops up from time to time is because "checkout -"
> exists and because "@{-N}" notation, which is accepted everywhere
> uniformly and does not have problems "-" has, is not very well
> known.
>
>> A documentation update as in [1] would be a good idea, though,
>> if resubmitted.
>
> Yeah, [1] talked about both "@{-1}" and "-", but limiting it to the
> former may make sense.  It feels a bit odd that we single out "git
> branch" and describe "@{-1}" there, when the notation is universally
> available, though.
>
>     $ git grep -l '@{-' -- Documentation/ :\!Documentation/\*/\*
>
> shows hits only in check-ref-format, checkout, switch, and worktree,
> but the mention in "revisions.txt" is included in all commands in
> the "log" family of commands.  If we add one to "branch", we should
> at least teach "@{-1}" to the documentation of merge, rebase, and
> revert.  The hits we see here
>
>     $ git grep -l -B1 '"@{-' \*.c
>     builtin/checkout.c
>     builtin/merge.c
>     builtin/rebase.c
>     builtin/revert.c
>     builtin/worktree.c
>
> all are about replacing "-" the user typed with "@{-1}".
>
> Continuing the "thinking aloud" a bit, I _think_ this tells us these
> things:
>
>  * @{-1} has way too many letters to type to be liked by users, who
>    won't learn or remember what they do not appreciate (and do not
>    blame them---it is a bad notation).
>
>  * @{-<n>} may have been a generalized way that satisfied geeky mind
>    while being implemented, but the users only need the "last one"
>    and no such generalization.
>
> If it is too late for a more easy-to-type-and-pleasant-to-eyes
> notation, perhaps "@-", that does not have downsides of "-" or
> "@{-1}", I have to wonder.

I too find the syntax really annoying to type.

I wonder if we couldn't say that:

 * @[-]N is the same as @{[-]N}. I.e. @1 is the same as @{1} and @{-1}
   is the same as @-1

 * Optionally (and this is a bit nasty) say that @{-} is a synonym for
   @{-1}, and therefore @- is the same as @-1 is the same as @{-1}.

   Nasty because the logical conclusion would be that @ is the same as
   @1, but it's HEAD, but this would allow us to have a shorter "@-" for
   "delete last", as opposed to "@-1". Also @{-0} (which would presumably
   be a synonym for "HEAD", or "@" errors out currently, and would continue
   to do so).

 * Declare that any other single-letter special @{...} syntax is the
   same as @...; In particular that @u would be @{u} which is short for
   @{upstream}.

 * Live more dangerously and allow @push @upstream etc.? One the one
   hand it feels a bit usurp-y to close the door on such a syntax having
   a similar meaning as regex flags where /ix is /i and /x, but on the
   other hand I don't really see us wanting @pu for "@push" and
   "@upstream" at the same time (makes no sense...).

I haven't hacked it up (and won't any time soon), Erlend: are you
interested? :)

Re: [PATCH] branch: delete now accepts '-' as branch name

[Junio C Hamano]

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

>> Continuing the "thinking aloud" a bit, I _think_ this tells us these
>> things:
>>
>>  * @{-1} has way too many letters to type to be liked by users, who
>>    won't learn or remember what they do not appreciate (and do not
>>    blame them---it is a bad notation).
>>
>>  * @{-<n>} may have been a generalized way that satisfied geeky mind
>>    while being implemented, but the users only need the "last one"
>>    and no such generalization.
>>
>> If it is too late for a more easy-to-type-and-pleasant-to-eyes
>> notation, perhaps "@-", that does not have downsides of "-" or
>> "@{-1}", I have to wonder.
>
> I too find the syntax really annoying to type.
>
> I wonder if we couldn't say that:
> ...

We could, but I do not think I like any of it, except for adding
"@-".  We learned that we do not need @{-4} generalization and
people only want "the last one" with nothing else.  It is repeating
the same mistake without learning any from the lesson to take any
random string that follows @ as if it is inside @{}, I am afraid.

P.S. It seems it is holiday around here and I hear it is at GitHub,
too, so I expect the day to be slow and my presence may be sporadic.

Re: [PATCH] branch: delete now accepts '-' as branch name

[Ævar Arnfjörð Bjarmason]

On Mon, Feb 21 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> Continuing the "thinking aloud" a bit, I _think_ this tells us these
>>> things:
>>>
>>>  * @{-1} has way too many letters to type to be liked by users, who
>>>    won't learn or remember what they do not appreciate (and do not
>>>    blame them---it is a bad notation).
>>>
>>>  * @{-<n>} may have been a generalized way that satisfied geeky mind
>>>    while being implemented, but the users only need the "last one"
>>>    and no such generalization.
>>>
>>> If it is too late for a more easy-to-type-and-pleasant-to-eyes
>>> notation, perhaps "@-", that does not have downsides of "-" or
>>> "@{-1}", I have to wonder.
>>
>> I too find the syntax really annoying to type.
>>
>> I wonder if we couldn't say that:
>> ...
>
> We could, but I do not think I like any of it, except for adding
> "@-".  We learned that we do not need @{-4} generalization and
> people only want "the last one" with nothing else.

FWIW I often use @{-2}, @{-3} or equivalent, but never @{-28} or
whatever.

Not often.

But I would very much appreciate @u if it existed. I use @{u} a lot, and
with the shift-key dancing it's a hassle to typ it.

> the same mistake without learning any from the lesson to take any
> random string that follows @ as if it is inside @{}, I am afraid.

I think we should be careful to squat on namespaces needlessly, but if
we can't think of a reason for why we wouldn't make typing some of these
shortcuts easier...

IOW can we think of a reason we'd ever use @1 @-1 or @u for anything
else? It would be *very* confusing to add a new @{u} that behaved
differently from @u, whatever such a @u would mean.

> P.S. It seems it is holiday around here and I hear it is at GitHub,
> too, so I expect the day to be slow and my presence may be sporadic.

*nod*

Re: [PATCH] branch: delete now accepts '-' as branch name

[Erlend Aasland]

> On 21 Feb 2022, at 20:20, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> 
> On Mon, Feb 21 2022, Junio C Hamano wrote:
> 
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> 
>>>> Continuing the "thinking aloud" a bit, I _think_ this tells us these
>>>> things:
>>>> 
>>>> * @{-1} has way too many letters to type to be liked by users, who
>>>>   won't learn or remember what they do not appreciate (and do not
>>>>   blame them---it is a bad notation).

+1

>>>> * @{-<n>} may have been a generalized way that satisfied geeky mind
>>>>   while being implemented, but the users only need the "last one"
>>>>   and no such generalization.

That harmonises with my usage.

> FWIW I often use @{-2}, @{-3} or equivalent, but never @{-28} or
> whatever.

Ditto; I mostly (at least 95% of the time) only use @{-1}. I've used other
small indices sporadically, but never a two-digit index.

> I think we should be careful to squat on namespaces needlessly, but if
> we can't think of a reason for why we wouldn't make typing some of these
> shortcuts easier...

IMO, it's not a shortcut if it's a hassle to type ;)


IIRC, the GitHub "Git Highlights" Blog has promoted the '-' trick every now and then.


> I haven't hacked it up (and won't any time soon), Erlend: are you interested? :)

There's a lot of suggestions floating around now, but sure, I'd be happy to give one of
them a go; at least as a proof-of-concept :)



Erlend