From: Mark Jamsek Subject: Re: gotwebd: fix briefs/tags navigation overlap To: Omar Polo Cc: gameoftrees@openbsd.org Date: Wed, 1 Feb 2023 13:40:43 +1100 On 23-02-01 12:32AM, Omar Polo wrote: > 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? ok Just a suggestion to think about but please disregard if you don't like it... perhaps keep "More" left-aligned: #nav_more { padding: 5px 0; padding-left: 10px; } and (this I'm sure you won't like :), maybe add a down arrow (stolen from how Fossil does this to the timeline/log web UI) to the "More" link: More  ↓ You can see how this looks here: http://jamsek.net/?action=summary&path=got.git > 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 }} > -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68