"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
gotwebd: templateify gotweb_render_diff
To:
gameoftrees@openbsd.org
Date:
Fri, 06 Jan 2023 09:33:26 +0100

Download raw body.

Thread
To avoid calling functions from got_operations.c directly from a
template (which would hide the error), change got_output_repo_diff
into got_open_diff_for_output that returns a file with the diff in it,
to be then rendered by the template.  got_gotweb_flushfile needs to be
exposed then.

It also bundles a smaller change: the `label' argument of
got_repo_match_object_id is optional and not used, so just pass NULL
there.

note that this requires a tweak to the template I just committed.
(add while loops) so remember to `make -C template' if not running
`make webd' from the top-level directory.

diff 411c220f7134f9aea00d14f2601433cde03b0e06 169b163113a6db9878c9166aa05fbd30d05eb832
commit - 411c220f7134f9aea00d14f2601433cde03b0e06
commit + 169b163113a6db9878c9166aa05fbd30d05eb832
blob - f8b05cf0a3f3223ffcb6a59216871982fd980bc9
blob + 23e3304ca1f74cdc37c3574a5ece6285b1744bca
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -48,11 +48,10 @@ static const struct got_error *got_gotweb_flushfile(FI
     struct got_object_id *);
 static const struct got_error *got_gotweb_dupfd(int *, int *);
 static const struct got_error *got_gotweb_openfile(FILE **, int *, int *);
-static const struct got_error *got_gotweb_flushfile(FILE *, int);
 static const struct got_error *got_gotweb_blame_cb(void *, int, int,
     struct got_commit_object *,struct got_object_id *);
 
-static const struct got_error *
+const struct got_error *
 got_gotweb_flushfile(FILE *f, int fd)
 {
 	if (fseek(f, 0, SEEK_SET) == -1)
@@ -1331,7 +1330,7 @@ got_output_repo_diff(struct request *c)
 }
 
 const struct got_error *
-got_output_repo_diff(struct request *c)
+got_open_diff_for_output(FILE **fp, int *fd, struct request *c)
 {
 	const struct got_error *error = NULL;
 	struct transport *t = c->t;
@@ -1340,13 +1339,11 @@ got_output_repo_diff(struct request *c)
 	struct got_object_id *id1 = NULL, *id2 = NULL;
 	struct got_reflist_head refs;
 	FILE *f1 = NULL, *f2 = NULL, *f3 = NULL;
-	char *label1 = NULL, *label2 = NULL, *line = NULL;
-	char *newline, *eline = NULL, *color = NULL;
 	int obj_type, fd1, fd2, fd3, fd4 = -1, fd5 = -1;
-	size_t linesize = 0;
-	ssize_t linelen;
-	int wrlen = 0;
 
+	*fp = NULL;
+	*fd = -1;
+
 	TAILQ_INIT(&refs);
 
 	error = got_gotweb_openfile(&f1, &c->priv_fd[DIFF_FD_1], &fd1);
@@ -1365,14 +1362,14 @@ got_output_repo_diff(struct request *c)
 
 	if (rc->parent_id != NULL &&
 	    strncmp(rc->parent_id, "/dev/null", 9) != 0) {
-		error = got_repo_match_object_id(&id1, &label1,
+		error = got_repo_match_object_id(&id1, NULL,
 		    rc->parent_id, GOT_OBJ_TYPE_ANY,
 		    &refs, repo);
 		if (error)
 			goto done;
 	}
 
-	error = got_repo_match_object_id(&id2, &label2, rc->commit_id,
+	error = got_repo_match_object_id(&id2, NULL, rc->commit_id,
 	    GOT_OBJ_TYPE_ANY, &refs, repo);
 	if (error)
 		goto done;
@@ -1426,109 +1423,10 @@ got_output_repo_diff(struct request *c)
 		goto done;
 	}
 
-	while ((linelen = getline(&line, &linesize, f3)) != -1) {
-		if (strncmp(line, "-", 1) == 0) {
-			color = strdup("diff_minus");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "+", 1) == 0) {
-			color = strdup("diff_plus");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "@@", 2) == 0) {
-			color = strdup("diff_chunk_header");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "@@", 2) == 0) {
-			color = strdup("diff_chunk_header");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "commit +", 8) == 0) {
-			color = strdup("diff_meta");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "commit -", 8) == 0) {
-			color = strdup("diff_meta");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "blob +", 6) == 0) {
-			color = strdup("diff_meta");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "blob -", 6) == 0) {
-			color = strdup("diff_meta");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "file +", 6) == 0) {
-			color = strdup("diff_meta");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "file -", 6) == 0) {
-			color = strdup("diff_meta");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "from:", 5) == 0) {
-			color = strdup("diff_author");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "via:", 4) == 0) {
-			color = strdup("diff_author");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		} else if (strncmp(line, "date:", 5) == 0) {
-			color = strdup("diff_date");
-			if (color == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		}
+	*fp = f3;
+	*fd = fd3;
 
-		newline = strchr(line, '\n');
-		if (newline)
-			*newline = '\0';
-
-		error = gotweb_escape_html(&eline, line);
-		if (error)
-			goto done;
-
-		fcgi_printf(c, "<div class='diff_line %s'>%s</div>\n",
-		    color ? color : "", eline);
-		free(eline);
-		eline = NULL;
-
-		if (linelen > 0)
-			wrlen = wrlen + linelen;
-		free(color);
-		color = NULL;
-	}
-	if (linelen == -1 && ferror(f3))
-		error = got_error_from_errno("getline");
 done:
-	free(color);
 	if (fd4 != -1 && close(fd4) == -1 && error == NULL)
 		error = got_error_from_errno("close");
 	if (fd5 != -1 && close(fd5) == -1 && error == NULL)
@@ -1545,17 +1443,12 @@ done:
 		if (error == NULL)
 			error = f2_err;
 	}
-	if (f3) {
-		const struct got_error *f3_err =
-		    got_gotweb_flushfile(f3, fd3);
-		if (error == NULL)
-			error = f3_err;
+	if (error && f3) {
+		got_gotweb_flushfile(f3, fd3);
+		*fp = NULL;
+		*fd = -1;
 	}
 	got_ref_list_free(&refs);
-	free(line);
-	free(eline);
-	free(label1);
-	free(label2);
 	free(id1);
 	free(id2);
 	return error;
blob - a8fdd952ec48f1a229440e115de67205cf20f049
blob + 51b52835107f6c3c212348d380c7f90f51dc5b84
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -94,7 +94,6 @@ static const struct got_error *gotweb_render_diff(stru
 static const struct got_error *gotweb_get_clone_url(char **, struct server *,
     const char *, int);
 static const struct got_error *gotweb_render_blame(struct request *);
-static const struct got_error *gotweb_render_diff(struct request *);
 static const struct got_error *gotweb_render_summary(struct request *);
 static const struct got_error *gotweb_render_tag(struct request *);
 static const struct got_error *gotweb_render_tags(struct request *);
@@ -113,6 +112,7 @@ gotweb_process_request(struct request *c)
 	struct server *srv = NULL;
 	struct querystring *qs = NULL;
 	struct repo_dir *repo_dir = NULL;
+	FILE *fp = NULL;
 	uint8_t err[] = "gotwebd experienced an error: ";
 	int r, html = 0, fd = -1;
 
@@ -270,11 +270,18 @@ render:
 			goto err;
 		break;
 	case DIFF:
-		error = gotweb_render_diff(c);
+		error = got_get_repo_commits(c, 1);
 		if (error) {
 			log_warnx("%s: %s", __func__, error->msg);
 			goto err;
 		}
+		error = got_open_diff_for_output(&fp, &fd, c);
+		if (error) {
+			log_warnx("%s: %s", __func__, error->msg);
+			goto err;
+		}
+		if (gotweb_render_diff(c->tp, fp) == -1)
+			goto err;
 		break;
 	case INDEX:
 		error = gotweb_render_index(c);
@@ -340,6 +347,13 @@ done:
 done:
 	if (blob)
 		got_object_blob_close(blob);
+	if (fp) {
+		error = got_gotweb_flushfile(fp, fd);
+		if (error)
+			log_warnx("%s: got_gotweb_flushfile failure: %s",
+			    __func__, error->msg);
+		fd = -1;
+	}
 	if (fd != -1)
 		close(fd);
 	if (html && srv != NULL)
@@ -1121,75 +1135,6 @@ gotweb_render_diff(struct request *c)
 }
 
 static const struct got_error *
-gotweb_render_diff(struct request *c)
-{
-	const struct got_error *error = NULL;
-	struct transport *t = c->t;
-	struct repo_commit *rc = NULL;
-	char *age = NULL, *author = NULL, *msg = NULL;
-	int r;
-
-	error = got_get_repo_commits(c, 1);
-	if (error)
-		return error;
-
-	rc = TAILQ_FIRST(&t->repo_commits);
-
-	error = gotweb_get_time_str(&age, rc->committer_time, TM_LONG);
-	if (error)
-		goto done;
-	error = gotweb_escape_html(&author, rc->author);
-	if (error)
-		goto done;
-	error = gotweb_escape_html(&msg, rc->commit_msg);
-	if (error)
-		goto done;
-
-	r = fcgi_printf(c, "<div id='diff_title_wrapper'>\n"
-	    "<div id='diff_title'>Commit Diff</div>\n"
-	    "</div>\n"		/* #diff_title_wrapper */
-	    "<div id='diff_content'>\n"
-	    "<div id='diff_header_wrapper'>\n"
-	    "<div id='diff_header'>\n"
-	    "<div id='header_diff_title'>Diff:</div>\n"
-	    "<div id='header_diff'>%s<br />%s</div>\n"
-	    "<div class='header_commit_title'>Commit:</div>\n"
-	    "<div class='header_commit'>%s</div>\n"
-	    "<div id='header_tree_title'>Tree:</div>\n"
-	    "<div id='header_tree'>%s</div>\n"
-	    "<div class='header_author_title'>Author:</div>\n"
-	    "<div class='header_author'>%s</div>\n"
-	    "<div class='header_age_title'>Date:</div>\n"
-	    "<div class='header_age'>%s</div>\n"
-	    "<div id='header_commit_msg_title'>Message:</div>\n"
-	    "<div id='header_commit_msg'>%s</div>\n"
-	    "</div>\n"		/* #diff_header */
-	    "</div>\n"		/* #diff_header_wrapper */
-	    "<div class='dotted_line'></div>\n"
-	    "<div id='diff'>\n",
-	    rc->parent_id, rc->commit_id,
-	    rc->commit_id,
-	    rc->tree_id,
-	    author,
-	    age,
-	    msg);
-	if (r == -1)
-		goto done;
-
-	error = got_output_repo_diff(c);
-	if (error)
-		goto done;
-
-	fcgi_printf(c, "</div>\n"); /* #diff */
-	fcgi_printf(c, "</div>\n"); /* #diff_content */
-done:
-	free(age);
-	free(author);
-	free(msg);
-	return error;
-}
-
-static const struct got_error *
 gotweb_render_summary(struct request *c)
 {
 	const struct got_error *error = NULL;
blob - e633c57fe7cd48fed7a5ed3beb070b4acddc97c9
blob + 995c0cc55c236bc7cfd6ff5fcd21fa447b1d87ba
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -469,6 +469,7 @@ int	gotweb_render_rss(struct template *);
 int	gotweb_render_commits(struct template *);
 int	gotweb_render_blob(struct template *, struct got_blob_object *);
 int	gotweb_render_tree(struct template *);
+int	gotweb_render_diff(struct template *, FILE *);
 int	gotweb_render_rss(struct template *);
 
 /* parse.y */
@@ -490,13 +491,15 @@ const struct got_error *got_get_repo_owner(char **, st
 int fcgi_gen_binary_response(struct request *, const uint8_t *, int);
 
 /* got_operations.c */
+const struct got_error *got_gotweb_flushfile(FILE *, int);
 const struct got_error *got_get_repo_owner(char **, struct request *);
 const struct got_error *got_get_repo_age(char **, struct request *,
     const char *, int);
 const struct got_error *got_get_repo_commits(struct request *, int);
 const struct got_error *got_get_repo_tags(struct request *, int);
 const struct got_error *got_get_repo_heads(struct request *);
-const struct got_error *got_output_repo_diff(struct request *);
+const struct got_error *got_open_diff_for_output(FILE **, int *,
+    struct request *);
 int got_output_repo_tree(struct request *,
     int (*)(struct template *, struct got_tree_entry *));
 const struct got_error *got_open_blob_for_output(struct got_blob_object **,
blob - 4990d4532fe8a87678d718337343c8101501b84d
blob + 2c69414f89ce33ab6b1364fdb7b6f3c36f310825
--- gotwebd/pages.tmpl
+++ gotwebd/pages.tmpl
@@ -39,6 +39,7 @@ static inline int rss_tag_item(struct template *, stru
 static int gotweb_render_blob_line(struct template *, const char *, size_t);
 static int gotweb_render_tree_item(struct template *, struct got_tree_entry *);
 
+static inline int gotweb_render_diff_line(struct template *, char *);
 static inline int rss_tag_item(struct template *, struct repo_tag *);
 static inline int rss_author(struct template *, char *);
 
@@ -568,6 +569,83 @@ gotweb_render_age(struct template *tp, time_t time, in
 !}
 {{ end }}
 
+{{ define gotweb_render_diff(struct template *tp, FILE *fp) }}
+{!
+	struct request		*c = tp->tp_arg;
+	struct transport	*t = c->t;
+	struct repo_commit	*rc = TAILQ_FIRST(&t->repo_commits);
+	char			*line = NULL;
+	size_t			 linesize = 0;
+	ssize_t			 linelen;
+!}
+<div id="diff_title_wrapper">
+  <div id="diff_title">Commit Diff</div>
+</div>
+<div id="diff_content">
+  <div id="diff_header_wrapper">
+    <div id="diff_header">
+      <div id="header_diff_title">Diff:</div>
+      <div id="header_diff">
+        {{ rc->parent_id }}
+        <br />
+        {{ rc->commit_id }}
+      </div>
+      <div class="header_commit_title">Commit:</div>
+      <div class="header_commit">{{ rc->commit_id }}</div>
+      <div id="header_tree_title">Tree:</div>
+      <div id="header_tree">{{ rc->tree_id }}</div>
+      <div class="header_author_title">Author:</div>
+      <div class="header_author">{{ rc->author }}</div>
+      <div class="header_age_title">Date:</div>
+      <div class="header_age">
+        {{ render gotweb_render_age(tp, rc->committer_time, TM_LONG) }}
+      </div>
+      <div id="header_commit_msg_title">Message</div>
+      <div id="header_commit_msg">{{ rc->commit_msg }}</div>
+    </div>
+  </div>
+  <div class="dotted_line"></div>
+  <div id="diff">
+    {{ "\n" }}
+    {{ while (linelen = getline(&line, &linesize, fp)) != -1 }}
+      {{ render gotweb_render_diff_line(tp, line) }}
+    {{ end }}
+  </div>
+</div>
+{{ finally }}
+{! free(line); !}
+{{ end }}
+
+{{ define gotweb_render_diff_line(struct template *tp, char *line )}}
+{!
+	const char		*color = NULL;
+	char			*nl;
+
+	if (!strncmp(line, "-", 1))
+		color = "diff_minus";
+	else if (!strncmp(line, "+", 1))
+		color = "diff_plus";
+	else if (!strncmp(line, "@@", 2))
+		color = "diff_chunk_header";
+	else if (!strncmp(line, "commit +", 8) ||
+	    !strncmp(line, "commit -", 8) ||
+	    !strncmp(line, "blob +", 6) ||
+	    !strncmp(line, "blob -", 6) ||
+	    !strncmp(line, "file +", 6) ||
+	    !strncmp(line, "file -", 6))
+		color = "diff_meta";
+	else if (!strncmp(line, "from:", 5) || !strncmp(line, "via:", 4))
+		color = "diff_author";
+	else if (!strncmp(line, "date:", 5))
+		color = "diff_date";
+
+	nl = strchr(line, '\n');
+	if (nl)
+		*nl = '\0';
+!}
+<div class="diff_line {{ color }}">{{ line }}</div>
+{{ end }}
+
 {{ define gotweb_render_rss(struct template *tp) }}
 {!
 	struct request		*c = tp->tp_arg;