From mboxrd@z Thu Jan 1 00:00:00 1970 From: john at keeping.me.uk (John Keeping) Date: Sun, 28 Feb 2016 13:02:42 +0000 Subject: [PATCH 2/3] ui-repolist: Restructure internal logic to be more extensible In-Reply-To: <1456520339-32708-3-git-send-email-tim.nordell@logicpd.com> References: <1456520339-32708-1-git-send-email-tim.nordell@logicpd.com> <1456520339-32708-3-git-send-email-tim.nordell@logicpd.com> Message-ID: <20160228130242.GB1766@serenity.lan> On Fri, Feb 26, 2016 at 02:58:58PM -0600, Tim Nordell wrote: > The internal logic has been restructured so that there is a "walking" > routine that filters the repo list based on the visible criteria, and > subsequently calls a given callback for each repo found. Additionally, > split out generating a table line for a given repo, and a table line > for a given section. This makes this more loosely coupled and allows > reuse of more logic for changes to the way the repo list is displayed. > This patch does not make any functional changes to the system and is > a reorganization of the existing logic only. There's a lot going on in this patch that makes it very hard to review. It would be easier if it were broken down into: extract struct repolist_ctx extract the new generate_repolist() function add repolist_walk_visible() It would also be helpful to mention in the commit message why repolist_walk_visible() is needed: in a following patch we will be adding a variant that prints only section headers. There are also various style problems here, but they're similar to earlier patches and the Linux kernel's scripts/checkpatch.pl script catches them. > Signed-off-by: Tim Nordell > > diff --git a/ui-repolist.c b/ui-repolist.c > index 7af77e0..6b06a1a 100644 > --- a/ui-repolist.c > +++ b/ui-repolist.c > @@ -269,13 +269,135 @@ static int sort_repolist(char *field) > return 0; > } > > +struct repolist_ctx > +{ > + /* From outer contex passed to interior */ > + int columns; > + int sorted; > + > + /* Used in interior context; should be reset in repolist_walk_visible() */ > + int hits; > + const char *last_section; > +}; > + > +static void html_section(struct cgit_repo *repo, int columns) > +{ > + htmlf("", > + columns); > + if (ctx.cfg.section_filter) > + cgit_open_filter(ctx.cfg.section_filter); > + html_txt(repo->section); > + if (ctx.cfg.section_filter) > + cgit_close_filter(ctx.cfg.section_filter); > + html(""); > +} > + > +static void html_repository(struct cgit_repo *repo, int sorted) > +{ > + bool is_toplevel; > + > + ctx.repo = repo; > + > + is_toplevel = (!sorted && NULL != repo->section && repo->section[0] != '\0'); > + htmlf("", is_toplevel ? "sublevel-repo" : "toplevel-repo"); > + cgit_summary_link(repo->name, repo->name, NULL, NULL); > + html(""); > + html_link_open(cgit_repourl(repo->url), NULL, NULL); > + html_ntxt(ctx.cfg.max_repodesc_len, repo->desc); > + html_link_close(); > + html(""); > + if (ctx.cfg.enable_index_owner) { > + if (repo->owner_filter) { > + cgit_open_filter(repo->owner_filter); > + html_txt(repo->owner); > + cgit_close_filter(repo->owner_filter); > + } else { > + html(""); > + html_txt(repo->owner); > + html(""); > + } > + html(""); > + } > + print_modtime(repo); > + html(""); > + if (ctx.cfg.enable_index_links) { > + html(""); > + cgit_summary_link("summary", NULL, "button", NULL); > + cgit_log_link("log", NULL, "button", NULL, NULL, NULL, > + 0, NULL, NULL, ctx.qry.showmsg, 0); > + cgit_tree_link("tree", NULL, "button", NULL, NULL, NULL); > + html(""); > + } > + html("\n"); > +} > + > +static inline bool should_emit_section(struct cgit_repo *repo, struct repolist_ctx *c) > +{ > + /* If we're sorted, we will not have a new section emitted. */ > + if (c->sorted) > + return false; > + > + /* We need a valid repo section for the rest of the checks */ > + if(NULL == repo->section) > + return false; > + > + /* If the section title is blank (e.g. top-level), we never emit > + * a section heading. */ > + if('\0' == repo->section[0]) > + return false; > + > + /* Finally, compare the last section name to the current. If they're > + * the same, do not emit a section area. */ > + if(NULL != c->last_section && !strcmp(repo->section, c->last_section)) > + return false; > + > + c->last_section = repo->section; > + return true; > +} > + > +static int generate_repolist(struct cgit_repo *repo, struct repolist_ctx *c) > +{ > + c->hits++; > + if (c->hits <= ctx.qry.ofs) > + return 0; > + if (c->hits > ctx.qry.ofs + ctx.cfg.max_repo_count) > + return 0; > + > + if(should_emit_section(repo, c)) > + html_section(repo, c->columns); > + html_repository(repo, c->sorted); > + > + return 0; > +} > + > +typedef int (*repolist_walk_callback_t)(struct cgit_repo *repo, struct repolist_ctx *c); > +static int repolist_walk_visible(repolist_walk_callback_t callback, struct repolist_ctx *c) > +{ > + struct cgit_repo *repo; > + int i; > + > + c->hits = 0; > + c->last_section = NULL; > + > + for (i = 0; i < cgit_repolist.count; i++) { > + repo = &cgit_repolist.repos[i]; > + if (!is_visible(repo)) > + continue; > + if(NULL != callback) > + callback(repo, c); > + } > + return 0; > +} > > void cgit_print_repolist(void) > { > - int i, columns = 3, hits = 0, header = 0; > - char *last_section = NULL; > - char *section; > - int sorted = 0; > + struct repolist_ctx repolist_ctx = {0}; > + > + repolist_ctx.columns = 3; > > if (!any_repos_visible()) { > cgit_print_error_page(404, "Not found", "No repositories found"); > @@ -283,9 +405,9 @@ void cgit_print_repolist(void) > } > > if (ctx.cfg.enable_index_links) > - ++columns; > + ++repolist_ctx.columns; > if (ctx.cfg.enable_index_owner) > - ++columns; > + ++repolist_ctx.columns; > > ctx.page.title = ctx.cfg.root_title; > cgit_print_http_headers(); > @@ -296,79 +418,19 @@ void cgit_print_repolist(void) > html_include(ctx.cfg.index_header); > > if (ctx.qry.sort) > - sorted = sort_repolist(ctx.qry.sort); > + repolist_ctx.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]; > - if (!is_visible(ctx.repo)) > - continue; > - hits++; > - if (hits <= ctx.qry.ofs) > - continue; > - if (hits > ctx.qry.ofs + ctx.cfg.max_repo_count) > - continue; > - if (!header++) > - print_header(); > - section = ctx.repo->section; > - if (section && !strcmp(section, "")) > - section = NULL; > - if (!sorted && > - ((last_section == NULL && section != NULL) || > - (last_section != NULL && section == NULL) || > - (last_section != NULL && section != NULL && > - strcmp(section, last_section)))) { > - htmlf(""); > - last_section = section; > - } > - htmlf(""); > - if (ctx.cfg.enable_index_links) { > - html(""); > - } > - html("\n"); > - } > + print_header(); > + > + /* Count visible repositories */ > + repolist_walk_visible(generate_repolist, &repolist_ctx); > + > html("
", > - columns); > - if (ctx.cfg.section_filter) > - cgit_open_filter(ctx.cfg.section_filter); > - html_txt(section); > - if (ctx.cfg.section_filter) > - cgit_close_filter(ctx.cfg.section_filter); > - html("
", > - !sorted && section ? "sublevel-repo" : "toplevel-repo"); > - cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL); > - html(""); > - html_link_open(cgit_repourl(ctx.repo->url), NULL, NULL); > - html_ntxt(ctx.cfg.max_repodesc_len, ctx.repo->desc); > - html_link_close(); > - html(""); > - if (ctx.cfg.enable_index_owner) { > - if (ctx.repo->owner_filter) { > - cgit_open_filter(ctx.repo->owner_filter); > - html_txt(ctx.repo->owner); > - cgit_close_filter(ctx.repo->owner_filter); > - } else { > - html(""); > - html_txt(ctx.repo->owner); > - html(""); > - } > - html(""); > - } > - print_modtime(ctx.repo); > - html(""); > - cgit_summary_link("summary", NULL, "button", NULL); > - cgit_log_link("log", NULL, "button", NULL, NULL, NULL, > - 0, NULL, NULL, ctx.qry.showmsg, 0); > - cgit_tree_link("tree", NULL, "button", NULL, NULL, NULL); > - html("
"); > - if (hits > ctx.cfg.max_repo_count) > - print_pager(hits, ctx.cfg.max_repo_count, ctx.qry.search, ctx.qry.sort); > + if (repolist_ctx.hits > ctx.cfg.max_repo_count) > + print_pager(repolist_ctx.hits, ctx.cfg.max_repo_count, ctx.qry.search, ctx.qry.sort); > cgit_print_docend(); > } > > -- > 2.4.9 > > _______________________________________________ > CGit mailing list > CGit at lists.zx2c4.com > http://lists.zx2c4.com/mailman/listinfo/cgit