[PATCH v2 4/4] notes: only allow to append a blob to a blob

["Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 On Thu, May 10, 2012 at 10:45 PM, Jeff King <peff@peff.net> wrote:
 > On Thu, May 10, 2012 at 10:31:23PM +0700, Nguyen Thai Ngoc Duy wrote:
 >
 >> >> +static const char *type_name(enum object_type type)
 >> >> +{
 >> >> +     switch (type) {
 >> >> +     case OBJ_BLOB: return _("a blob");
 >> >> +     case OBJ_TAG: return _("a tag");
 >> >> +     case OBJ_COMMIT: return _("a commit");
 >> >> +     case OBJ_TREE: return _("a tree");
 >> >> +     default:
 >> >> +             die("BUG: put a new string for type %d here", type);
 >> >> +     }
 >> >> +}
 >> >
 >> > Don't we have object.c:typename for this
 >>
 >> The key difference here is _() with an article. It's i18n friendly. I
 >> wanted to make 15 combinations (blob/blob cannot happen) of "cannot
 >> append %s to %s", which is best for translators but probably too much
 >> for C developers.
 >
 > I do not pay much attention to the translation details, but I would
 > think that we would keep terms like "tree" and "blob" universal, as they
 > are technical terms. IOW, you would not expect the "blob" in "git
 > cat-file blob $sha1" to be internationalized, and this seems like the
 > same level of technical detail.

 It's the article that's important here. Putting "appending a %s to a
 %s" may be safe in this case because no object type begins with a
 vowel (and we would need to turn "a" to "an"). But it raises a
 question: does any other language have different forms of article
 depending on the main noun?

 But this is for the sake of discussion. The new patch (with your
 suggestion) does not have this problem any more.

 builtin/notes.c  |    7 ++++++-
 t/t3301-notes.sh |    6 ++++++
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 44fb8b6..a90d9b6 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -204,8 +204,13 @@ static void create_note(const unsigned char *object, struct msg_arg *msg,
 		strbuf_grow(&(msg->buf), size + 1);
 		if (msg->buf.len && prev_buf && size)
 			strbuf_insert(&(msg->buf), 0, "\n", 1);
-		if (prev_buf && size)
+		if (prev_buf && size) {
+			if (type != OBJ_BLOB)
+				die(_("cannot append to a non-blob note"));
+			if (msg->type != OBJ_BLOB)
+				die(_("cannot append non-blob object to a note"));
 			strbuf_insert(&(msg->buf), 0, prev_buf, size);
+		}
 		free(prev_buf);
 	}
 
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 66cc872..7a682fe 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -1240,4 +1240,10 @@ test_expect_success 'cannot edit non-blob notes' '
 	test_must_fail git notes edit
 '
 
+test_expect_success 'refuse to concatenate two notes of different type' '
+	test_must_fail git notes append -m foo &&
+	git notes add -f -m foo &&
+	test_must_fail git notes append -C HEAD^{tree}
+'
+
 test_done
-- 
1.7.3.1.256.g2539c.dirty