[PATCH] ui-repolist: Allow sections to be collapsible

John Keeping john at keeping.me.uk
Thu Aug 25 23:13:50 CEST 2016


On Tue, Aug 09, 2016 at 04:53:07PM -0500, Andy Doan wrote:
> The index page can be difficult to navigate for really large git
> servers. This change allows a configuration like:
> 
>  section-collapse=people
>  section-collapse=tests
> 
> And an index page would only display the "people" and "tests" section
> headers entries (not their repos) with a hyperlink that can be used to
> drill down into each section.
> 
> Signed-off-by: Andy Doan <andy.doan at linaro.org>
> ---
>  cgit.c        | 33 +++++++++++++++++++++++++++++----
>  cgit.h        | 11 +++++++++--
>  cgitrc.5.txt  |  5 +++++
>  scan-tree.c   |  6 +++---
>  shared.c      |  5 ++++-
>  ui-repolist.c | 29 ++++++++++++++++-------------
>  6 files changed, 66 insertions(+), 23 deletions(-)
> 
> diff --git a/cgit.c b/cgit.c
> index 9427c4a..477c920 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -19,6 +19,12 @@
> 
>  const char *cgit_version = CGIT_VERSION;
> 
> +struct cgit_section_list {
> +	struct cgit_section section;
> +	struct cgit_section_list *next;
> +};
> +static struct cgit_section_list *sections = NULL;

Should this list of sections live in ctx somewhere?

I also think it would be simpler to just add:

	struct cgit_section *next;

into the struct cgit_section and avoid this second level of wrapping.

