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

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