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

From:
Omar Polo <op@omarpolo.com>
Subject:
gotwebd: fix briefs/tags navigation overlap
To:
gameoftrees@openbsd.org
Date:
Wed, 01 Feb 2023 00:32:55 +0100

Download raw body.

Thread
When I started to templateify everything I actually introduced a error
in the pagination handling. I failed to note that the `next_id' and
`prev_id' were *shared* by tags and commits/briefs, and this is a
problem for the summary page. got_get_repo_commits/tags overwrite each'
other next/prev and so we leak a bit of memory *and* fail to provide the
tab navigation in the SUMMARY page.

So, now that we have dropped the "prev" button for commits, let's fix
the situation and actually improve it a little bit.

Diff below introduces a separate field for the "next" button (now called
"More" -- was discussed a bit on IRC and the gist I got was that tracey
liked it) and adjusted the CSS/HTML so it's used. Finally, drops the old
code used to handle the pagination for the BRIEFS/COMMITS cases.

Needs a small hack, see the last chunk where I'm setting the action
to TAGS. This is due how stuff is processed, will try to simplify the
handling of qs->action in the future.

I have this on my instance and this time I've actually remembered to
test the COMMITS page too! ;-)

ok?

diff /tmp/got
commit - 2aea6fc2c2c95ba4968b598247e00edb6f91048b
path + /tmp/got
blob - c8f7a859f74578199b84566a25e7a1b31a3717d9
file + gotwebd/files/htdocs/gotwebd/gotweb.css
--- gotwebd/files/htdocs/gotwebd/gotweb.css
+++ gotwebd/files/htdocs/gotwebd/gotweb.css
@@ -117,6 +117,10 @@ body {
 	background-color: #f5fcfb;
 	overflow: hidden;
 }
+#nav_more {
+	padding: 5px 0;
+	text-align: center;
+}
 #nav_prev {
 	float: left;
 	padding-left: 10px;
blob - e22d214bd2d3e68c26a5394cc271c0a6e204a5aa
file + gotwebd/got_operations.c
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -418,8 +418,8 @@ got_get_repo_commits(struct request *c, int limit)
 			 */
 			if (chk_next && (qs->action == BRIEFS ||
 			    qs->action == COMMITS || qs->action == SUMMARY)) {
-				t->next_id = strdup(repo_commit->commit_id);
-				if (t->next_id == NULL) {
+				t->more_id = strdup(repo_commit->commit_id);
+				if (t->more_id == NULL) {
 					error = got_error_from_errno("strdup");
 					goto done;
 				}
blob - de3c351cbe1ced66e0a1e55b0df7afae4fe2b894
file + gotwebd/gotweb.c
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -782,6 +782,7 @@ gotweb_free_transport(struct transport *t)
 	}
 	gotweb_free_repo_dir(t->repo_dir);
 	gotweb_free_querystring(t->qs);
+	free(t->more_id);
 	free(t->next_id);
 	free(t->prev_id);
 	free(t);
@@ -818,60 +819,6 @@ gotweb_get_navs(struct request *c, struct gotweb_url *
 			};
 		}
 		break;
