Re: [PATCH] branch: allow "-" as a short-hand for "previous branch"
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Aug 19, 2022, 11:42 UTC
- Message-ID
- <7s9s8p38-r22n-opnn-9219-0p49onrro70s@tzk.qr>
- In-Reply-To
- <986564d9-4d49-b1ef-e2e0-65d6f67f9e79@gmail.com>
Hi Rubén,
On Tue, 16 Aug 2022, Rubén Justo wrote:
Show 54 quoted lines
> On 8/16/22 11:31 AM, Johannes Schindelin wrote:
>
> > > $ git merge - old-branch
> > > merge: - - not something we can merge
> >
> > This is confusing me: how is the patch supporting `git branch -d -`
> > aligned with the presented `git merge` invocations?
>
> "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 <gitster@pobox.com>
> 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.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index d54e7ddbb1..0bdd19a137 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -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);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.
Show 18 quoted lines
> 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...
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