On Thu, Mar 28, 2024 at 10:44:37AM +0100, Omar Polo wrote:
> On 2024/03/28 10:28:27 +0100, Omar Polo <op@omarpolo.com> wrote:
> > This changes the produced json so that the "author" field becomes a
> > object itself with the following fields
> > 
> > 	"author": {
> > 		"full": "Omar Polo <op@openbsd.org>",
> > 		"name": "Omar Polo",
> > 		"mail": "op@openbsd.org",
> > 		"user": "op"
> > 	}
> > 
> > and similarly for the committer.  The only field that is guaranteed to
> > be always present is author.user since the short formats shows only the
> > (abbreviated) username.
> > 
> > In the short format we should report "committer" instead of "author"
> > since it's actually the committer.
> > 
> > To make things consistent, in the "long" format remember the author and
> > use that for the committer too if not provided.  This way, we can
> > guaranteed that committer.user is always present in all notifications.
> > 
> > Then, similarly, instead of a single field "message" that is the full
> > commit message in some cases and the first line in some others, add an
> > extra field "short_message" that is just the first line.  Again, this is
> > to guarantee that "short_message" is always present.
> > 
> > There is some redundancy in the JSON now, but I think it's better to
> > make the consuming easier.
> > 
> > For example, knowing this, to produce notifications in the IRC channel
> > one only has to extract the id, committer.user and short_message field
> > (that are now guaranteed to always exist).
> > 
> > ok?
> 
> updated diff after the unicode one went in

ok by me

