From mboxrd@z Thu Jan 1 00:00:00 1970 From: john at keeping.me.uk (John Keeping) Date: Thu, 25 Aug 2016 22:13:50 +0100 Subject: [PATCH] ui-repolist: Allow sections to be collapsible In-Reply-To: <1470779587-17667-2-git-send-email-andy.doan@linaro.org> References: <1470779587-17667-1-git-send-email-andy.doan@linaro.org> <1470779587-17667-2-git-send-email-andy.doan@linaro.org> Message-ID: <20160825211350.g4udteqtvot5kinj@john.keeping.me.uk> 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 > --- > 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(""); > 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(""); > - last_section = section; > } > + last_section = section; > + if (section && section->collapse) > + continue; > + > htmlf("
", > columns); > - htmlf("%s", section, section); > + htmlf("%s", section->name, section->name); > html("
", > !sorted && section ? "sublevel-repo" : "toplevel-repo"); > cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL); > -- > 2.7.4