From: Omar Polo Subject: gotwebd: fix briefs/tags navigation overlap To: gameoftrees@openbsd.org Date: Wed, 01 Feb 2023 00:32:55 +0100 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 *
{{ end }} - {{ if t->next_id || t->prev_id }} - {{ render gotweb_render_navs(tp) }} - {{ end }} + {{ render gotweb_render_more(tp, BRIEFS) }} {{ 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 }} +
+ +
+ {{ 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 *
{{ end }} - {{ if t->next_id || t->prev_id }} - {{ render gotweb_render_navs(tp) }} - {{ end }} + {{ render gotweb_render_more(tp, COMMITS) }} {{ 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 }}