Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

Michał Górny [off-list ref] writes:

Now %(contents:subject) contains the message subject, %(contents:body)
main body part and %(contents:signature) GPG signature.
---

Please sign-off when submitting the final round of this patch.

+The complete message in a commit and tag object is `contents`.
+Its first line is `contents:subject`, the remaining lines
+are `contents:body` and the optional GPG signature
+is `contents:signature`.

To match the parsing of commit objects, I would prefer to see "subject" to
mean "the first paragraph" (usually the first line alone but that is
purely from convention), but that probably is a separate topic.

To paraphrase the last part of your sentence, if a tag is merely annotated
and not signed, contents:signature would be empty (I am just making sure
that I am reading the description correctly).

Is it possible to get %(contents) with a combination of these three new
variants, or does the calling script need to see if each part is empty and
decide where to place newlines?  IOW, a naïve attempt:

	--format='%(contents:subject)\n%(contents:body)\n%(contents:signature)'

is not equivalent to

	--format='%(contents)'

right?

If there is no signature, parse_signature() would return (size_t) 0, no?
I suspect it may be easier for the caller if *signature pointed at the
terminating NUL at the end of the whole thing in such a case. Otherwise,
the caller that finds the buf and the signature points at the same address
cannot tell if the object was a signed tag with no message

	$ git tag -s -m "" empty-tag

or if it was an annotated but not signed tag.

Or better yet, you may even want to stuff NULL there if there is no signature
so that it is absolutely clear for the caller which case it is.

Again, how does this work for an annotated but not signed tag?

On Wed, Aug 31, 2011 at 03:54:35PM -0700, Junio C Hamano wrote:

quoted

+The complete message in a commit and tag object is `contents`.
+Its first line is `contents:subject`, the remaining lines
+are `contents:body` and the optional GPG signature
+is `contents:signature`.

To match the parsing of commit objects, I would prefer to see "subject" to
mean "the first paragraph" (usually the first line alone but that is
purely from convention), but that probably is a separate topic.

Good idea. I suspect pretty.c:format_subject can be reused here.

To paraphrase the last part of your sentence, if a tag is merely annotated
and not signed, contents:signature would be empty (I am just making sure
that I am reading the description correctly).

That is what I checked for in the tests I added.

quoted

 	while (*buf == '\n')
 		buf++; /* skip blank between subject and body */
 	*body = buf;
+	*signature = buf + parse_signature(buf, strlen(buf));

If there is no signature, parse_signature() would return (size_t) 0, no?

No, it returns strlen(buf) in that case, making signature the empty
string. It would perhaps better be called find_signature_in_body(),
since it is actually about parsing the rest of the body until we get to
the signature.

-Peff

On Wed, 31 Aug 2011 19:22:01 -0400
Jeff King [off-list ref] wrote:

On Wed, Aug 31, 2011 at 03:54:35PM -0700, Junio C Hamano wrote:

quoted

quoted

+The complete message in a commit and tag object is `contents`.
+Its first line is `contents:subject`, the remaining lines
+are `contents:body` and the optional GPG signature
+is `contents:signature`.

To match the parsing of commit objects, I would prefer to see
"subject" to mean "the first paragraph" (usually the first line
alone but that is purely from convention), but that probably is a
separate topic.

Good idea. I suspect pretty.c:format_subject can be reused here.

Should I fix regular 'subject' and 'body' as well, or just
the 'contents:' variants?

-- 
Best regards,
Michał Górny