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

From:
Mark Jamsek <mark@jamsek.com>
Subject:
Re: gotwebd: fix briefs/tags navigation overlap
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Wed, 1 Feb 2023 13:40:43 +1100

Download raw body.

Thread
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:

        <a href="{{ render gotweb_render_url(c, &more) }}">
          More &nbsp;&darr;
        </a>

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 *
>      </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 }}
> 

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68