From: Cornelius Weig <cornelius.w...@tngtech.com>

Thanks for taking a look at my last version.

> On the other hand, it's not like failing to describe the tagged
> commit in the reflog is such a grave error.  If we can get away with
> being vague on a tag that points at an object of unknown type like
> the above code does, we could loosen the "oops, we thought we got a
> commit, but it turns out that we cannot read it" case below from
> die() to just stuffing generic _("commit object") in the reflog.

Good point. I agree that failing to create the message should be no reason to
die().
As you also pointed out, "internal object" is no reliable description
for unhandled object types. I changed that as well.

Changes wrt v3 (interdiff below):
 - change default message for unhandled object types
 - do not die if commit is not readable, but use default description instead
 - test: use verbatim HT character instead of \t


Cornelius Weig (1):
  tag: generate useful reflog message

 builtin/tag.c  | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 t/t7004-tag.sh | 16 +++++++++++++++-
 2 files changed, 68 insertions(+), 2 deletions(-)

Interdiff v3..v4:
diff --git a/builtin/tag.c b/builtin/tag.c
index 638b68e..9b2eabd 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -323,19 +323,19 @@ static void create_reflog_msg(const unsigned char *sha1, 
struct strbuf *sb)
        type = sha1_object_info(sha1, NULL);
        switch (type) {
        default:
-               strbuf_addstr(sb, _("internal object"));
+               strbuf_addstr(sb, _("object of unknown type"));
                break;
        case OBJ_COMMIT:
-               c = lookup_commit_reference(sha1);
-               buf = read_sha1_file(sha1, &type, &size);
-               if (!c || !buf) {
-                       die(_("commit object %s could not be read"),
-                               sha1_to_hex(sha1));
+               if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
+                       subject_len = find_commit_subject(buf, &subject_start);
+                       strbuf_insert(sb, sb->len, subject_start, subject_len);
+               } else {
+                       strbuf_addstr(sb, _("commit object"));
                }
-               subject_len = find_commit_subject(buf, &subject_start);
-               strbuf_insert(sb, sb->len, subject_start, subject_len);
-               strbuf_addf(sb, ", %s", show_date(c->date, 0, 
DATE_MODE(SHORT)));
                free(buf);
+
+               if ((c = lookup_commit_reference(sha1)) != NULL)
+                       strbuf_addf(sb, ", %s", show_date(c->date, 0, 
DATE_MODE(SHORT)));
                break;
        case OBJ_TREE:
                strbuf_addstr(sb, _("tree object"));
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 3c4cb58..894959f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -86,7 +86,7 @@ test_expect_success 'creating a tag with --create-reflog 
should create reflog' '
        test_when_finished "git tag -d tag_with_reflog" &&
        git tag --create-reflog tag_with_reflog &&
        git reflog exists refs/tags/tag_with_reflog &&
-       sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+       sed -e "s/^.*   //" .git/logs/refs/tags/tag_with_reflog > actual &&
        test_cmp expected actual
 '

@@ -96,7 +96,7 @@ test_expect_success 'annotated tag with --create-reflog has 
correct message' '
        test_when_finished "git tag -d tag_with_reflog" &&
        git tag -m "annotated tag" --create-reflog tag_with_reflog &&
        git reflog exists refs/tags/tag_with_reflog &&
-       sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+       sed -e "s/^.*   //" .git/logs/refs/tags/tag_with_reflog > actual &&
        test_cmp expected actual
 '
-- 
2.10.2

Reply via email to