branch: allow "-" as a short-hand for "previous branch"

From: rjusto <[email protected]>

Align "branch" with the intuitive use of "-" as a short-hand for "@{-1}", like in "checkout" and "merge" commands.

$ git branch -d -      # short-hand for: "git branch -d @{-1}"
$ git branch -D -      # short-hand for: "git branch -D @{-1}"

---
branch: allow "-" as a short-hand for "previous branch"

Align "branch" with the intuitive use of "-" as a short-hand for "@{-1}", like in "checkout" and "merge" commands.

$ git branch -d - # short-hand for: "git branch -d @{-1}" $ git branch -D - # short-hand for: "git branch -D @{-1}"

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1315%2Frjusto%2Fmaster-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1315/rjusto/master-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1315

 builtin/branch.c | 4 ++++
 1 file changed, 4 insertions(+)

index 55cd9a6e998..59c19f38d2e 100644
@@ -241,6 +241,10 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			die(_("Couldn't look up commit object for HEAD"));
 	}
 
+	if ((argc == 1) && !strcmp(argv[0], "-")) {
+		argv[0] = "@{-1}";
+	}
+
 	for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
 		char *target = NULL;
 		int flags = 0;

base-commit: 679aad9e82d0dfd8ef3d1f98fa4629665496cec9

gitgitgadget

Hi Rubén,

A valuable goal!

	git branch -d --remotes -

