FEATURE REQUEST: display <commit SHA> in message: git tag -d

FEATURE REQUEST: display <commit SHA> in message: git tag -d

[Jari Aalto]

It would be helpful if the delete command would displayed the SHA:

    $ git tag -d foo/20060213-1
    Deleted tag 'foo/20060213-1'

    ...  oops, I didn't want to do that
    ...  Whatwas the commit ID again that I can restore the tag?

Instead:

    $ git tag -d foo/20060213-1
    Deleted tag 'foo/20060213-1' 4b397f6

    ...  oops, I didn't want to do that

    $ git tag 'foo/20060213-1' 4b397f6

    ... retagged, phew.

Notice that the message is in the format of copy/paste for immediate
retagging.

Jari

[PATCH] tag -d: print sha1 of deleted tag

[Michael J Gruber]

Print the sha1 of the deleted tag (in addition to the tag name) so that
one can easily recreate a mistakenly deleted tag:

git tag -d tagname
Deleted tag 'tagname' DEADBEEF
git tag 'tagname' DEADBEEF # for lightweight tags
git update-ref refs/tags/'tagname' DEADBEEF # for annotated tags

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Suggested-by: Jari Aalto <jari.aalto@cante.net>
---
 builtin-tag.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index c479018..39d0ce2 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -140,7 +140,7 @@ static int delete_tag(const char *name, const char *ref,
 {
 	if (delete_ref(ref, sha1, 0))
 		return 1;
-	printf("Deleted tag '%s'\n", name);
+	printf("Deleted tag '%s' %s\n", name, sha1_to_hex(sha1));
 	return 0;
 }
 
-- 
1.6.6.rc1.292.gd8fe

Re: [PATCH] tag -d: print sha1 of deleted tag

[Björn Steinbrink]

On 2009.12.10 13:23:43 +0100, Michael J Gruber wrote:
> Print the sha1 of the deleted tag (in addition to the tag name) so that
> one can easily recreate a mistakenly deleted tag:
> 
> git tag -d tagname
> Deleted tag 'tagname' DEADBEEF
> git tag 'tagname' DEADBEEF # for lightweight tags
> git update-ref refs/tags/'tagname' DEADBEEF # for annotated tags

Using "git tag 'tagname' DEADBEEF" should actually work in both cases.
As that does nothing but creating the ref in the refs/tags/ namespace.

Bjoern

Re: [PATCH] tag -d: print sha1 of deleted tag

[Michael J Gruber]

Björn Steinbrink venit, vidit, dixit 10.12.2009 13:47:
> On 2009.12.10 13:23:43 +0100, Michael J Gruber wrote:
>> Print the sha1 of the deleted tag (in addition to the tag name) so that
>> one can easily recreate a mistakenly deleted tag:
>>
>> git tag -d tagname
>> Deleted tag 'tagname' DEADBEEF
>> git tag 'tagname' DEADBEEF # for lightweight tags
>> git update-ref refs/tags/'tagname' DEADBEEF # for annotated tags
> 
> Using "git tag 'tagname' DEADBEEF" should actually work in both cases.
> As that does nothing but creating the ref in the refs/tags/ namespace.
> 
> Bjoern

Cool, even better! So, an annotated tag is practically a lightweight tag
pointing to a tag object. Once you think of it it's natural!

Michael

Re: [PATCH] tag -d: print sha1 of deleted tag

[Jeff King]

On Thu, Dec 10, 2009 at 01:23:43PM +0100, Michael J Gruber wrote:

> Print the sha1 of the deleted tag (in addition to the tag name) so that
> one can easily recreate a mistakenly deleted tag:
> 
> git tag -d tagname
> Deleted tag 'tagname' DEADBEEF
> git tag 'tagname' DEADBEEF # for lightweight tags
> git update-ref refs/tags/'tagname' DEADBEEF # for annotated tags

I think this is a good idea, and we already do the same for branch
deletion.

I'm not sure your example is right. If "tag -d" always prints out the
sha1 in the tag ref, can't you just use "git tag 'tagname' DEADBEEF" to
recreate both lightweight and annotated tags? That is, making a
lightweight tag of an annotated tag's sha1 should just recreate the
original annotated tag.

That being said, I am not a fan of the cut-and-paste format. This is not
something that happens so frequently that I think we need to go out of
our way to save some typing. And for a user seeing this message for the
first time:

  1. It is not immediately obvious to a user seeing this message
     for this first time exactly what the trailing sha1 means. We
     already had this discussion with "git branch -d" and decided
     that "(was DEADBEEF)" was more readable.

  2. Even if they know what it means, it is not immediately obvious that
     the error line is meant to be cut-and-pasted. If you are going to
     give something to cut-and-paste, I think you are better off making
     it obvious, like:

        Deleted tag 'foo'; you can recreate it with

           git tag 'foo' DEADBEEF

     Of course that is painfully long for a message that is meant to be
     a "just in case" notification of a successful command (I can see it
     more for an actual error, where git is telling you "I couldn't do
     what you wanted, but you might try running this command first").

-Peff

Re: [PATCH] tag -d: print sha1 of deleted tag

[Jari Aalto]

Jeff King <peff@peff.net> writes:

> On Thu, Dec 10, 2009 at 01:23:43PM +0100, Michael J Gruber wrote:
>
>> Print the sha1 of the deleted tag (in addition to the tag name) so that
>> one can easily recreate a mistakenly deleted tag:
>> 
>> git tag -d tagname
>> Deleted tag 'tagname' DEADBEEF
>> git tag 'tagname' DEADBEEF # for lightweight tags
>> git update-ref refs/tags/'tagname' DEADBEEF # for annotated tags

> That being said, I am not a fan of the cut-and-paste format. This is not
> something that happens so frequently

It dpends on user. For me it it does.

>   1. It is not immediately obvious to a user seeing this message
>      for this first time exactly what the trailing sha1 means.
>
>   2. Even if they know what it means, it is not immediately obvious that
>      the error line is meant to be cut-and-pasted.

"not meant" specifically, but it's very convenient to have it in format
that happens to be "cut-n-paste ready". The SHA itself is easily
understood in the context.

Thanks for all that have so quickly implemented this,
Jari

Re: [PATCH] tag -d: print sha1 of deleted tag

[Michael J Gruber]

Jeff King venit, vidit, dixit 10.12.2009 13:49:
> On Thu, Dec 10, 2009 at 01:23:43PM +0100, Michael J Gruber wrote:
> 
>> Print the sha1 of the deleted tag (in addition to the tag name) so that
>> one can easily recreate a mistakenly deleted tag:
>>
>> git tag -d tagname
>> Deleted tag 'tagname' DEADBEEF
>> git tag 'tagname' DEADBEEF # for lightweight tags
>> git update-ref refs/tags/'tagname' DEADBEEF # for annotated tags
> 
> I think this is a good idea, and we already do the same for branch
> deletion.
> 
> I'm not sure your example is right. If "tag -d" always prints out the
> sha1 in the tag ref, can't you just use "git tag 'tagname' DEADBEEF" to
> recreate both lightweight and annotated tags? That is, making a
> lightweight tag of an annotated tag's sha1 should just recreate the
> original annotated tag.

While my example is right it is unnecessarily complex. I learned that
through Björns and your remark.

> That being said, I am not a fan of the cut-and-paste format. This is not
> something that happens so frequently that I think we need to go out of
> our way to save some typing. And for a user seeing this message for the
> first time:
> 
>   1. It is not immediately obvious to a user seeing this message
>      for this first time exactly what the trailing sha1 means. We
>      already had this discussion with "git branch -d" and decided
>      that "(was DEADBEEF)" was more readable.

So, should we simply go with that then?

Meanwhile, RFCs/PATCHes crossed paths. I take it that Zoltan suggests
giving the same output for force-overwritten existing tags. I beat him
by 11 minutes, though ;)

Michael

Re: [PATCH] tag -d: print sha1 of deleted tag

[Jeff King]

On Thu, Dec 10, 2009 at 02:27:15PM +0100, Michael J Gruber wrote:

> >   1. It is not immediately obvious to a user seeing this message
> >      for this first time exactly what the trailing sha1 means. We
> >      already had this discussion with "git branch -d" and decided
> >      that "(was DEADBEEF)" was more readable.
> 
> So, should we simply go with that then?

I think so. Jari obviously disagrees, but I don't have much more to say
in favor of it except that I find the other ugly and unintuitive. So it
is up to you what you want to submit and Junio what he wants to apply.
:)

