From: Omar Polo <op@omarpolo.com>
Subject: gotwebd: enhance blob pages
To: gameoftrees@openbsd.org
Date: Wed, 04 Jan 2023 10:47:47 +0100

Hello,

this reworks the current blob page so that it displays the file
content inline.  The current blob page is renamed to BLOBRAW, and
binary files are automatically redirected to this page (so that
i.e. clicking on a PNG icon downloads it as it does now.)

I'll add a few links to the raw version of the files in the blob page
and in the tree listing in a follow-up commit.

This also introduces a getline(3)-like function for blobs, as hinted
in a previous mail.

There are some details that I'd like to improve, both in the CSS and
in the code, but I think it can be done in tree.

I'm running with this patch on my server.  Here's an example of a
comment I need to tweak eventually:

https://git.omarpolo.com/?action=blob&commit=025c25369f278576ed92bc0c80a5b9a61a2e851f&file=got-read-patch.c&folder=%2Flibexec%2Fgot-read-patch&path=got.git#line433

(I still trust Larry, don't worry, I just got why patch(1) prefers to
do an append rather than insert, but that's for another tread ;-)

Note that this needs my latest commit in main.


diff refs/heads/main refs/heads/blob
commit - 0b287d3f8df90ae3e97d9c35ac16299fd63e0c3e
commit + 0e7a0d67888857fbe676b6c6ef640373e663b4d7
blob - d7d581b830dd549f2e3d19757290d8539f83618d
blob + c83793f9a486da3fff129c76d2b915edd05b7c40
--- gotwebd/files/htdocs/gotwebd/gotweb.css
+++ gotwebd/files/htdocs/gotwebd/gotweb.css
@@ -527,36 +527,36 @@ body {
 	white-space: pre-wrap;
 }
 
-#blame_title_wrapper {
+#blame_title_wrapper, #blob_title_wrapper {
 	clear: left;
 	float: left;
 	width: 100%;
 	background-color: LightSlateGray;
 	color: #ffffff;
 }
-#blame_title {
+#blame_title, #blob_title_wrapper {
 	padding-left: 10px;
 	padding-top: 5px;
 	padding-bottom: 5px;
 }
-#blame_content {
+#blame_content, #blob_content {
 	clear: left;
 	float: left;
 	width: 100%;
 }
-#blame_header_wrapper {
+#blame_header_wrapper, #blob_header_wrapper {
 	float: left;
 	background-color: #f5fcfb;
 	width: 100%;
 }
-#blame_header {
+#blame_header, #blob_header {
 	float: left;
 	padding-left: 10px;
 	padding-top: 5px;
 	padding-bottom: 2px;
 	width: 80%;
 }
-#blame {
+#blame, #blob {
 	clear: left;
 	float: left;
 	margin-left: 20px;
@@ -565,12 +565,15 @@ body {
 	white-space: pre;
 	overflow: auto;
 }
