From: Mark Jamsek <mark@jamsek.com>
Subject: Re: gotwebd: templatefy gotweb_render_blame
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Sun, 15 Jan 2023 16:26:42 +1100

On 23-01-13 09:47AM, Omar Polo wrote:
> finally, at least :)

Nice :)

> Had to move `struct blame_line' to the header for it to be visible and
> typedef the callback because was painful to type.
> 
> no functional change intended.
> 
> ok?

ok with or without the following tiny style nit.

Please consider prefixing the `render_blame_line_fn` typedef with
"got_", and using the existing "_cb" suffix convention for callbacks:

typedef int (*got_render_blame_line_cb)(struct template *, const char *,
    struct blame_line *, int, int);

> diff /home/op/w/got
> commit - 69525b4e7234cc5d81905e84a67d9b1768606e34
> path + /home/op/w/got
> blob - 6ee4848f094c2566d8233b5aefa8fbbaabc158e9
> file + gotwebd/got_operations.c
> --- gotwebd/got_operations.c
> +++ gotwebd/got_operations.c
> @@ -1027,13 +1027,6 @@ struct blame_line {
>  	return 0;
>  }
>  
> -struct blame_line {
> -	int annotated;
> -	char *id_str;
> -	char *committer;
> -	char datebuf[11]; /* YYYY-MM-DD + NUL */
> -};
> -
>  struct blame_cb_args {
>  	struct blame_line *lines;
>  	int nlines;
> @@ -1043,6 +1036,7 @@ struct blame_cb_args {
>  	FILE *f;
>  	struct got_repository *repo;
>  	struct request *c;
> +	render_blame_line_fn cb;
>  };
>  
>  static const struct got_error *
> @@ -1053,14 +1047,11 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
>  	struct blame_cb_args *a = arg;
>  	struct blame_line *bline;
>  	struct request *c = a->c;
> -	struct transport *t = c->t;
> -	struct repo_dir *repo_dir = t->repo_dir;
> -	char *line = NULL, *eline = NULL;
> +	char *line = NULL;
>  	size_t linesize = 0;
>  	off_t offset;
>  	struct tm tm;
>  	time_t committer_time;
> -	int r;
>  
>  	if (nlines != a->nlines ||
>  	    (lineno != -1 && lineno < 1) || lineno > a->nlines)
> @@ -1105,76 +1096,26 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
>  	}
>  
>  	while (a->lineno_cur <= a->nlines && bline->annotated) {
> -		char *smallerthan, *at, *nl, *committer;
> -		size_t len;
> -
>  		if (getline(&line, &linesize, a->f) == -1) {
>  			if (ferror(a->f))
>  				err = got_error_from_errno("getline");
>  			break;
>  		}
>  
> -		committer = bline->committer;
> -		smallerthan = strchr(committer, '<');
> -		if (smallerthan && smallerthan[1] != '\0')
> -			committer = smallerthan + 1;
> -		at = strchr(committer, '@');
> -		if (at)
> -			*at = '\0';
> -		len = strlen(committer);
> -		if (len >= 9)
> -			committer[8] = '\0';
> +		if (a->cb(c->tp, line, bline, a->nlines_prec,
> +		    a->lineno_cur) == -1)
> +			break;
>  
> -		nl = strchr(line, '\n');
> -		if (nl)
> -			*nl = '\0';
> -
> -		err = gotweb_escape_html(&eline, line);
> -		if (err)
> -			goto done;
> -
> -		if (fcgi_printf(c, "<div class='blame_wrapper'>"
> -		    "<div class='blame_number'>%.*d</div>"
> -		    "<div class='blame_hash'>",
> -		    a->nlines_prec, a->lineno_cur) == -1)
> -			goto done;
> -
> -		r = gotweb_link(c, &(struct gotweb_url){
> -			.action = DIFF,
> -			.index_page = -1,
> -			.page = -1,
> -			.path = repo_dir->name,
> -			.commit = bline->id_str,
> -		    }, "%.8s", bline->id_str);
> -		if (r == -1)
> -			goto done;
> -
> -		r = fcgi_printf(c,
> -		    "</div>"
> -		    "<div class='blame_date'>%s</div>"
> -		    "<div class='blame_author'>%s</div>"
> -		    "<div class='blame_code'>%s</div>"
> -		    "</div>",	/* .blame_wrapper */
> -		    bline->datebuf,
> -		    committer,
> -		    eline);
> -		if (r == -1)
> -			goto done;
> -
>  		a->lineno_cur++;
>  		bline = &a->lines[a->lineno_cur - 1];
> -
> -		free(eline);
> -		eline = NULL;
>  	}
>  done:
>  	free(line);
> -	free(eline);
>  	return err;
>  }
>  
>  const struct got_error *
> -got_output_file_blame(struct request *c)
> +got_output_file_blame(struct request *c, render_blame_line_fn cb)
>  {
>  	const struct got_error *error = NULL;
>  	struct transport *t = c->t;
> @@ -1194,6 +1135,7 @@ got_output_file_blame(struct request *c)
>  	TAILQ_INIT(&refs);
>  	bca.f = NULL;
>  	bca.lines = NULL;
> +	bca.cb = cb;
>  
>  	if (asprintf(&path, "%s%s%s", qs->folder ? qs->folder : "",
>  	    qs->folder ? "/" : "", qs->file) == -1) {
> blob - dadd1ae79464caf377f53bb82de16886cdeb9df9
> file + gotwebd/gotweb.c
> --- gotwebd/gotweb.c
> +++ gotwebd/gotweb.c
> @@ -93,7 +93,6 @@ static const struct got_error *gotweb_render_blame(str
>      struct server *, const char *, int);
>  static const struct got_error *gotweb_get_clone_url(char **, struct server *,
>      const char *, int);
> -static const struct got_error *gotweb_render_blame(struct request *);
>  
>  static void gotweb_free_querystring(struct querystring *);
>  static void gotweb_free_repo_dir(struct repo_dir *);
> @@ -245,11 +244,13 @@ render:
>  
>  	switch(qs->action) {
>  	case BLAME:
> -		error = gotweb_render_blame(c);
> +		error = got_get_repo_commits(c, 1);
>  		if (error) {
>  			log_warnx("%s: %s", __func__, error->msg);
>  			goto err;
>  		}
> +		if (gotweb_render_blame(c->tp) == -1)
> +			goto done;
>  		break;
>  	case BLOB:
>  		if (gotweb_render_blob(c->tp, blob) == -1)
> @@ -974,59 +975,6 @@ static const struct got_error *
>  	return error;
>  }
>  
> -static const struct got_error *
> -gotweb_render_blame(struct request *c)
> -{
> -	const struct got_error *error = NULL;
> -	struct transport *t = c->t;
> -	struct repo_commit *rc = NULL;
> -	char *age = NULL, *msg = NULL;
> -	int r;
> -
> -	error = got_get_repo_commits(c, 1);
> -	if (error)
> -		return error;
> -
> -	rc = TAILQ_FIRST(&t->repo_commits);
> -
> -	error = gotweb_get_time_str(&age, rc->committer_time, TM_LONG);
> -	if (error)
> -		goto done;
> -	error = gotweb_escape_html(&msg, rc->commit_msg);
> -	if (error)
> -		goto done;
> -
> -	r = fcgi_printf(c, "<div id='blame_title_wrapper'>\n"
> -	    "<div id='blame_title'>Blame</div>\n"
> -	    "</div>\n"		/* #blame_title_wrapper */
> -	    "<div id='blame_content'>\n"
> -	    "<div id='blame_header_wrapper'>\n"
> -	    "<div id='blame_header'>\n"
> -	    "<div class='header_age_title'>Date:</div>\n"
> -	    "<div class='header_age'>%s</div>\n"
> -	    "<div id='header_commit_msg_title'>Message:</div>\n"
> -	    "<div id='header_commit_msg'>%s</div>\n"
> -	    "</div>\n"		/* #blame_header */
> -	    "</div>\n"		/* #blame_header_wrapper */
> -	    "<div class='dotted_line'></div>\n"
> -	    "<div id='blame'>\n",
> -	    age,
> -	    msg);
> -	if (r == -1)
> -		goto done;
> -
> -	error = got_output_file_blame(c);
> -	if (error)
> -		goto done;
> -
> -	fcgi_printf(c, "</div>\n"	/* #blame */
> -	    "</div>\n");		/* #blame_content */
> -done:
> -	free(age);
> -	free(msg);
> -	return error;
> -}
> -
>  const struct got_error *
>  gotweb_escape_html(char **escaped_html, const char *orig_html)
>  {
> blob - 2b26cfe7719c3bdd3462b9c0934f59fbcaac5aed
> file + gotwebd/gotwebd.h
> --- gotwebd/gotwebd.h
> +++ gotwebd/gotwebd.h
> @@ -143,6 +143,13 @@ struct repo_dir {
>  	uint8_t		reserved;
>  }__attribute__((__packed__));
>  
> +struct blame_line {
> +	int		 annotated;
> +	char		*id_str;
> +	char		*committer;
> +	char		 datebuf[11]; /* YYYY-MM-DD + NUL */
> +};
> +
>  struct repo_dir {
>  	char			*name;
>  	char			*owner;
> @@ -431,6 +438,9 @@ extern struct gotwebd	*gotwebd_env;
>  
>  extern struct gotwebd	*gotwebd_env;
>  
> +typedef int (*render_blame_line_fn)(struct template *, const char *,
> +    struct blame_line *, int, int);
> +
>  /* sockets.c */
>  void sockets(struct privsep *, struct privsep_proc *);
>  void sockets_shutdown(void);
> @@ -475,6 +485,7 @@ int	gotweb_render_rss(struct template *);
>  int	gotweb_render_diff(struct template *, FILE *);
>  int	gotweb_render_branches(struct template *, struct got_reflist_head *);
>  int	gotweb_render_summary(struct template *, struct got_reflist_head *);
> +int	gotweb_render_blame(struct template *);
>  int	gotweb_render_rss(struct template *);
>  
>  /* parse.y */
> @@ -512,7 +523,8 @@ const struct got_error *got_output_file_blame(struct r
>  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 *);
> +const struct got_error *got_output_file_blame(struct request *,
> +    render_blame_line_fn);
>  
>  /* config.c */
>  int config_setserver(struct gotwebd *, struct server *);
> blob - 4ed5a74cc4f6b80c911b880ef2f0a628eed248c0
> file + gotwebd/pages.tmpl
> --- gotwebd/pages.tmpl
> +++ gotwebd/pages.tmpl
> @@ -40,6 +40,8 @@ static int gotweb_render_tree_item(struct template *, 
>  
>  static int gotweb_render_blob_line(struct template *, const char *, size_t);
>  static int gotweb_render_tree_item(struct template *, struct got_tree_entry *);
> +static int blame_line(struct template *, const char *, struct blame_line *,
> +    int, int);
>  
>  static inline int diff_line(struct template *, char *);
>  static inline int tag_item(struct template *, struct repo_tag *);
> @@ -870,6 +872,77 @@ gotweb_render_age(struct template *tp, time_t time, in
>  {{ render gotweb_render_branches(tp, refs) }}
>  {{ end }}
>  
> +{{ define gotweb_render_blame(struct template *tp) }}
> +{!
> +	const struct got_error	*err;
> +	struct request		*c = tp->tp_arg;
> +	struct transport	*t = c->t;
> +	struct repo_commit	*rc = TAILQ_FIRST(&t->repo_commits);
> +!}
> +<div id="blame_title_wrapper">
> +  <div id="blame_title">Blame</div>
> +</div>
> +<div id="blame_content">
> +  <div id="blame_header_wrapper">
> +    <div id="blame_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="blame">
> +    {{ "\n" }}
> +    {!
> +	err = got_output_file_blame(c, &blame_line);
> +	if (err) {
> +		log_warnx("%s: got_output_file_blame: %s", __func__,
> +		    err->msg);
> +		return (-1);
> +	}
> +    !}
> +  </div>
> +</div>
> +{{ end }}
> +
> +{{ define blame_line(struct template *tp, const char *line,
> +    struct blame_line *bline, int lprec, int lcur) }}
> +{!
> +	struct request		*c = tp->tp_arg;
> +	struct transport	*t = c->t;
> +	struct repo_dir		*repo_dir = t->repo_dir;
> +	char			*committer, *s;
> +	struct gotweb_url	 url = {
> +		.action = DIFF,
> +		.index_page = -1,
> +		.page = -1,
> +		.path = repo_dir->name,
> +		.commit = bline->id_str,
> +	};
> +
> +	s = strchr(bline->committer, '<');
> +	committer = s ? s + 1 : bline->committer;
> +
> +	s = strchr(committer, '@');
> +	if (s)
> +		*s = '\0';
> +!}
> +<div class="blame_wrapper">
> +  <div class="blame_number">{{ printf "%.*d", lprec, lcur }}</div>
> +  <div class="blame_hash">
> +    <a href="{{ render gotweb_render_url(c, &url) }}">
> +      {{ printf "%.8s", bline->id_str }}
> +    </a>
> +  </div>
> +  <div class="blame_date">{{ bline->datebuf }}</div>
> +  <div class="blame_author">{{ printf "%.9s", committer }}</div>
> +  <div class="blame_code">{{ line }}</div>
> +</div>
> +{{ end }}
> +
>  {{ define gotweb_render_rss(struct template *tp) }}
>  {!
>  	struct request		*c = tp->tp_arg;
> 

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