index 4d2746574cd..ae6c2ed7b83 100644
@@ -1420,6 +1420,12 @@ static int interpret_nth_prior_checkout(struct repository *r,
 	const char *brace;
 	char *num_end;
 
+	if (namelen == 1 && *name == '-') {
+		brace = name;
+		nth = 1;
+		goto find_nth_checkout;
+	}
+
 	if (namelen < 4)
 		return -1;
 	if (name[0] != '@' || name[1] != '{' || name[2] != '-')

@@ -1432,6 +1438,8 @@ static int interpret_nth_prior_checkout(struct repository *r,
 		return -1;
 	if (nth <= 0)
 		return -1;
+
+find_nth_checkout:
 	cb.remaining = nth;
 	cb.sb = buf;
 

Agree. To make a substitution in the command line and to consider "-" in interpret_nth_prior_checkout, I see them as two very different games.

Previous to this, I thought about making also a "git diff -",
https://github.com/gitgitgadget/git/pull/1314 <https://github.com/gitgitgadget/git/pull/1314>. Suddenly there was 5 commands with this substitution (checkout, merge, rebase, branch, diff)
so I follow a little the path now Johannes suggests, making the
substitution "- ~ @{-1}" deep in the system. For me, the implications,
error cases, test cases... to consider was not worth the change to
get what I was looking for: align the workflow "checkout/merge/branch
-d".

Also discarded the "git diff -" change, because of so many flags and conditions "diff" has. So I only sent the "branch -" patch.

Yeah, that is why the approach Johannes showed, i.e. instead of making "in this context but not in that context, '-' means 'previous'" changes to random commands, teaching anybody that takes @{-1} to understand that '-' means 'previous' everywhere, appears very tempting.

Hi Junio,

Indeed, but it does not work as-is: interpret_nth_prior_checkout() is used on prefixes of a rev, and for the special handling of - we cannot have that.

To illustrate what I mean: - should not be idempotent to @{-1} because we want to allow things like @{-1}^2~15 but we do not want -^2~15 to be a synonym for that.

Therefore, the layer where this - handling needs to happen is somewhere above interpret_nth_prior_checkout(), but still well below delete_branches().

I haven't had the time to perform a thorough analysis (and hoped that Rubén would rise up to the challenge), but I have not seen a lot of places where - would be ambiguous, especially when taking into account that revision and file name arguments can be separated via --.

One thing we could do, however, would be to patch only repo_interpret_branch_name(), i.e. only allow - to imply the previous branch name in invocations where a branch name is asked for _explicitly_. I.e. not any random revision, but specifically a branch name.

This would address all of the git branch operations we care about, and leave invocations like git diff - unaddressed (which might be more confusing than we want it to be).

I am not claiming that it will be easy to perform that analysis. It will be worth the effort, though, I am sure.

And it will be necessary because the current approach of special-special-casing git branch -d - is just too narrow, and a recipe for totally valid complaints by users.

Ciao,
Dscho

Hi Junio & Rubén,

This patch does that:

-- snip --

index 4d2746574cd..a2732be3b71 100644
@@ -1616,6 +1616,20 @@ int repo_interpret_branch_name(struct repository *r,
 	if (!namelen)
 		namelen = strlen(name);
 
+	if (namelen == 1 && *name == '-') {
+		struct grab_nth_branch_switch_cbdata cb = {
+			.remaining = 1,
+			.sb = buf
+		};
+
+		if (refs_for_each_reflog_ent_reverse(get_main_ref_store(r),
+						     "HEAD",
+						     grab_nth_branch_switch,
+						     &cb) <= 0)
+			return -1;
+		return namelen;
+	}
+
 	if (!options->allowed || (options->allowed & INTERPRET_BRANCH_LOCAL)) {
 		len = interpret_nth_prior_checkout(r, name, namelen, buf);
 		if (!len) {

index 993a6b5eff7..5acbef95913 100755
@@ -67,6 +67,22 @@ test_expect_success 'delete branch via @{-1}' '
 	expect_deleted previous-del
 '
 
+test_expect_success 'delete branch via -' '
+	git checkout -b previous-del &&
+	git checkout - &&
+
+	git branch -d - &&
+	expect_deleted previous-del &&
+
+	git branch previous-del2 &&
+	git checkout -b previous-del &&
+	git checkout - &&
+
+	git branch -d previous-del2 - &&
+	expect_deleted previous-del &&
+	expect_deleted previous-del2
+'
+
 test_expect_success 'delete branch via local @{upstream}' '
 	git branch local-del &&
 	git branch --set-upstream-to=local-del &&

-- snap --

This adds support for things like git branch -d -, and it even verifies that that works.

What does not work with this patch is git show -. I am not sure whether we want to make that work, although I have to admit that I would use it. Often. And this patch would make it work, the test suite even passes with it:

-- snip --

index f4eee11cc8b..207b554aef1 100644
@@ -2802,7 +2802,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 		revarg_opt |= REVARG_CANNOT_BE_FILENAME;
 	for (left = i = 1; i < argc; i++) {
 		const char *arg = argv[i];
-		if (!seen_end_of_options && *arg == '-') {
+		if (!seen_end_of_options && *arg == '-' && arg[1]) {
 			int opts;
 
 			opts = handle_revision_pseudo_opt(

-- snap --

That latter patch obviously needs at least one accompanying test case, and the patch series would then need to drop the special-casings we already have for "-".

Rubén, do you want to take this a bit further?

Ciao,
Dscho

P.S.: For convenient fetching, I opened a draft PR here:
https://github.com/gitgitgadget/git/pull/1329

Exactly my above point. Nobody including our tests expect that a single '-' to be taken as a rev when we disambiguate command line arguments, so it is very unlikely that it is tested to ensure that a single '-' ends the revs and is checked for its "path-ness". It is not surprising that the tests do not fail ;-).

Hi,

Just wanted to delete the previous branch, I didn't want to enter in a deep change... but here we are :-)

Allow the "-" in setup_revisions:

index f4eee11cc8..65e7eb85d8 100644
@@ -2802,7 +2802,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
                revarg_opt |= REVARG_CANNOT_BE_FILENAME;
        for (left = i = 1; i < argc; i++) {
                const char *arg = argv[i];
-               if (!seen_end_of_options && *arg == '-') {
+               if (!seen_end_of_options && *arg == '-' && !strchr(".^~:@", arg[1])) {
                        int opts;
 

Then, consider "-" as nth_prior, just like @{-1}:

index 4d2746574c..87b4c33cce 100644
@@ -934,6 +934,9 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
                }
        }
 
+        if ((len == 1) && (str[0] == '-'))
+                nth_prior = 1;
+
        /* Accept only unambiguous ref paths. */
        if (len && ambiguous_path(str, len))
                return -1;

index 4d2746574c..87b4c33cce 100644
@@ -1420,18 +1423,24 @@ static int interpret_nth_prior_checkout(struct repository *r,
        const char *brace;
        char *num_end;
 
-       if (namelen < 4)
-               return -1;
-       if (name[0] != '@' || name[1] != '{' || name[2] != '-')
-               return -1;
-       brace = memchr(name, '}', namelen);
-       if (!brace)
-               return -1;
-       nth = strtol(name + 3, &num_end, 10);
-       if (num_end != brace)
-               return -1;
-       if (nth <= 0)
-               return -1;
+        if (name[0] == '-' && strchr(".^~:@", name[1])) {
+                nth = 1;
+                brace = name;
+        } else {
+                if (namelen < 4)
+                        return -1;
+                if (name[0] != '@' || name[1] != '{' || name[2] != '-')
+                        return -1;
+                brace = memchr(name, '}', namelen);
+                if (!brace)
+                        return -1;
+                nth = strtol(name + 3, &num_end, 10);
+                if (num_end != brace)
+                        return -1;
+                if (nth <= 0)
+                        return -1;
+        }
+
        cb.remaining = nth;
 

Two checks needs to be adjusted:

index 90bcb27168..0ed9f99ccc 100644
@@ -198,6 +198,11 @@ static int check_or_sanitize_refname(const char *refname, int flags,
                else
                        return -1;
        }
+
+       if (component_len == 1 && refname[0] == '-') {
+                return -1;
+       }
+
 

index 4d2746574c..87b4c33cce 100644
@@ -1684,7 +1693,7 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
         */
        strbuf_splice(sb, 0, 0, "refs/heads/", 11);
 
-       if (*name == '-' ||
+       if ((*name == '-' && name[1]) ||
            !strcmp(sb->buf, "refs/heads/HEAD"))
                return -1;
 

I know this changes open the possibility of having things like "-^2" or -@{yesterday} that you said was not desiable. But, why wouldn't we want that? Having parse_opt_result to handle that:

index edf55d3ef5..2757bd94c1 100644
@@ -740,7 +740,7 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
                    ctx->argc != ctx->total)
                        break;
 
-               if (*arg != '-' || !arg[1]) {
+               if (*arg != '-' || strchr(".^~:@", arg[1])) {
                        if (parse_nodash_opt(ctx, arg, options) == 0)
                                continue;
                        if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)

With this changes, all the current uses of "-", with the hacks already removed, keep working and fixes the weird cases:

$ git merge branch - other_branch
$ git branch -d branch - other_branch

Also, I've checked that work:

$ git diff -
$ git show -
$ git blame -
and branch -d :-)

I've checked the current tests and added new ones for this, all passes. ie:

index 4a5758f08a..231457df50 100755
@@ -40,10 +40,18 @@ test_expect_success '@{-1} works' '
        test_cmp_rev side @{-1}
 '
 
+test_expect_success '- works' '
+       test_cmp_rev side -
+'
+
 test_expect_success '@{-1}~2 works' '
        test_cmp_rev side~2 @{-1}~2
 '
 
+test_expect_success '-~2 works' '
+       test_cmp_rev side~2 -~2
+'
+
 test_expect_success '@{-1}^2 works' '
        test_cmp_rev side^2 @{-1}^2
 '

Still needs some work, for example: git log shows "-" in the warning:

$ ../git/git log "-@{10000 minutes ago}"
warning: log for '-' only goes back to Wed, 24 Aug 2022 14:16:17 +0200

What do you think, is it worth the change?

I've created a PR with all the changes and tests.
https://github.com/gitgitgadget/git/pull/1338

Thanks.

Yuck. I really didn't want to see that strchr() or any other logic that _knows_ what letter is allowed after a "rev". It will be impossible to keep it in sync with the real code paths that needs to know and parses these syntactical constructs, and folks new to the codebase will never be able to tell at a first glance if the above is sufficient (or what the strchr() is doing).

Some logic is needed to disambiguate from options. It is difficult than that set of chars changes, they are all around the code. And if any new is added should be reviewed and hopefully some test will be broken.

Maybe a more centralized approach?

index 2757bd94c1..303854e8a4 100644
@@ -740,7 +741,7 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
                    ctx->argc != ctx->total)
                        break;
 
-               if (*arg != '-' || strchr(".^~:@", arg[1])) {
+               if (*arg != '-' || !check_refchar_component_special(arg[1])) {
                        if (parse_nodash_opt(ctx, arg, options) == 0)
                                continue;
                        if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)

index 0ed9f99ccc..5327a8ec1f 100644
@@ -159,6 +159,32 @@ static int check_refname_component(const char *refname, int *flags,
        return cp - refname;
 }
 
+int check_refchar_component_special(char refchar)
+{
+        int ch = refchar & 255;
+        unsigned char disp = refname_disposition[ch];
+
+        switch (disp) {
+        case 1:
+                /* end of component */
+                return 0;
+        case 2:
+                /* ".." components */
+                return 0;
+        case 3:
+                /* "@{" components */
+                return 0;
+        case 4:
+                /* forbidden char */
+                return 0;
+        case 5:
+                /* pattern */
+                return 0;
+        }
+
+        return -1;
+}
+
 static int check_or_sanitize_refname(const char *refname, int flags,
                                     struct strbuf *sanitized)
 {

Hi Johannes,

Thanks for the /allow!

I did it on purpose, to be interpreted in the context of "git branch -d/D" with just one branch: "previous branch". I agree the commit message does not suggest this, I can fix it.

My intention is a short-hand for "delete previous branch", the same way "git merge -" is "merge previous branch".

The workflow to address is just allow doing:

$ git checkout work_to_review
$ git checkout -
$ git merge -
$ git branch -d -

Instead of:

$ git checkout work_to_review
$ git checkout -
$ git merge -
$ git branch -d work_to_review

The syntax @{-1} is hard for me to write, and I feel intuitive "-", like in "cd -".

Make sense to me...

The "-d" and "-D" options being the more detructive ones among other operation modes of the command, I am not sure if this change is even desirable. Even if it were, the implementation to special case a single argument case like this ...

 $ git branch -m - new-name
 $ git branch new-branch -
 $ git branch --set-upstream-to=<upstream> -

Agree. But the approach is to ease the deletion of previous branch, aligned with merge:

$ git merge - -
merge: - - not something we can merge
$ git merge - old-branch
merge: - - not something we can merge

In fact, I think it is a bit confuse to allow use it that way, and probably induces to error.

Haven't think about -m, -c. If you think it is a good addition, I can do it.

I can fix the braces around that single statement block, sorry.

Hi Rubén,

This is confusing me: how is the patch supporting git branch -d - aligned with the presented git merge invocations?

In any case, you now have two sets of feedback that say that special-casing one particular command-line and leaving all other invocations using - unchanged is undesirable, if you needed more than one such feedback.

Ciao,
Dscho

Hi Johannes,

"merge" supports multiple objects to be specified, but "-" only is accepted if just one argument is specified, as Junio did it in:

commit 4e8115fff135a09f75020083f51722e7e35eb6e9

Author: Junio C Hamano <[email protected]>
Date:   Thu Apr 7 15:57:57 2011 -0700

merge: allow "-" as a short-hand for "previous branch"

Just like "git checkout -" is a short-hand for "git checkout @{-1}" to conveniently switch back to the previous branch, "git merge -" is a short-hand for "git merge @{-1}" to conveniently merge the previous
branch.

It will allow me to say:

$ git checkout -b au/topic
$ git am -s ./+au-topic.mbox
$ git checkout pu
$ git merge -

which is an extremely typical and repetitive operation during my
git day.

index d54e7ddbb1..0bdd19a137 100644
@@ -1062,9 +1062,12 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
        if (!allow_fast_forward && fast_forward_only)
                die(_("You cannot combine --no-ff with --ff-only."));
 
-       if (!argc && !abort_current_merge && default_to_upstream)
-               argc = setup_with_upstream(&argv);
-
+       if (!abort_current_merge) {
+               if (!argc && default_to_upstream)
+                       argc = setup_with_upstream(&argv);
+               else if (argc == 1 && !strcmp(argv[0], "-"))
+                       argv[0] = "@{-1}";
+       }
        if (!argc)
                usage_with_options(builtin_merge_usage,
                        builtin_merge_options);

So I aligned "branch -d" (or "delete-branch") with that.

The other two commands that already support "-", also works the same way:

$ git checkout -B - default
fatal: '-' is not a valid branch name

$ git rebase default -
fatal: no such branch/commit '-'

To summarize, my goal is to allow:

$ git checkout work_to_review
$ git checkout -
$ git merge - # or git rebase -
$ git branch -d -

Makes sense to me...

I've updated the commit message with a more specific message and removed the braces, jic.

Hi Rubén,

Ah, the vagaries of being a maintainer and everybody following your lead, even if you have a bad day and are grumpy, or as in this case just want to get a quick fix in that supports your workflow better, and then move on.

If you read the commit message carefully, you will note that there is no justification for restricting it to the argc == 1 case.

I assume that the implicit rationale is that it was just simpler to do it this way.

The alternative would have been to modify collect_parents(), or even get_merge_parent() (which has many more callers), and at some stage the investigation would have been as involved as it will be in this here thread.

However, it is one thing to integrate such a patch as a one-off, or do it two times, or three.

It is another thing to do this again and again and again and seeing that we're not getting anywhere and only piling hack upon hack.

It is this latter stage that we have arrived at.

There are different qualities at play with these commands, though. git checkout cannot support more than a single revision argument. With git merge, technically we do support more than a single revision argument (via octopus merges), but support for it is limited (for example, we do not even support recursive octopus merges). You might say that it is discouraged to call git merge with more than one revision argument.

With git branch -d or with git branch --list, we are in a different league. Those commands are commonly called with more than just a single branch name.

And then there are the other commands that would benefit from support for - and that accept many more than one revision argument, too, such as log, rev-parse, merge-base, etc.

Sure, we can accept one more one-off hack to support a single - argument to refer to the previous branch. The sum of those hacks, however, becomes a burden.

Ciao,
Dscho

From: rjusto <[email protected]>

Align "branch -d" with the intuitive use of "-" as a short-hand for "@{-1}", like in "checkout", "rebase" and "merge" commands.

$ git branch -d -      # short-hand for: "git branch -d @{-1}"
$ git branch -D -      # short-hand for: "git branch -D @{-1}"

So I can do:

$ git checkout work_to_review
$ git checkout -
$ git merge - # or git rebase -
$ git branch -d -

---
branch: allow "-" as a short-hand for "previous branch"

Align "branch" with the intuitive use of "-" as a short-hand for "@{-1}", like in "checkout" and "merge" commands.

$ git branch -d - # short-hand for: "git branch -d @{-1}" $ git branch -D - # short-hand for: "git branch -D @{-1}"

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1315%2Frjusto%2Fmaster-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1315/rjusto/master-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1315

Range-diff vs v1:

1: 3ba75097ea0 ! 1: 0fe48ada15b branch: allow "-" as a short-hand for "previous branch"
@@ Metadata
Author: rjusto <[email protected]>

## Commit message ##
- branch: allow "-" as a short-hand for "previous branch"
+ allow "-" as short-hand for "@{-1}" in "branch -d"

- Align "branch" with the intuitive use of "-" as a short-hand
- for "@{-1}", like in "checkout" and "merge" commands.
+ Align "branch -d" with the intuitive use of "-" as a short-hand
+ for "@{-1}", like in "checkout", "rebase" and "merge" commands.

$ git branch -d - # short-hand for: "git branch -d @{-1}"
$ git branch -D - # short-hand for: "git branch -D @{-1}"

+ So I can do:
+
+ $ git checkout work_to_review
+ $ git checkout -
+ $ git merge - # or git rebase -
+ $ git branch -d -
+

## builtin/branch.c ##
@@ builtin/branch.c: static int delete_branches(int argc, const char **argv, int fo
die(_("Couldn't look up commit object for HEAD"));
}

-+ if ((argc == 1) && !strcmp(argv[0], "-")) {
++ if ((argc == 1) && !strcmp(argv[0], "-"))
+ argv[0] = "@{-1}";
-+ }
+
for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
char *target = NULL;

 builtin/branch.c | 3 +++
 1 file changed, 3 insertions(+)

index 55cd9a6e998..7f7589bd4a8 100644
@@ -241,6 +241,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			die(_("Couldn't look up commit object for HEAD"));
 	}
 
+	if ((argc == 1) && !strcmp(argv[0], "-"))
+		argv[0] = "@{-1}";
+
 	for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
 		char *target = NULL;
 		int flags = 0;

base-commit: 679aad9e82d0dfd8ef3d1f98fa4629665496cec9

gitgitgadget

Documentation/SubmittingPatches:

Also notice that a real name is used in the Signed-off-by trailer. Please don't hide your real name.

The "branch:" prefix is lost here, which is not an improvement. The "<area>:" prefix is what makes it easier to locate a particular change in "git shortlog --no-merges v2.37.0..v2.38.0".

As to the implementation, there is nothing to complain about, but as we already discussed during the reivew of the first iteration, I am not sure if the goal is sound in the first place.

Thanks.

Fixed.

Just want to reduce the confusion, as Johannes suggested, that could apply to the whole command. But ok, I've put it back.

Sorry, I don't want to be annoying, I just think the effort is worth of it.

The change rounds the workflow: "checkout/merge/branch -d" and does not introduce any new confusion or new circumstance. The commands that already support the substitution doesn't support it in all combinations, complex use cases can (probably, should) use the @{-1} syntax and definitely those uses are not the candidates to use the short-hands.

Thank you.

Rubén.

From: Rubén Justo <[email protected]>

Align "branch -d" with the intuitive use of "-" as a short-hand for "@{-1}", like in "checkout", "rebase" and "merge" commands.

$ git branch -d -      # short-hand for: "git branch -d @{-1}"
$ git branch -D -      # short-hand for: "git branch -D @{-1}"

So I can do:

$ git checkout work_to_review
$ git checkout -
$ git merge - # or git rebase -
$ git branch -d -

---
branch: allow "-" as a short-hand for "previous branch"

Align "branch -d" with the intuitive use of "-" as a short-hand for "@{-1}", like in "checkout", "rebase" and "merge" commands.

$ git branch -d - # short-hand for: "git branch -d @{-1}" $ git branch -D - # short-hand for: "git branch -D @{-1}"

So I can do:

$ git checkout work_to_review $ git checkout - $ git merge - # or git rebase - $ git branch -d -

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1315%2Frjusto%2Fmaster-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1315/rjusto/master-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1315

Range-diff vs v2:

1: 0fe48ada15b ! 1: cc28546aea9 allow "-" as short-hand for "@{-1}"
in "branch -d"
@@
## Metadata ##
-Author: rjusto <[email protected]>
+Author: Rubén Justo <[email protected]>

## Commit message ##
- allow "-" as short-hand for "@{-1}" in "branch -d"
+ branch: allow "-" as short-hand for "@{-1}" in "branch -d"

Align "branch -d" with the intuitive use of "-" as a short-hand for "@{-1}", like in "checkout", "rebase" and "merge" commands.
@@ Commit message
$ git merge - # or git rebase -
$ git branch -d -

- Signed-off-by: rjusto <[email protected]>
+ Signed-off-by: Rubén Justo <[email protected]>

## builtin/branch.c ##
@@ builtin/branch.c: static int delete_branches(int argc, const
char **argv, int force, int kinds,

 builtin/branch.c | 3 +++
 1 file changed, 3 insertions(+)

index 55cd9a6e998..7f7589bd4a8 100644
@@ -241,6 +241,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			die(_("Couldn't look up commit object for HEAD"));
 	}
 
+	if ((argc == 1) && !strcmp(argv[0], "-"))
+		argv[0] = "@{-1}";
+
 	for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
 		char *target = NULL;
 		int flags = 0;

base-commit: 679aad9e82d0dfd8ef3d1f98fa4629665496cec9

2.36.1

Teach "git branch" the use of "-" as a shortcut for "@{-1}", like in "checkout -", "merge -" and other commands.

Usage examples:

	$ git branch -d -              # git branch -d @{-1}
	$ git branch -M some-branch -  # git branch -M some-branch @{-1}
	$ git branch -c - new-branch   # git branch -c @{-1} new-branch

Possible workflow:

	$ git checkout work_to_review
	$ git checkout -
	$ git merge -  # or git rebase -
	$ git branch -d -

This change will be unnecessary if "-" ends up being supported globally as a shortcut for "previous branch". The usage examples and the tests will remain valid.

---

Let's recap this.

We have discussed two topics, related but somewhat different:
1.- teach "git branch" to support "-" as a shortcut of "previous-branch" 2.- teach "git" to support "-" as a shortcut of "previous-branch" _anywhere a
branch is expected_

I'll refer to the former as "local support" and the latter as "global support".

Teach "git branch -d" about "-" as a shortcut of "previous-branch" is the main intention of the patch that originates this thread.

Teach "git" about the shortcut makes unnecessary, at first, the change in "git
branch". It makes "-" work in "git branch" and everywhere else as well. And
this is what makes tempting to skip the "local support" and go for the "global support".

Attempts of "global support" can be tracked back to Jan 2009 [2], just days after the syntax "@{-1}" was presented [1] or the "git checkout -" (the real originator of all of this) patch was committed. Each of this attempts has received some concerns and not many reviews and/or iterations to allow the curation and integration of the change. Meanwhile, since "checkout -" was
introduced: "merge -", "cherry-pick -", "rebase -" and "worktree -", have gained support for "-" on its own, via a local: if (!strcmp(argv[i], "-")) argv[i] = "@{-1}".

Since 2016 there have been, at least, five patch series (see below) to make a similar (some equal) change in "git branch -d". The motivations suggested each time have been the same: fulfill the intuitive use of "-" when deleting branches, following the path of, initially, "cd -" and then "checkout -", "merge -",.. not any other function of "git branch". As well as the attempts for the "global support", each of this attempts has also received some concerns, reviews and done some iterations.

Let's have a look at the reviews and/or concerns about the "local support" first, and then the ones about the "global support".

Reviews and/or concerns about "git branch -d -":

* /if (!strcmp(argv[i], "-")) argv[i] = "@{-1}/ is somewhat of a hack

https://lore.kernel.org/git/[email protected]/

Do we want a more elaborated way of the hack? ie:

+ const char* handle_dash_alias_argv(const char* arg)
+ {
+ if (arg && !strcmp(argv[i], "-"))
+ return "@{-1}";
+
+ return arg;
+ }
...
- if (!strcmp(arg, "-"))
- arg = "@{-1}";
+ arg = handle_dash_alias_argv(arg);
...
- } else if (argc == 1 && !strcmp(argv[0], "-")) {
- argv[0] = "@{-1}";
+ } else if (argc == 1) {
+ argv[0] = handle_dash_alias(argv[0]);

Or, do we want to keep it simple and expect that the "global support" will allow this hacks vanish easily?

The "global support" resolves automatically this. Will make all of this hacks unnecessary. So, isn't this more of an argument to defend the work in the "global support" than a concern for this patch?

* The patch only considers "git branch -d/-D", not any other options like
* "-m", "-c", "-u", "--unset-upstream", "--edit-description"

First things first. This review has already been productive: some of those uncovered options by the patch weren't even supporting "@{-N}", which is documented to be supported everywhere. So, thank you. A patch to fix this has already been submitted [E].

On the other hand, an argument can be used here: the operations that currently support the shortcut (ie: merge, rebase, workdir...) do not do it in all of its capabilities, and this can considered a good thing. Allowing the previously commented hack to stay simple by not covering the corner-cases no one has needed yet (maybe never will). Eventually, if the "global support" is achieved, there would be no need to do anything to complete that currently missing support. Will be completed automatically.

* "git branch -d -" is a destructive operation unlike the other operations
* currently supporting the "-" alias (ie: checkout, merge,...)

There is a reasonable twist for this argument: is this a concern for this patch or a spark to think about the destructive aspects of "git branch -d"? Maybe the reflog can be gc'ed and not deleted [F]? Better or new advices [7]?
...

Also an interesting argument to consider:

https://lore.kernel.org/git/[email protected]/

More about this, below in the concerns about "global support" where isolated "-" in the command line are described.

* Confusing error message: "error: branch "@{-1}" not found." received, but
* "-" was specified.

This is a legit concern and can be argued that it is already happening in the commands that already support the shortcut:

       $ git merge -
       merge: @{-1} - not something we can merge

This can be reasoned because solving it will require /dig deeper/ in the hack discussed above and shouldn't be creating a lot of confusion.

Current WIP patches for "global support" show that this problem will vanish along with the hacks. So, again, this becomes more of an argument for having "global support" than a complain about this patch. Thought in this case, it is not just the _burden in the code_ but the experience of the user, which has repercussion so it requires special consideration.

Reviews and/or concerns about global support of the shortcut:

* '-' is also known as a synonym for stdin

https://lore.kernel.org/git/[email protected]/

* Isolated "-" is a risk when misspelling command lines

https://lore.kernel.org/git/[email protected]/

* "-" must be a synonym of "@{-1}", but not making "-^2~15" a synonym of "@{-1}^2~15"

https://lore.kernel.org/git/[email protected]/

I cannot find any argument for this. "-" should or shouldn't be idempotent to "@{-1}", for me it needs more thought and exploration. But an agreement to left it restricted but open to consideration sounds plausible.

After all of this, if you are still here, sounds reasonable to say:
- there is no clear concern or review that justifies why "git branch" hasn't been taught to support the "-" shortcut
- "global support" of "-" as a shortcut for "@{-1}" or "previous-branch" deserves some work

This is the v4 of the patch to support the shortcut "-" in "branch -d". The changes are:
- Handle "-" when deleting multiple branches
- Add support for -m/-c.
- New tests
- Commit message, to reflect changes and include names of people who previously worked on this.

The switches "--edit-description", "--set-upstream-to" and "--unset-upstream" can adopt the hack, if necessary, whenever the patch [E] that fixes supporting "@{-1}" is merged. It is not necessary, and it is better this way, to link the fix with the support of "-", which is not clear if it will be.

I won't send a v5, promise :-)... but an RFC v1 for the "global support"?:

    diff --git a/setup.c b/setup.c
    index cefd5f63c4..de86bb2d98 100644
    --- a/setup.c
    +++ b/setup.c
    @@ -266,8 +266,6 @@ void verify_filename(const char *prefix,
                         const char *arg,
    ?                    int diagnose_misspelt_rev)
     {
    ?       if (*arg == '-')
    ?               die(_("option '%s' must come before non-option arguments"), arg);
            if (looks_like_pathspec(arg) || check_filename(prefix, arg))

Thank you all for making this not only useful but also interesting and enriching.

Pd. A chronology:

15 Jan 2009 - [1] - Patch to introduce "checkout '-'" that itself

                         introduces the notation @{-N}

17 Jan 2009 - ae5a6c36 - checkout: implement "@{-N}" shortcut name for N-th
last branch

17 Jan 2009 - 696acf45 - checkout: implement "-" abbreviation, add docs and
tests

13 Feb 2009 - 8415d5c7 - Teach the "@{-1} syntax to "git branch"

13 Feb 2009 - c9717ee9 - Teach @{-1} to git merge

21 Mar 2009 - [2] - 1st attempt to have "-" as an universal alias of

                         @{-1}

7 Apr 2011 - 4e8115ff - merge: allow "-" as a short-hand for "previous
branch"

5 Sep 2013 - 182d7dc4 - cherry-pick: allow "-" as abbreviation of '@{-1}'

19 Jul 2013 - [3] - [RFC] Delete current branch

19 Mar 2014 - 4f407407 - rebase: allow "-" short-hand for the previous branch

10 Mar 2015 - [4] - [JFF] "-" and "@{-1}" on various program

16 Mar 2015 - [5] - [PATCH/RFC 0/2][GSoC] revision.c: Allow "-" as
stand-in for "@{-1}" everywhere a branch is allowed

5 Mar 2016 - [6] - [PATCH] branch.c: Allow "-" as a short-hand for

                         "@{-1}" in "git branch -d @{-1}"

18 Mar 2016 - [7] - [PATCH][Outreachy] branch: allow - as abbreviation of

                         '@{-1}'

27 May 2016 - 1a450e2f - worktree: allow "-" short-hand for @{-1} in add
command

5 Feb 2017 - [8] - [PATCH/RFC] WIP: log: allow "-" as a short-hand for
"previous branch"

8 Mar 2017 - [9] - [PATCH] diff: allow "-" as a short-hand for "last
branch"

21 Mar 2018 - [A] - [PATCH] branch: implement shortcut to delete last
branch

29 Abr 2020 - [B] - [PATCH] branch: add '-' to delete previous branch

16 Feb 2022 - [C] - [PATCH] branch: delete now accepts '-' as branch name

8 Ago 2022 - [D] - [PATCH] branch: allow "-" as a short-hand for
"previous branch"

[1] [email protected]
[2] [email protected]
[3] CALkWK0=8q4J2yi2to_+41kJSA5E59CBwkG69Hj7MmTPgUnSh5Q@mail.gmail.com
[4] [email protected]
[5] [email protected]
[6] [email protected]
[7] [email protected]
[8] [email protected]
[9] [email protected]
[A] [email protected]
[B] [email protected]
[C] [email protected]
[D] [email protected]
[E] [email protected]
[F] [email protected]

 builtin/branch.c                      | 11 +++++++++++
 t/t3204-branch-name-interpretation.sh | 19 ++++++++++++++-----
 2 files changed, 25 insertions(+), 5 deletions(-)

index 55cd9a6e99..66503d18d1 100644
@@ -245,6 +245,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 		char *target = NULL;
 		int flags = 0;
 
+		if (argv[i] && !strcmp(argv[i], "-"))
+			argv[i] = "@{-1}";
+
 		strbuf_branchname(&bname, argv[i], allowed_interpret);
 		free(name);
 		name = mkpathdup(fmt, bname.buf);

@@ -527,6 +530,14 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 			die(_("cannot rename the current branch while not on any."));
 	}
 
+	if (oldname && !strcmp(oldname, "-")) {
+		oldname = "@{-1}";
+	}
+
+	if (newname && !strcmp(newname, "-")) {
+		newname = "@{-1}";
+	}
+
 	if (strbuf_check_branch_ref(&oldref, oldname)) {
 		/*
 		 * Bad name --- this could be an attempt to rename a

index 993a6b5eff..84b646e1f7 100755
@@ -57,14 +57,15 @@ test_expect_success 'create branch with pseudo-qualified name' '
 	expect_branch refs/heads/refs/heads/qualified two
 '
 
-test_expect_success 'delete branch via @{-1}' '
-	git branch previous-del &&
+test_expect_success 'delete branch via @{-N} and -' '
+	git checkout -b previous-one-del &&
+	git checkout -b previous-two-del &&
 
-	git checkout previous-del &&
 	git checkout main &&
 
-	git branch -D @{-1} &&
-	expect_deleted previous-del
+	git branch -D - @{-2} &&
+	expect_deleted previous-one-del &&
+	expect_deleted previous-two-del
 '
 
 test_expect_success 'delete branch via local @{upstream}' '

@@ -133,4 +134,12 @@ test_expect_success 'checkout does not treat remote @{upstream} as a branch' '
 	expect_branch HEAD one
 '
 
+test_expect_success 'rename and copy branches via -' '
+	git checkout -b one-branch &&
+	git checkout - &&
+	git branch -c - two-branch &&
+	git branch -m - three-branch &&
+	git branch -m three-branch -
+'
+
 test_done

2.36.1

I am sorry that this is now at v4, but let's not do this.

"git checkout -" was started purely because "cd -" is a fairly well known way to say "go back to the previous directory" (it is "cd ~-" in some shells).

No shells accept "mv - newname" to rename the directory we were previously in, or "rmdir -" to remove it. And "diff -r - ." does not compare the previous and the current directory recursively. But all of these can be done (with some shells) if you use a proper syntax, "mv ~- newname", "rmdir ~-", or "diff -r ~- .".

Yes, we may have by mistake added "git merge -" as well, but it is perfectly fine to say that we should admit it as a mistake and plan to possibly deprecate it in the longer term. We shouldn't use it as an excuse to make things even more confusing.

Thanks. I think your other "git branch -d @{-1}" thing is sound, though.

Fair enough.

Just to be sure. I'm assuming you're referring to the patch for the other branch options: "-edit-description", "-set-upstream-to" and "-unset-upstream". Routing their arguments through strbuf_branchname to allow the expansion of @{-N}, @{u}... Option "-d" is fine with this. Otherwise please let me know.

Rubén.