-.blame_wrapper {
+.blame_wrapper, .blob_line {
 	clear: left;
 	float: left;
 	width: 100%;
 }
-.blame_number {
+.blame_wrapper:target, .blob_line:target {
+	background-color: Khaki;
+}
+.blame_number, .blob_number {
 	float: left;
 	width: 6em;
 	overflow: hidden;
@@ -590,7 +593,7 @@ body {
 	width: 6em;
 	overflow: hidden;
 }
-.blame_code {
+.blame_code, .blob_code {
 	float:left;
 	width: 50%;
 	overflow: visible;
blob - d4196649f19af83bb7448f6007b5db724a9b3866
blob + 9d187ce1e16a66d113377842fd55ccbd016fd2e3
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -960,7 +960,8 @@ got_output_file_blob(struct request *c)
 }
 
 const struct got_error *
-got_output_file_blob(struct request *c)
+got_open_blob_for_output(struct got_blob_object **blob, int *fd,
+    int *binary, struct request *c)
 {
 	const struct got_error *error = NULL;
 	struct transport *t = c->t;
@@ -969,14 +970,15 @@ got_output_file_blob(struct request *c)
 	struct got_commit_object *commit = NULL;
 	struct got_object_id *commit_id = NULL;
 	struct got_reflist_head refs;
-	struct got_blob_object *blob = NULL;
 	char *path = NULL, *in_repo_path = NULL;
-	int bin, obj_type, fd = -1;
-	size_t len;
-	const uint8_t *buf;
+	int obj_type;
 
 	TAILQ_INIT(&refs);
 
+	*blob = NULL;
+	*fd = -1;
+	*binary = 0;
+
 	if (asprintf(&path, "%s%s%s", qs->folder ? qs->folder : "",
 	    qs->folder ? "/" : "", qs->file) == -1) {
 		error = got_error_from_errno("asprintf");
@@ -1014,19 +1016,52 @@ got_output_file_blob(struct request *c)
 		goto done;
 	}
 
-	error = got_gotweb_dupfd(&c->priv_fd[BLOB_FD_1], &fd);
+	error = got_gotweb_dupfd(&c->priv_fd[BLOB_FD_1], fd);
 	if (error)
 		goto done;
 
-	error = got_object_open_as_blob(&blob, repo, commit_id, BUF, fd);
+	error = got_object_open_as_blob(blob, repo, commit_id, BUF, *fd);
 	if (error)
 		goto done;
 
-	error = got_object_blob_is_binary(&bin, blob);
+	error = got_object_blob_is_binary(binary, *blob);
 	if (error)
 		goto done;
 
-	if (bin)
+ done:
+	if (commit)
+		got_object_commit_close(commit);
+
+	if (error) {
+		if (*fd != -1)
+			close(*fd);
+		if (*blob)
+			got_object_blob_close(*blob);
+		*fd = -1;
+		*blob = NULL;
+	}
+
+	free(in_repo_path);
+	free(commit_id);
+	free(path);
+	return error;
+}
+
+const struct got_error *
+got_output_file_blob(struct request *c)
+{
+	const struct got_error *error = NULL;
+	struct querystring *qs = c->t->qs;
+	struct got_blob_object *blob = NULL;
+	size_t len;
+	int binary, fd = -1;
+	const uint8_t *buf;
+
+	error = got_open_blob_for_output(&blob, &fd, &binary, c);
+	if (error)
+		return error;
+
+	if (binary)
 		error = gotweb_render_content_type_file(c,
 		    "application/octet-stream", qs->file, NULL);
 	else
@@ -1046,19 +1081,44 @@ done:
 		buf = got_object_blob_get_read_buf(blob);
 		fcgi_gen_binary_response(c, buf, len);
 	}
-done:
-	if (commit)
-		got_object_commit_close(commit);
-	if (fd != -1 && close(fd) == -1 && error == NULL)
+ done:
+	if (close(fd) == -1 && error == NULL)
 		error = got_error_from_errno("close");
 	if (blob)
 		got_object_blob_close(blob);
-	free(in_repo_path);
-	free(commit_id);
-	free(path);
 	return error;
 }
 
+int
+got_output_blob_by_lines(struct template *tp, struct got_blob_object *blob,
+    int (*cb)(struct template *, const char *, size_t))
+{
+	const struct got_error	*err;
+	char			*line = NULL;
+	size_t			 linesize = 0;
+	size_t			 lineno = 0;
+	ssize_t			 linelen = 0;
+
+	for (;;) {
+		err = got_object_blob_getline(&line, &linelen, &linesize,
+		    blob);
+		if (err || linelen == -1)
+			break;
+		lineno++;
+		if (cb(tp, line, lineno) == -1)
+			break;
+	}
+
+	free(line);
+
+	if (err) {
+		log_warnx("%s: got_object_blob_getline failed: %s",
+		    __func__, err->msg);
+		return -1;
+	}
+	return 0;
+}
+
 struct blame_line {
 	int annotated;
 	char *id_str;
blob - 39453efb256d54ee495702a9fef915782dec76ef
blob + 37d8624ec792d91e8f7438d5cc3682be3a54d179
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -66,6 +66,7 @@ static const struct action_keys action_keys[] = {
 static const struct action_keys action_keys[] = {
 	{ "blame",	BLAME },
 	{ "blob",	BLOB },
+	{ "blobraw",	BLOBRAW },
 	{ "briefs",	BRIEFS },
 	{ "commits",	COMMITS },
 	{ "diff",	DIFF },
@@ -109,11 +110,12 @@ gotweb_process_request(struct request *c)
 gotweb_process_request(struct request *c)
 {
 	const struct got_error *error = NULL, *error2 = NULL;
+	struct got_blob_object *blob = NULL;
 	struct server *srv = NULL;
 	struct querystring *qs = NULL;
 	struct repo_dir *repo_dir = NULL;
 	uint8_t err[] = "gotwebd experienced an error: ";
-	int r, html = 0;
+	int r, html = 0, fd = -1;
 
 	/* init the transport */
 	error = gotweb_init_transport(&c->t);
@@ -150,10 +152,12 @@ gotweb_process_request(struct request *c)
 	 * querystring.
 	 */
 
-	if (qs->commit == NULL && (qs->action == BLAME || qs->action == BLOB ||
-	    qs->action == DIFF)) {
-		error2 = got_error(GOT_ERR_QUERYSTRING);
-		goto render;
+	if (qs->action == BLAME || qs->action == BLOB ||
+	    qs->action == BLOBRAW || qs->action == DIFF) {
+		if (qs->commit == NULL) {
+			error2 = got_error(GOT_ERR_QUERYSTRING);
+			goto render;
+		}
 	}
 
 	if (qs->action != INDEX) {
@@ -166,7 +170,7 @@ gotweb_process_request(struct request *c)
 			goto err;
 	}
 
-	if (qs->action == BLOB) {
+	if (qs->action == BLOBRAW) {
 		error = got_get_repo_commits(c, 1);
 		if (error)
 			goto done;
@@ -178,6 +182,34 @@ gotweb_process_request(struct request *c)
 		goto done;
 	}
 
+	if (qs->action == BLOB) {
+		int binary;
+		struct gotweb_url url = {
+			.index_page = -1,
+			.page = -1,
+			.action = BLOBRAW,
+			.path = qs->path,
+			.commit = qs->commit,
+			.folder = qs->folder,
+			.file = qs->file,
+		};
+
+		error = got_get_repo_commits(c, 1);
+		if (error)
+			goto done;
+
+		error2 = got_open_blob_for_output(&blob, &fd, &binary, c);
+		if (error2)
+			goto render;
+		if (binary) {
+			fcgi_puts(c->tp, "Status: 302\r\n");
+			fcgi_puts(c->tp, "Location: ");
+			gotweb_render_url(c, &url);
+			fcgi_puts(c->tp, "\r\n\r\n");
+			goto done;
+		}
+	}
+
 	if (qs->action == RSS) {
 		error = gotweb_render_content_type_file(c,
 		    "application/rss+xml;charset=utf-8",
@@ -221,6 +253,10 @@ render:
 			goto err;
 		}
 		break;
+	case BLOB:
+		if (gotweb_render_blob(c->tp, blob) == -1)
+			goto err;
+		break;
 	case BRIEFS:
 		if (gotweb_render_briefs(c->tp) == -1)
 			goto err;
@@ -301,6 +337,10 @@ done:
 	if (html && fcgi_printf(c, "</div>\n") == -1)
 		return;
 done:
+	if (blob)
+		got_object_blob_close(blob);
+	if (fd != -1)
+		close(fd);
 	if (html && srv != NULL)
 		gotweb_render_footer(c->tp);
 }
@@ -1626,6 +1666,8 @@ gotweb_action_name(int action)
 		return "blame";
 	case BLOB:
 		return "blob";
+	case BLOBRAW:
+		return "blobraw";
 	case BRIEFS:
 		return "briefs";
 	case COMMITS:
blob - f3c092bd9402f47b404b596072606dfc3196fce8
blob + e4e23bf9c198a25c2338660068796cd385a8498c
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -115,6 +115,9 @@ enum imsg_type {
 
 #define GOTWEB_PACK_NUM_TEMPFILES     32
 
+/* Forward declaration */
+struct got_blob_object;
+
 enum imsg_type {
 	IMSG_CFG_SRV = IMSG_PROC_MAX,
 	IMSG_CFG_SOCK,
@@ -404,6 +407,7 @@ enum query_actions {
 enum query_actions {
 	BLAME,
 	BLOB,
+	BLOBRAW,
 	BRIEFS,
 	COMMITS,
 	DIFF,
@@ -462,6 +466,7 @@ int	gotweb_render_rss(struct template *);
 int	gotweb_render_briefs(struct template *);
 int	gotweb_render_navs(struct template *);
 int	gotweb_render_commits(struct template *);
+int	gotweb_render_blob(struct template *, struct got_blob_object *);
 int	gotweb_render_rss(struct template *);
 
 /* parse.y */
@@ -491,7 +496,11 @@ const struct got_error *got_output_file_blob(struct re
 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_output_repo_tree(struct request *);
+const struct got_error *got_open_blob_for_output(struct got_blob_object **,
+    int *, int *, struct request *);
 const struct got_error *got_output_file_blob(struct request *);
+int got_output_blob_by_lines(struct template *, struct got_blob_object *,
+    int (*)(struct template *, const char *, size_t));
 const struct got_error *got_output_file_blame(struct request *);
 
 /* config.c */
blob - 31d9c2b76e13b67d88a33db6d31ba3ce49398422
blob + 5dac668e8a6c3b5de586bc8484746a6397e29dfb
--- gotwebd/pages.tmpl
+++ gotwebd/pages.tmpl
@@ -31,6 +31,8 @@ static inline int rss_tag_item(struct template *, stru
 #include "gotwebd.h"
 #include "tmpl.h"
 
+static int gotweb_render_blob_line(struct template *, const char *, size_t);
+
 static inline int rss_tag_item(struct template *, struct repo_tag *);
 static inline int rss_author(struct template *, char *);
 
@@ -406,6 +408,54 @@ gotweb_render_age(struct template *tp, time_t time, in
 </div>
 {{ end }}
 
+{{ define gotweb_render_blob(struct template *tp,
+    struct got_blob_object *blob) }}
+{!
+	struct request		*c = tp->tp_arg;
+	struct transport	*t = c->t;
+	struct repo_commit	*rc = TAILQ_FIRST(&t->repo_commits);
+!}
+<div id="blob_title_wrapper">
+  <div id="blob_title">Blob</div>
+</div>
+<div id="blob_content">
+  <div id="blob_header_wrapper">
+    <div id="blob_header">
+      <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="blob">
+    <pre>
+      {{ render got_output_blob_by_lines(tp, blob, gotweb_render_blob_line) }}
+    </pre>
+  </div>
+</div>
+{{ end }}
+
+{{ define gotweb_render_blob_line(struct template *tp, const char *line,
+    size_t no) }}
+{!
+	char		 lineno[16];
+	int		 r;
+
+	r = snprintf(lineno, sizeof(lineno), "%zu", no);
+	if (r < 0 || (size_t)r >= sizeof(lineno))
+		return -1;
+!}
+<div class="blob_line" id="line{{ lineno }}">
+  <div class="blob_number">
+    <a href="#line{{ lineno }}">{{ lineno }}</a>
+  </div>
+  <div class="blob_code">{{ line }}</div>
+</div>
+{{ end }}
+
 {{ define gotweb_render_rss(struct template *tp) }}
 {!
 	struct request		*c = tp->tp_arg;
blob - 34c1b6df1b13b23dea3ca12f6a6cdaf82e3106f4
blob + 9d11c8f4f518f5f911295671d3aafcb494a7ec10
--- include/got_object.h
+++ include/got_object.h
@@ -298,6 +298,12 @@ const struct got_error *got_object_blob_is_binary(int 
     struct got_blob_object *);
 
 /*
+ * getline(3) for blobs.
+ */
+const struct got_error *got_object_blob_getline(char **, ssize_t *,
+    size_t *, struct got_blob_object *);
+
+/*
  * Read the entire content of a blob and write it to the specified file.
  * Flush and rewind the file as well. Indicate the amount of bytes
  * written in the size_t output argument, and the number of lines in the
blob - 4d1c1cdecf129d5464dd74b1c503a6164e6939b5
blob + 9834b049e72a099055a8c44dc2b896abe7ad2ec3
--- lib/object.c
+++ lib/object.c
@@ -425,6 +425,16 @@ got_object_blob_dump_to_file(off_t *filesize, int *nli
 }
 
 const struct got_error *
+got_object_blob_getline(char **line, ssize_t *linelen, size_t *linesize,
+    struct got_blob_object *blob)
+{
+	*linelen = getline(line, linesize, blob->f);
+	if (*linelen == -1 && !feof(blob->f))
+		return got_error_from_errno("getline");
+	return NULL;
+}
+
+const struct got_error *
 got_object_blob_dump_to_file(off_t *filesize, int *nlines,
     off_t **line_offsets, FILE *outfile, struct got_blob_object *blob)
 {