-	case BRIEFS:
-		if (t->prev_id && qs->commit != NULL &&
-		    strcmp(qs->commit, t->prev_id) != 0) {
-			*have_prev = 1;
-			*prev = (struct gotweb_url){
-				.action = BRIEFS,
-				.index_page = -1,
-				.page = qs->page - 1,
-				.path = qs->path,
-				.commit = t->prev_id,
-				.headref = qs->headref,
-			};
-		}
-		if (t->next_id) {
-			*have_next = 1;
-			*next = (struct gotweb_url){
-				.action = BRIEFS,
-				.index_page = -1,
-				.page = qs->page + 1,
-				.path = qs->path,
-				.commit = t->next_id,
-				.headref = qs->headref,
-			};
-		}
-		break;
-	case COMMITS:
-		if (t->prev_id && qs->commit != NULL &&
-		    strcmp(qs->commit, t->prev_id) != 0) {
-			*have_prev = 1;
-			*prev = (struct gotweb_url){
-				.action = COMMITS,
-				.index_page = -1,
-				.page = qs->page - 1,
-				.path = qs->path,
-				.commit = t->prev_id,
-				.headref = qs->headref,
-				.folder = qs->folder,
-				.file = qs->file,
-			};
-		}
-		if (t->next_id) {
-			*have_next = 1;
-			*next = (struct gotweb_url){
-				.action = COMMITS,
-				.index_page = -1,
-				.page = qs->page + 1,
-				.path = qs->path,
-				.commit = t->next_id,
-				.headref = qs->headref,
-				.folder = qs->folder,
-				.file = qs->file,
-			};
-		}
-		break;
 	case TAGS:
 		if (t->prev_id && qs->commit != NULL &&
 		    strcmp(qs->commit, t->prev_id) != 0) {
blob - b84fe9526fa6e65fe446b6bdf8b85d2b63e73233
file + gotwebd/gotwebd.h
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -189,6 +189,7 @@ struct transport {
 	struct got_repository	*repo;
 	struct repo_dir		*repo_dir;
 	struct querystring	*qs;
+	char			*more_id;
 	char			*next_id;
 	char			*prev_id;
 	unsigned int		 repos_total;
blob - bd93fb21ec16695dbf29784ea4a8d2de3efdecac
file + gotwebd/pages.tmpl
--- gotwebd/pages.tmpl
+++ gotwebd/pages.tmpl
@@ -43,6 +43,8 @@ static inline int diff_line(struct template *, char *)
 static int blame_line(struct template *, const char *, struct blame_line *,
     int, int);

+static inline int gotweb_render_more(struct template *, int);
+
 static inline int diff_line(struct template *, char *);
 static inline int tag_item(struct template *, struct repo_tag *);
 static inline int branch(struct template *, struct got_reflist_entry *);
@@ -304,12 +306,34 @@ static inline int rss_author(struct template *, char *
     </div>
     <div class="dotted_line"></div>
   {{ end }}
-  {{ if t->next_id || t->prev_id }}
-    {{ render gotweb_render_navs(tp) }}
-  {{ end }}
+  {{ render gotweb_render_more(tp, BRIEFS) }}
 </div>
 {{ end }}

+{{ define gotweb_render_more(struct template *tp, int action) }}
+{!
+	struct request		*c = tp->tp_arg;
+	struct transport	*t = c->t;
+	struct querystring	*qs = t->qs;
+	struct gotweb_url	 more = {
+		.action = action,
+		.index_page = -1,
+		.path = qs->path,
+		.commit = t->more_id,
+		.headref = qs->headref,
+	};
+!}
+  {{ if t->more_id }}
+    <div id="np_wrapper">
+      <div id="nav_more">
+        <a href="{{ render gotweb_render_url(c, &more) }}">
+          More
+        </a>
+      </div>
+    </div>
+  {{ end }}
+{{ end }}
+
 {{ define gotweb_render_navs(struct template *tp) }}
 {!
 	struct request		*c = tp->tp_arg;
@@ -404,9 +428,7 @@ static inline int rss_author(struct template *, char *
     </div>
     <div class="dotted_line"></div>
   {{ end }}
-  {{ if t->next_id || t->prev_id }}
-    {{ render gotweb_render_navs(tp) }}
-  {{ end }}
+  {{ render gotweb_render_more(tp, COMMITS) }}
 </div>
 {{ end }}

@@ -590,6 +612,7 @@ static inline int rss_author(struct template *, char *
       {{ end }}
     {{ end }}
     {{ if t->next_id || t->prev_id }}
+      {! qs->action = TAGS; !}
       {{ render gotweb_render_navs(tp) }}
     {{ end }}
   {{ end }}