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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michał Górny <mgorny@xxxxxxxxxx> 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?

> @@ -478,18 +481,20 @@ static void find_subpos(const char *buf, unsigned long sz, const char **sub, con
>  	buf = strchr(buf, '\n');
>  	if (!buf) {
>  		*body = "";
> +		*signature = *body;
>  		return; /* no body */
>  	}
>  	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?
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.

>  static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
>  {
>  	int i;
> -	const char *subpos = NULL, *bodypos = NULL;
> +	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
>  
>  	for (i = 0; i < used_atom_cnt; i++) {
>  		const char *name = used_atom[i];
> @@ -500,19 +505,26 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
>  			name++;
>  		if (strcmp(name, "subject") &&
>  		    strcmp(name, "body") &&
> -		    strcmp(name, "contents"))
> +		    strcmp(name, "contents") &&
> +		    strcmp(name, "contents:subject") &&
> +		    strcmp(name, "contents:body") &&
> +		    strcmp(name, "contents:signature"))
>  			continue;
>  		if (!subpos)
> -			find_subpos(buf, sz, &subpos, &bodypos);
> +			find_subpos(buf, sz, &subpos, &bodypos, &sigpos);
>  		if (!subpos)
>  			return;
>  
> -		if (!strcmp(name, "subject"))
> +		if (!strcmp(name, "subject") || !strcmp(name, "contents:subject"))
>  			v->s = copy_line(subpos);
>  		else if (!strcmp(name, "body"))
>  			v->s = xstrdup(bodypos);
>  		else if (!strcmp(name, "contents"))
>  			v->s = xstrdup(subpos);
> +		else if (!strcmp(name, "contents:body"))
> +			v->s = xstrndup(bodypos, sigpos - bodypos);
> +		else if (!strcmp(name, "contents:signature"))
> +			v->s = xstrdup(sigpos);

Again, how does this work for an annotated but not signed tag?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]