[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