> +
>  static void add_mimetype(const char *name, const char *value)
>  {
>  	struct string_list_item *item;
> @@ -77,7 +83,7 @@ static void repo_config(struct cgit_repo *repo, const char *name, const char *va
>  		item = string_list_append(&repo->submodules, xstrdup(name + 12));
>  		item->util = xstrdup(value);
>  	} else if (!strcmp(name, "section"))
> -		repo->section = xstrdup(value);
> +		repo->section = get_or_create_section(value);
>  	else if (!strcmp(name, "readme") && value != NULL) {
>  		if (repo->readme.items == ctx.cfg.readme.items)
>  			memset(&repo->readme, 0, sizeof(repo->readme));
> @@ -107,7 +113,7 @@ static void repo_config(struct cgit_repo *repo, const char *name, const char *va
>  static void config_cb(const char *name, const char *value)
>  {
>  	if (!strcmp(name, "section") || !strcmp(name, "repo.group"))
> -		ctx.cfg.section = xstrdup(value);
> +		ctx.cfg.section = get_or_create_section(value);
>  	else if (!strcmp(name, "repo.url"))
>  		ctx.repo = cgit_add_repo(value);
>  	else if (ctx.repo && !strcmp(name, "repo.path"))
> @@ -242,6 +248,8 @@ static void config_cb(const char *name, const char *value)
>  		ctx.cfg.section_from_path = atoi(value);
>  	else if (!strcmp(name, "repository-sort"))
>  		ctx.cfg.repository_sort = xstrdup(value);
> +	else if (!strcmp(name, "section-collapse"))
> +		get_or_create_section(value)->collapse = 1;
>  	else if (!strcmp(name, "section-sort"))
>  		ctx.cfg.section_sort = atoi(value);
>  	else if (!strcmp(name, "source-filter"))
> @@ -385,7 +393,7 @@ static void prepare_context(void)
>  	ctx.cfg.root_desc = "a fast webinterface for the git dscm";
>  	ctx.cfg.scan_hidden_path = 0;
>  	ctx.cfg.script_name = CGIT_SCRIPT_NAME;
> -	ctx.cfg.section = "";
> +	ctx.cfg.section = NULL;
>  	ctx.cfg.repository_sort = "name";
>  	ctx.cfg.section_sort = 1;
>  	ctx.cfg.summary_branches = 10;
> @@ -794,7 +802,7 @@ static void print_repo(FILE *f, struct cgit_repo *repo)
>  	if (repo->module_link)
>  		fprintf(f, "repo.module-link=%s\n", repo->module_link);
>  	if (repo->section)
> -		fprintf(f, "repo.section=%s\n", repo->section);
> +		fprintf(f, "repo.section=%s\n", repo->section->name);
>  	if (repo->homepage)
>  		fprintf(f, "repo.homepage=%s\n", repo->homepage);
>  	if (repo->clone_url)
> @@ -1026,6 +1034,23 @@ static int calc_ttl(void)
>  	return ctx.cfg.cache_repo_ttl;
>  }
> 
> +struct cgit_section* get_or_create_section(const char *section)
> +{
> +	struct cgit_section_list *ptr = sections;
> +	while(ptr) {
> +		if (!strcmp(section, ptr->section.name))
> +			return &ptr->section;
> +		ptr = ptr->next;
> +	}
> +	/* Not found insert into head of list */

Neat.  We optimize the common case of finding the same section twice in
a row.

> +	ptr = malloc(sizeof(struct cgit_section_list));

This should be xmalloc.  And it might be better to use sizeof(*ptr).

> +	ptr->section.name = xstrdup(section);
> +	ptr->section.collapse = 0;
> +	ptr->next = sections;
> +	sections = ptr;
> +	return &ptr->section;
> +}
> +
>  int main(int argc, const char **argv)
>  {
>  	const char *path;
> diff --git a/cgit.h b/cgit.h
> index 325432b..f334bb3 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -75,6 +75,11 @@ struct cgit_exec_filter {
>  	int pid;
>  };
> 
> +struct cgit_section {
> +	unsigned int collapse : 1;
> +	char *name;
> +};
> +
>  struct cgit_repo {
>  	char *url;
>  	char *name;
> @@ -85,7 +90,7 @@ struct cgit_repo {
>  	char *defbranch;
>  	char *module_link;
>  	struct string_list readme;
> -	char *section;
> +	struct cgit_section *section;
>  	char *clone_url;
>  	char *logo;
>  	char *logo_link;
> @@ -208,7 +213,7 @@ struct cgit_config {
>  	char *root_desc;
>  	char *root_readme;
>  	char *script_name;
> -	char *section;
> +	struct cgit_section *section;
>  	char *repository_sort;
>  	char *virtual_root;	/* Always ends with '/'. */
>  	char *strict_export;
> @@ -322,6 +327,8 @@ extern struct cgit_repolist cgit_repolist;
>  extern struct cgit_context ctx;
>  extern const struct cgit_snapshot_format cgit_snapshot_formats[];
> 
> +extern struct cgit_section* get_or_create_section(const char *section);
> +
>  extern char *cgit_default_repo_desc;
>  extern struct cgit_repo *cgit_add_repo(const char *url);
>  extern struct cgit_repo *cgit_get_repoinfo(const char *url);
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 9fcf445..2762657 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -404,6 +404,11 @@ section::
>  	after this option will inherit the current section name. Default value:
>  	none.
> 
> +section-collapse::
> +	Name of a section to "collapse" and not display on the index page.
> +	Multiple config entries can be specified and each one will be
> +	collapsed.
> +
>  section-sort::
>  	Flag which, when set to "1", will sort the sections on the repository
>  	listing by name. Set this flag to "0" if the order in the cgitrc file should
> diff --git a/scan-tree.c b/scan-tree.c
> index 1cb4e5d..fa21fc4 100644
> --- a/scan-tree.c
> +++ b/scan-tree.c
> @@ -164,10 +164,10 @@ static void add_repo(const char *base, struct strbuf *path, repo_config_fn fn)
>  		}
>  		if (slash && !n) {
>  			*slash = '\0';
> -			repo->section = xstrdup(rel.buf);
> +			repo->section = get_or_create_section(rel.buf);
>  			*slash = '/';
> -			if (starts_with(repo->name, repo->section)) {
> -				repo->name += strlen(repo->section);
> +			if (starts_with(repo->name, repo->section->name)) {
> +				repo->name += strlen(repo->section->name);
>  				if (*repo->name == '/')
>  					repo->name++;
>  			}
> diff --git a/shared.c b/shared.c
> index a63633b..8348581 100644
> --- a/shared.c
> +++ b/shared.c
> @@ -444,13 +444,16 @@ typedef struct {
> 
>  void cgit_prepare_repo_env(struct cgit_repo * repo)
>  {
> +	char *section = NULL;
> +	if (repo->section)
> +		section = repo->section->name;
>  	cgit_env_var env_vars[] = {
>  		{ .name = "CGIT_REPO_URL", .value = repo->url },
>  		{ .name = "CGIT_REPO_NAME", .value = repo->name },
>  		{ .name = "CGIT_REPO_PATH", .value = repo->path },
>  		{ .name = "CGIT_REPO_OWNER", .value = repo->owner },
>  		{ .name = "CGIT_REPO_DEFBRANCH", .value = repo->defbranch },
> -		{ .name = "CGIT_REPO_SECTION", .value = repo->section },
> +		{ .name = "CGIT_REPO_SECTION", .value = section },
>  		{ .name = "CGIT_REPO_CLONE_URL", .value = repo->clone_url }
>  	};
>  	int env_var_count = ARRAY_SIZE(env_vars);
> diff --git a/ui-repolist.c b/ui-repolist.c
> index f6b6b47..81f5603 100644
> --- a/ui-repolist.c
> +++ b/ui-repolist.c
> @@ -188,10 +188,16 @@ static int sort_section(const void *a, const void *b)
>  {
>  	const struct cgit_repo *r1 = a;
>  	const struct cgit_repo *r2 = b;
> +	const char *s1 = "";
> +	const char *s2 = "";
>  	int result;
>  	time_t t;
> 
> -	result = cmp(r1->section, r2->section);
> +	if (r1->section)
> +		s1 = r1->section->name;
> +	if (r2->section)
> +		s2 = r2->section->name;
> +	result = cmp(s1, s2);
>  	if (!result) {
>  		if (!strcmp(ctx.cfg.repository_sort, "age")) {
>  			// get_repo_modtime caches the value in r->mtime, so we don't
> @@ -273,8 +279,8 @@ static int sort_repolist(char *field)
>  void cgit_print_repolist(void)
>  {
>  	int i, columns = 3, hits = 0, header = 0;
> -	char *last_section = NULL;
> -	char *section;
> +	struct cgit_section *last_section = NULL;
> +	struct cgit_section *section;
>  	int sorted = 0;
> 
>  	if (!any_repos_visible()) {
> @@ -294,12 +300,10 @@ void cgit_print_repolist(void)
> 
>  	if (ctx.cfg.index_header)
>  		html_include(ctx.cfg.index_header);
> -
>  	if (ctx.qry.sort)
>  		sorted = sort_repolist(ctx.qry.sort);
>  	else if (ctx.cfg.section_sort)
>  		sort_repolist("section");
> -
>  	html("<table summary='repository list' class='list nowrap'>");
>  	for (i = 0; i < cgit_repolist.count; i++) {
>  		ctx.repo = &cgit_repolist.repos[i];
> @@ -313,19 +317,18 @@ void cgit_print_repolist(void)
>  		if (!header++)
>  			print_header();
>  		section = ctx.repo->section;
> -		if (section && !strcmp(section, ""))
> +		if (section && !strcmp(section->name, ""))
>  			section = NULL;
> -		if (!sorted &&
> -		    ((last_section == NULL && section != NULL) ||
> -		    (last_section != NULL && section == NULL) ||
> -		    (last_section != NULL && section != NULL &&
> -		     strcmp(section, last_section)))) {
> +		if (!sorted && section && last_section != section ) {

Shouldn't this just be:

	if (!sorted && last_section != section)

?  Otherwise we're missing the case where section is NULL and
last_section is non-null.

>  			htmlf("<tr class='nohover'><td colspan='%d' class='reposection'>",
>  			      columns);
> -			htmlf("<a href='%s'>%s</a>", section, section);
> +			htmlf("<a href='%s'>%s</a>", section->name, section->name);
>  			html("</td></tr>");
> -			last_section = section;
>  		}
> +		last_section = section;
> +		if (section && section->collapse)
> +			continue;
> +
>  		htmlf("<tr><td class='%s'>",
>  		      !sorted && section ? "sublevel-repo" : "toplevel-repo");
>  		cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL);
> --
> 2.7.4


More information about the CGit mailing list