[PATCH v2 12/15] ui-tree: render any matching README file in tree view

[john at keeping.me.uk (John Keeping)]

On Mon, Jun 18, 2018 at 10:58:15AM +0800, Andy Green wrote:
> While listing the items in tree view, we collect a list
> of any filenames that match any tree-readme entries from the
> config file.
> 
> After the tree view has been shown, we iterate through any
> collected readme files rendering them inline.
> 
> Signed-off-by: Andy Green <andy at warmcat.com>

A couple of minor style points below, but this looks good.  With or
without the style changes:

Reviewed-by: John Keeping <john at keeping.me.uk>

> ---
>  ui-tree.c |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/ui-tree.c b/ui-tree.c
> index 368cdc5..9f59d18 100644
> --- a/ui-tree.c
> +++ b/ui-tree.c
> @@ -1,6 +1,6 @@
>  /* ui-tree.c: functions for tree output
>   *
> - * Copyright (C) 2006-2017 cgit Development Team <cgit at lists.zx2c4.com>
> + * Copyright (C) 2006-2018 cgit Development Team <cgit at lists.zx2c4.com>
>   *
>   * Licensed under GNU General Public License v2
>   *   (see COPYING for full license text)
> @@ -12,8 +12,10 @@
>  #include "ui-shared.h"
>  
>  struct walk_tree_context {
> +	struct object_id inline_oid;
>  	char *curr_rev;
>  	char *match_path;
> +	char *inline_path;
>  	int state;
>  	bool use_render;
>  };
> @@ -326,11 +328,19 @@ static int ls_item(const struct object_id *oid, struct strbuf *base,
>  				&fullpath);
>  	} else {
>  		char *ext = strrchr(name, '.');
> +
>  		strbuf_addstr(&class, "ls-blob");
>  		if (ext)
>  			strbuf_addf(&class, " %s", ext + 1);
> +
>  		cgit_tree_link(name, NULL, class.buf, ctx.qry.head,
>  			       walk_tree_ctx->curr_rev, fullpath.buf);
> +
> +		if (!walk_tree_ctx->inline_path &&
> +		    string_list_has_string(&ctx.repo->inline_readme, name)) {
> +			walk_tree_ctx->inline_path = xstrdup(pathname);
> +			oidcpy(&walk_tree_ctx->inline_oid, oid);
> +		}
>  	}
>  	htmlf("</td><td class='ls-size'>%li</td>", size);
>  
> @@ -368,7 +378,53 @@ static void ls_head(void)
>  
>  static void ls_tail(struct walk_tree_context *walk_tree_ctx)
>  {
> +	struct cgit_filter *render;
> +	enum object_type type;
> +	char *buf, *mimetype, *name;
> +	unsigned long size;
> +
>  	html("</table>\n");
> +
> +	if (!walk_tree_ctx->inline_path)
> +		goto done;
> +
> +	type = oid_object_info(the_repository, &walk_tree_ctx->inline_oid, &size);
> +	if (type == OBJ_BAD)
> +		goto done;
> +
> +	buf = read_object_file(&walk_tree_ctx->inline_oid, &type, &size);
> +	if (!buf)
> +		goto done;
> +
> +	/* create a vertical gap between tree nav / inline */
> +	html("<table class=\"tabs\"><tr><td></td></tr></table>");
> +
> +	render = get_render_for_filename(walk_tree_ctx->inline_path);
> +	mimetype = render ? NULL : get_mimetype_for_filename(
> +				walk_tree_ctx->inline_path);
> +
> +	name = strrchr(walk_tree_ctx->inline_path, '/');

Isn't this impossible?   inline_path is a filename at a single level of
the tree and Git forbids directory separators there.

> +	if (name)
> +		name++;
> +	else
> +		name = walk_tree_ctx->inline_path;
> +
> +	htmlf("<h2>%s</h2>", name);
> +	html("<div class=blob>&nbsp;</div>\n");
> +
> +	if (render || mimetype) {
> +		if (render)
> +			render_buffer(render, name, buf, size);
> +		else
> +			include_file(walk_tree_ctx->inline_path, mimetype);

We can lose a level of indentation here by writing it as:

	if (render)
		...
	else if (mimetype)
		...
	else
		...

> +	} else {
> +		print_buffer(name, buf, size);
> +	}
> +
> +	free(mimetype);
> +	free(buf);
> +
> +done:
>  	cgit_print_layout_end();
>  }
>  
> @@ -479,4 +535,6 @@ void cgit_print_tree(const char *rev, char *path, bool use_render)
>  
>  cleanup:
>  	free(walk_tree_ctx.curr_rev);
> +	if (walk_tree_ctx.inline_path)
> +		free(walk_tree_ctx.inline_path);
>  }