> Meanwhile, RFCs/PATCHes crossed paths. I take it that Zoltan suggests
> giving the same output for force-overwritten existing tags. I beat him
> by 11 minutes, though ;)

Yes, I think if you are going to protect "tag -d", you might as well
protect overwriting, as well.  Which made me think at first that we need
something similar for "branch -f", but I don't think we do; the last
branch value will be left in the reflog (but with tags, there is no
reflog).

-Peff

[PATCH v2] tag -d: print sha1 of deleted tag

[Michael J Gruber]

Print the sha1 of the deleted tag (in addition to the tag name) so that
one can easily recreate a mistakenly deleted tag:

git tag -d tagname
Deleted tag 'tagname' (was DEADBEEF)
git tag 'tagname' DEADBEEF

We output the previous ref also in the case of forcefully overwriting
tags.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Suggested-by: Jari Aalto <jari.aalto@cante.net>
Helped-by: Björn Steinbrink <B.Steinbrink@gmx.de>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Zoltán Füzesi <zfuzesi@eaglet.hu>
---
v2 changes the wording to match with branch -d and uses the same
for forcefully overwriting tags.

Zoltán, I don't think we should make this into a race. Posting in
the relevant thread (and actually following it) would help this.

Also, I think we should really compare the sha1 the tag points to,
i.e. like below and like in your v1 (not v2). Different tag object
is different tag (message may differ, e.g.).

 builtin-tag.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index c479018..4ef1c4f 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -140,7 +140,7 @@ static int delete_tag(const char *name, const char *ref,
 {
 	if (delete_ref(ref, sha1, 0))
 		return 1;
-	printf("Deleted tag '%s'\n", name);
+	printf("Deleted tag '%s' (was %s)\n", name, find_unique_abbrev(sha1, DEFAULT_ABBREV));
 	return 0;
 }
 
@@ -479,6 +479,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		die("%s: cannot lock the ref", ref);
 	if (write_ref_sha1(lock, object, NULL) < 0)
 		die("%s: cannot update the ref", ref);
+	if (force && hashcmp(prev, object))
+		printf("Updated tag '%s' (was %s)\n", tag, find_unique_abbrev(prev, DEFAULT_ABBREV));
 
 	strbuf_release(&buf);
 	return 0;
-- 
1.6.6.rc1.292.gd8fe

Re: [PATCH v2] tag -d: print sha1 of deleted tag

[Zoltán Füzesi]

2009/12/10 Michael J Gruber <git@drmicha.warpmail.net>:
> Zoltán, I don't think we should make this into a race. Posting in
> the relevant thread (and actually following it) would help this.

You are right. I've deleted the feature request mail, and then few
minutes laster decided to create the patch. So I've lost the message
ID (though I could retrieve it from the web...).
After posting the patch, noticed that you also posted one.

> Also, I think we should really compare the sha1 the tag points to,
> i.e. like below and like in your v1 (not v2). Different tag object
> is different tag (message may differ, e.g.).

I agree.