[PATCH] Allow "-" as a short-hand for "@{-1}" in branch deletions

[PATCH] Allow "-" as a short-hand for "@{-1}" in branch deletions

[Shuyang Shi]

The "-" shorthand that stands for "the branch we were previously on",
like we did for "git merge -" sometime after we introduced "git checkout -".
Now I am introducing this shorthand to branch delete, i.e.
"git branch -d -".

More reference:
  https://public-inbox.org/git/7vppuewl6h.fsf@alter.siamese.dyndns.org/

And this has been tested:

	Ivan:git Ivan$ (cd t; prove --timer --jobs 1 ./t3200-branch.sh)
	[00:21:26] ./t3200-branch.sh .. ok    12293 ms ( 0.04 usr  0.01 sys +
	5.97 cusr  2.52 csys =  8.54 CPU)
	[00:21:39]
	All tests successful.
	Files=1, Tests=113, 13 wallclock secs ( 0.07 usr  0.02 sys +
	5.97 cusr  2.52 csys =  8.58 CPU)
	Result: PASS

Signed-off-by: Shuyang Shi <shuyang790@gmail.com>
---
 builtin/branch.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 94f7de7f..1b72d80 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -215,8 +215,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 	for (i = 0; i < argc; i++, strbuf_release(&bname)) {
 		char *target = NULL;
 		int flags = 0;
+		const char *arg = argv[i];
 
-		strbuf_branchname(&bname, argv[i]);
+		if (!strcmp(arg, "-"))
+			arg = "@{-1}";
+
+		strbuf_branchname(&bname, arg);
 		free(name);
 		name = mkpathdup(fmt, bname.buf);
 

--
https://github.com/git/git/pull/337

[PATCH GSoC] Allow "-" as a short-hand for "@{-1}" in branch deletions

[Shuyang Shi]

The "-" shorthand that stands for "the branch we were previously on",
like we did for "git merge -" sometime after we introduced "git checkout -".
Now I am introducing this shorthand to branch delete, i.e.
"git branch -d -".

More reference:
  https://public-inbox.org/git/7vppuewl6h.fsf@alter.siamese.dyndns.org/

And this has been tested:

	Ivan:git Ivan$ (cd t; prove --timer --jobs 1 ./t3200-branch.sh)
	[00:21:26] ./t3200-branch.sh .. ok    12293 ms ( 0.04 usr  0.01 sys +
	5.97 cusr  2.52 csys =  8.54 CPU)
	[00:21:39]
	All tests successful.
	Files=1, Tests=113, 13 wallclock secs ( 0.07 usr  0.02 sys +
	5.97 cusr  2.52 csys =  8.58 CPU)
	Result: PASS

Signed-off-by: Shuyang Shi <shuyang790@gmail.com>
---
 builtin/branch.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 94f7de7f..1b72d80 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -215,8 +215,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 	for (i = 0; i < argc; i++, strbuf_release(&bname)) {
 		char *target = NULL;
 		int flags = 0;
+		const char *arg = argv[i];
 
-		strbuf_branchname(&bname, argv[i]);
+		if (!strcmp(arg, "-"))
+			arg = "@{-1}";
+
+		strbuf_branchname(&bname, arg);
 		free(name);
 		name = mkpathdup(fmt, bname.buf);
 

--
https://github.com/git/git/pull/337

Re: [PATCH GSoC] Allow "-" as a short-hand for "@{-1}" in branch deletions

[Stefan Beller]

Welcome to the Git community!

On Wed, Mar 8, 2017 at 7:31 PM, Shuyang Shi <shuyang790@gmail.com> wrote:
> The "-" shorthand that stands for "the branch we were previously on",
> like we did for "git merge -" sometime after we introduced "git checkout -".
> Now I am introducing this shorthand to branch delete, i.e.
> "git branch -d -".
>
> More reference:
>   https://public-inbox.org/git/7vppuewl6h.fsf@alter.siamese.dyndns.org/

Following that link:

> But there is a very commonly accepted long tradition for "-" to mean
> "read from the standard input", so we cannot reuse it to mean "the
> branch I was previously on" for every command without first making
> sure the command will never want to use "-" for the other common
> purpose.

This contradicts the introduction of "git branch -d -" to mean to delete
the last branch, but rather could mean "read from stdin which branches
to delete"? It would be nice if you could clarify in your commit message
which of both this is and how this fits into the big picture of "design
cleanliness".

>
> And this has been tested:
>
>         Ivan:git Ivan$ (cd t; prove --timer --jobs 1 ./t3200-branch.sh)
>         [00:21:26] ./t3200-branch.sh .. ok    12293 ms ( 0.04 usr  0.01 sys +
>         5.97 cusr  2.52 csys =  8.54 CPU)
>         [00:21:39]
>         All tests successful.
>         Files=1, Tests=113, 13 wallclock secs ( 0.07 usr  0.02 sys +
>         5.97 cusr  2.52 csys =  8.58 CPU)
>         Result: PASS

Thanks for being cautious when developing on Git. However this part
of the email would end up as part of the commit message. And as we expect
all commits that land eventually to not break tests, this information is better
put at a more non-permanent place, such as below the '---' line (where there is
also the built stat. For example see [1] how to have different message parts
(one permanent section and some chatter that is relevant for the process
at the moment)

Also for testing, the tests only ensure that the old behavior does not break;
but we'd want to make sure the new functionality doesn't break in the
future either,
which can be done best by writing a test as well for this functionality.

[1] https://public-inbox.org/git/xmqqvarj1kix.fsf_-_@gitster.mtv.corp.google.com/
and as a commit:
https://github.com/gitster/git/commit/83218867fbf6d27c78efe3cfba01790b2f1d15d4

> https://github.com/git/git/pull/337

Oh I see, you're using submitgit to communicate the patch to the mailing list.
I am not sure if it supports splitting up the message as I eluded to above.
IIRC some people use submitgit for the patch and then use a webmailer
(e.g. gmail) to send followup messages such as successful tests or what changed
to prior versions.

Thanks,
Stefan

Re: [PATCH GSoC] Allow "-" as a short-hand for "@{-1}" in branch deletions

[Siddharth Kannan]

Hey Shuyang,
On Thu, Mar 09, 2017 at 09:47:12AM -0800, Stefan Beller wrote:
> > The "-" shorthand that stands for "the branch we were previously on",
> > like we did for "git merge -" sometime after we introduced "git checkout -".
> > Now I am introducing this shorthand to branch delete, i.e.
> > "git branch -d -".
> >
> > More reference:
> >   https://public-inbox.org/git/7vppuewl6h.fsf@alter.siamese.dyndns.org/
> 

1. I have already worked on this project, and my patch is in the
"Needs review" section in "What's cooking". It implements this change
inside sha1_name.c and doesn't touch git branch. So, your patch is
mutually exclusive to my previous patch.

2. Matthieu made an argument against enabling commands like "git
branch -D -" even by mistake [1]. The way that I have implemented
ensured that not a lot of "rm"-like commands were enabled.

My patch that would enable this shorthand for other projects is
here[2].

[1]: http://public-inbox.org/git/vpqh944eof7.fsf@anie.imag.fr/
[2]: http://public-inbox.org/git/1488007487-12965-5-git-send-email-kannan.siddharth12@gmail.com/

Thanks,
Siddharth.

Re: [PATCH GSoC] Allow "-" as a short-hand for "@{-1}" in branch deletions

[Shuyang Shi]

Hey Siddharth,

It is good to know that you have been working on this, and sorry for
this "bump".
Although I see "git branch -d -" not so dangerous, it is okay for me that we
keep it out of the shorthand range.

And your work is awesome.

Thanks,
Shuyang
史舒扬 Shuyang Shi
Undergraduate
Department of CS, School of EECS, Peking University
Email: shuyang790@gmail.com
Mobile: +86-18301336991


On Fri, Mar 10, 2017 at 11:30 AM, Siddharth Kannan
<kannan.siddharth12@gmail.com> wrote:
> Hey Shuyang,
> On Thu, Mar 09, 2017 at 09:47:12AM -0800, Stefan Beller wrote:
>> > The "-" shorthand that stands for "the branch we were previously on",
>> > like we did for "git merge -" sometime after we introduced "git checkout -".
>> > Now I am introducing this shorthand to branch delete, i.e.
>> > "git branch -d -".
>> >
>> > More reference:
>> >   https://public-inbox.org/git/7vppuewl6h.fsf@alter.siamese.dyndns.org/
>>
>
> 1. I have already worked on this project, and my patch is in the
> "Needs review" section in "What's cooking". It implements this change
> inside sha1_name.c and doesn't touch git branch. So, your patch is
> mutually exclusive to my previous patch.
>
> 2. Matthieu made an argument against enabling commands like "git
> branch -D -" even by mistake [1]. The way that I have implemented
> ensured that not a lot of "rm"-like commands were enabled.
>
> My patch that would enable this shorthand for other projects is
> here[2].
>
> [1]: http://public-inbox.org/git/vpqh944eof7.fsf@anie.imag.fr/
> [2]: http://public-inbox.org/git/1488007487-12965-5-git-send-email-kannan.siddharth12@gmail.com/
>
> Thanks,
> Siddharth.

Re: [PATCH GSoC] Allow "-" as a short-hand for "@{-1}" in branch deletions

[Shuyang Shi]

Hi Stefan,

Really appreciate your help on this, but I guess I am cancelling this
patch for Siddharth's.

Thanks,
Shuyang
史舒扬 Shuyang Shi
Undergraduate
Department of CS, School of EECS, Peking University
Email: shuyang790@gmail.com
Mobile: +86-18301336991


On Fri, Mar 10, 2017 at 1:47 AM, Stefan Beller <sbeller@google.com> wrote:
> Welcome to the Git community!
>
> On Wed, Mar 8, 2017 at 7:31 PM, Shuyang Shi <shuyang790@gmail.com> wrote:
>> The "-" shorthand that stands for "the branch we were previously on",
>> like we did for "git merge -" sometime after we introduced "git checkout -".
>> Now I am introducing this shorthand to branch delete, i.e.
>> "git branch -d -".
>>
>> More reference:
>>   https://public-inbox.org/git/7vppuewl6h.fsf@alter.siamese.dyndns.org/
>
> Following that link:
>
>> But there is a very commonly accepted long tradition for "-" to mean
>> "read from the standard input", so we cannot reuse it to mean "the
>> branch I was previously on" for every command without first making
>> sure the command will never want to use "-" for the other common
>> purpose.
>
> This contradicts the introduction of "git branch -d -" to mean to delete
> the last branch, but rather could mean "read from stdin which branches
> to delete"? It would be nice if you could clarify in your commit message
> which of both this is and how this fits into the big picture of "design
> cleanliness".
>
>>
>> And this has been tested:
>>
>>         Ivan:git Ivan$ (cd t; prove --timer --jobs 1 ./t3200-branch.sh)
>>         [00:21:26] ./t3200-branch.sh .. ok    12293 ms ( 0.04 usr  0.01 sys +
>>         5.97 cusr  2.52 csys =  8.54 CPU)
>>         [00:21:39]
>>         All tests successful.
>>         Files=1, Tests=113, 13 wallclock secs ( 0.07 usr  0.02 sys +
>>         5.97 cusr  2.52 csys =  8.58 CPU)
>>         Result: PASS
>
> Thanks for being cautious when developing on Git. However this part
> of the email would end up as part of the commit message. And as we expect
> all commits that land eventually to not break tests, this information is better
> put at a more non-permanent place, such as below the '---' line (where there is
> also the built stat. For example see [1] how to have different message parts
> (one permanent section and some chatter that is relevant for the process
> at the moment)
>
> Also for testing, the tests only ensure that the old behavior does not break;
> but we'd want to make sure the new functionality doesn't break in the
> future either,
> which can be done best by writing a test as well for this functionality.
>
> [1] https://public-inbox.org/git/xmqqvarj1kix.fsf_-_@gitster.mtv.corp.google.com/
> and as a commit:
> https://github.com/gitster/git/commit/83218867fbf6d27c78efe3cfba01790b2f1d15d4
>
>> https://github.com/git/git/pull/337
>
> Oh I see, you're using submitgit to communicate the patch to the mailing list.
> I am not sure if it supports splitting up the message as I eluded to above.
> IIRC some people use submitgit for the patch and then use a webmailer
> (e.g. gmail) to send followup messages such as successful tests or what changed
> to prior versions.
>
> Thanks,
> Stefan