[PATCH v4] tag: generate useful reflog message

[cornelius.weig@xxxxxxxxxxx]

From: Cornelius Weig <cornelius.weig@xxxxxxxxxxx>

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