> diff refs/heads/main refs/heads/json
> commit - ea5e974da9b1047689411a00ecc0a9c1fb101d73
> commit + 8e0fc5336f9ab65e8836698df34c4855cdaa2152
> blob - e1dd226bd532d7adaf1e4712e8c13bf3cc29ffba
> blob + 65830ea27b3c8094af7418498500f922ed75ecee
> --- gotd/libexec/got-notify-http/got-notify-http.c
> +++ gotd/libexec/got-notify-http/got-notify-http.c
> @@ -149,6 +149,46 @@ json_field(FILE *fp, const char *key, const char *val,
>  	fprintf(fp, "\"%s", comma ? "," : "");
>  }
>  
> +static void
> +json_author(FILE *fp, const char *type, char *address, int comma)
> +{
> +	char	*gt, *lt, *at, *email, *endname;
> +
> +	fprintf(fp, "\"%s\":{", type);
> +
> +	gt = strchr(address, '<');
> +	if (gt != NULL) {
> +		/* long format, e.g. "Omar Polo <op@openbsd.org>" */
> +
> +		json_field(fp, "full", address, 1);
> +
> +		endname = gt;
> +		while (endname > address && endname[-1] == ' ')
> +			endname--;
> +
> +		*endname = '\0';
> +		json_field(fp, "name", address, 1);
> +
> +		email = gt + 1;
> +		lt = strchr(email, '>');
> +		if (lt)
> +			*lt = '\0';
> +
> +		json_field(fp, "mail", email, 1);
> +
> +		at = strchr(email, '@');
> +		if (at)
> +			*at = '\0';
> +
> +		json_field(fp, "user", email, 0);
> +	} else {
> +		/* short format only shows the username */
> +		json_field(fp, "user", address, 0);
> +	}
> +
> +	fprintf(fp, "}%s", comma ? "," : "");
> +}
> +
>  static int
>  jsonify_short(FILE *fp)
>  {
> @@ -187,9 +227,9 @@ jsonify_short(FILE *fp)
>  
>  		fprintf(fp, "{\"short\":true,");
>  		json_field(fp, "id", id, 1);
> -		json_field(fp, "author", author, 1);
> +		json_author(fp, "committer", author, 1);
>  		json_field(fp, "date", date, 1);
> -		json_field(fp, "message", message, 0);
> +		json_field(fp, "short_message", message, 0);
>  		fprintf(fp, "}");
>  	}
>  
> @@ -206,6 +246,7 @@ static int
>  jsonify(FILE *fp)
>  {
>  	const char	*errstr;
> +	char		*author = NULL;
>  	char		*l;
>  	char		*line = NULL;
>  	size_t		 linesize = 0;
> @@ -254,7 +295,13 @@ jsonify(FILE *fp)
>  			if (strncmp(l, "from: ", 6) != 0)
>  				errx(1, "unexpected from line");
>  			l += 6;
> -			json_field(fp, "author", l, 1);
> +
> +			author = strdup(l);
> +			if (author == NULL)
> +				err(1, "strdup");
> +
> +			json_author(fp, "author", l, 1);
> +
>  			phase = P_VIA;
>  			break;
>  
> @@ -262,10 +309,17 @@ jsonify(FILE *fp)
>  			/* optional */
>  			if (!strncmp(l, "via: ", 5)) {
>  				l += 5;
> -				json_field(fp, "via", l, 1);
> +				json_author(fp, "committer", l, 1);
>  				phase = P_DATE;
>  				break;
>  			}
> +
> +			if (author == NULL) /* impossible */
> +				err(1, "from not specified");
> +			json_author(fp, "committer", author, 1);
> +			free(author);
> +			author = NULL;
> +
>  			phase = P_DATE;
>  			/* fallthrough */
>  
> @@ -313,7 +367,6 @@ jsonify(FILE *fp)
>  			if (errstr)
>  				errx(1, "message len is %s: %s", errstr, l);
>  
> -			fprintf(fp, "\"message\":\"");
>  			phase = P_MSG;
>  			break;
>  
> @@ -329,14 +382,21 @@ jsonify(FILE *fp)
>  			 * tolerate one byte less than advertised.
>  			 */
>  			if (*l == ' ') {
> -				escape(fp, l + 1); /* skip leading space */
> +				l++; /* skip leading space */
> +				linelen--;
>  
> -				/* avoid pre-pending \n to the commit msg */
> -				msgwrote += linelen - 1;
> -				if (msgwrote != 0)
> +				if (msgwrote == 0 && linelen != 0) {
> +					json_field(fp, "short_message", l, 1);
> +					fprintf(fp, "\"message\":\"");
> +					escape(fp, l);
>  					escape(fp, "\n");
> +					msgwrote += linelen;
> +				} else if (msgwrote != 0) {
> +					escape(fp, l);
> +					escape(fp, "\n");
> +				}
>  			}
> -			msglen -= linelen;
> +			msglen -= linelen + 1;
>  			if (msglen <= 1) {
>  				fprintf(fp, "\",");
>  				msgwrote = 0;
> blob - 3255fa55f2841d50b5170e35303b4e7622843e04
> blob + 339f4a7c90ab5de6679eb61927eb9ff684c5acea
> --- regress/gotd/http_notification.sh
> +++ regress/gotd/http_notification.sh
> @@ -62,8 +62,20 @@ test_file_changed() {
>  	{"notifications":[{
>  		"short":false,
>  		"id":"$commit_id",
> -		"author":"$GOT_AUTHOR",
> +		"author":{
> +			"full":"$GOT_AUTHOR",
> +			"name":"$GIT_AUTHOR_NAME",
> +			"mail":"$GIT_AUTHOR_EMAIL",
> +			"user":"$GOT_AUTHOR_11"
> +		},
> +		"committer":{
> +			"full":"$GOT_AUTHOR",
> +			"name":"$GIT_AUTHOR_NAME",
> +			"mail":"$GIT_AUTHOR_EMAIL",
> +			"user":"$GOT_AUTHOR_11"
> +		},
>  		"date":"$d",
> +		"short_message":"make changes",
>  		"message":"make changes\n",
>  		"diffstat":{},
>  		"changes":{}
> @@ -131,8 +143,20 @@ test_bad_utf8() {
>  	{"notifications":[{
>  		"short":false,
>  		"id":"$commit_id",
> -		"author":"$GOT_AUTHOR",
> +		"author":{
> +			"full":"$GOT_AUTHOR",
> +			"name":"$GIT_AUTHOR_NAME",
> +			"mail":"$GIT_AUTHOR_EMAIL",
> +			"user":"$GOT_AUTHOR_11"
> +		},
> +		"committer":{
> +			"full":"$GOT_AUTHOR",
> +			"name":"$GIT_AUTHOR_NAME",
> +			"mail":"$GIT_AUTHOR_EMAIL",
> +			"user":"$GOT_AUTHOR_11"
> +		},
>  		"date":"$d",
> +		"short_message":"make\uFFFD\uFFFDchanges",
>  		"message":"make\uFFFD\uFFFDchanges\n",
>  		"diffstat":{},
>  		"changes":{}
> @@ -208,8 +232,20 @@ test_many_commits_not_summarized() {
>  		{
>  			"short":false,
>  			"id":"$commit_id",
> -			"author":"$GOT_AUTHOR",
> +			"author":{
> +				"full":"$GOT_AUTHOR",
> +				"name":"$GIT_AUTHOR_NAME",
> +				"mail":"$GIT_AUTHOR_EMAIL",
> +				"user":"$GOT_AUTHOR_11"
> +			},
> +			"committer":{
> +				"full":"$GOT_AUTHOR",
> +				"name":"$GIT_AUTHOR_NAME",
> +				"mail":"$GIT_AUTHOR_EMAIL",
> +				"user":"$GOT_AUTHOR_11"
> +			},
>  			"date":"$commit_time",
> +			"short_message":"make changes",
>  			"message":"make changes\n",
>  			"diffstat":{},
>  			"changes":{}
> @@ -289,9 +325,11 @@ test_many_commits_summarized() {
>  		{
>  			"short":true,
>  			"id":"$commit_id",
> -			"author":"$GOT_AUTHOR_8",
> +			"committer":{
> +				"user":"$GOT_AUTHOR_8"
> +			},
>  			"date":"$commit_time",
> -			"message":"make changes"
> +			"short_message":"make changes"
>  		}
>  		EOF
>  	done >> $testroot/stdout.expected
> 
>