* [PATCH 0/3] ui-repolist: Support additional formatting for sections @ 2016-02-26 20:58 tim.nordell 2016-02-26 20:58 ` [PATCH 1/3] ui-repolist: Add section filter tim.nordell ` (3 more replies) 0 siblings, 4 replies; 16+ messages in thread From: tim.nordell @ 2016-02-26 20:58 UTC (permalink / raw) On our internal server we have over 2,000 repositories, primarily due to mirroring external repositories locally. We patched cgit to make navigation a little easier by doing the following: - Make sections clickable (via a section filter) - Collapse repository listing to just sections when the repository listing is over 1 page worth of repositories The patchset here contains this modification and allows it to be enabled (or disabled) via configuration. You can see an example of this UI over here: http://git.logicpd.com (Note: That is running an older patched version not containing the changes here presently but has the same presentation.) Tim Nordell (3): ui-repolist: Add section filter ui-repolist: Restructure internal logic to be more extensible ui-repolist: Support a trimmed view when several sections are present cgit.c | 2 + cgit.h | 3 +- cgitrc.5.txt | 23 +++- filter.c | 1 + filters/section-example.lua | 33 ++++++ ui-repolist.c | 271 +++++++++++++++++++++++++++++++++----------- 6 files changed, 260 insertions(+), 73 deletions(-) create mode 100644 filters/section-example.lua -- 2.4.9 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] ui-repolist: Add section filter 2016-02-26 20:58 [PATCH 0/3] ui-repolist: Support additional formatting for sections tim.nordell @ 2016-02-26 20:58 ` tim.nordell 2016-02-28 12:51 ` john 2016-02-26 20:58 ` [PATCH 2/3] ui-repolist: Restructure internal logic to be more extensible tim.nordell ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: tim.nordell @ 2016-02-26 20:58 UTC (permalink / raw) This allows custom links to be added into the section headers by configuring a filter to be applied in the repository list. Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> create mode 100644 filters/section-example.lua diff --git a/cgit.c b/cgit.c index 87ba811..7dac332 100644 --- a/cgit.c +++ b/cgit.c @@ -234,6 +234,8 @@ static void config_cb(const char *name, const char *value) ctx.cfg.email_filter = cgit_new_filter(value, EMAIL); else if (!strcmp(name, "owner-filter")) ctx.cfg.owner_filter = cgit_new_filter(value, OWNER); + else if (!strcmp(name, "section-filter")) + ctx.cfg.section_filter = cgit_new_filter(value, SECTION); else if (!strcmp(name, "auth-filter")) ctx.cfg.auth_filter = cgit_new_filter(value, AUTH); else if (!strcmp(name, "embedded")) diff --git a/cgit.h b/cgit.h index b2b2ae9..f0ac36b 100644 --- a/cgit.h +++ b/cgit.h @@ -55,7 +55,7 @@ typedef enum { } diff_type; typedef enum { - ABOUT, COMMIT, SOURCE, EMAIL, AUTH, OWNER + ABOUT, COMMIT, SOURCE, EMAIL, AUTH, OWNER, SECTION } filter_type; struct cgit_filter { @@ -272,6 +272,7 @@ struct cgit_config { struct cgit_filter *source_filter; struct cgit_filter *email_filter; struct cgit_filter *owner_filter; + struct cgit_filter *section_filter; struct cgit_filter *auth_filter; }; diff --git a/cgitrc.5.txt b/cgitrc.5.txt index 3bca0ab..113b7a0 100644 --- a/cgitrc.5.txt +++ b/cgitrc.5.txt @@ -404,6 +404,13 @@ section:: after this option will inherit the current section name. Default value: none. +section-filter:: + Specifies a command which will be invoked to format section headings. + The command will get the section on its STDIN, and the STDOUT from the + command will be included verbatim as the section heading. + Default value: none. + See also: "FILTER API". + 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 @@ -695,6 +702,11 @@ owner filter:: standard input and the filter is expected to write to standard output. The output is included in the Owner column. +section filter:: + This filter is given no arguments. The section text is avilable on + standard input and the filter is expected to write to standard + output. The output is included in the section heading. + source filter:: This filter is given a single parameter: the filename of the source file to filter. The filter can use the filename to determine (for diff --git a/filter.c b/filter.c index 949c931..952336f 100644 --- a/filter.c +++ b/filter.c @@ -436,6 +436,7 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype) argument_count = 1; break; + case SECTION: case COMMIT: default: argument_count = 0; diff --git a/filters/section-example.lua b/filters/section-example.lua new file mode 100644 index 0000000..76076b2 --- /dev/null +++ b/filters/section-example.lua @@ -0,0 +1,33 @@ +-- This script is an example of an section-filter. It replaces the +-- section title with several links for each subdirectory represented +-- within the section title. (It's intended to be used where the section +-- title is also the same as the path to the repository, similar to +-- the output from the "section-from-path" option.) This script may +-- be used with the section-filter setting in cgitrc with the `lua:` +-- prefix. + +function gen_link(name, path) +end + +function filter_open() + buffer = "" +end + +function filter_close() + path = "/" + cnt = 0 + for i in string.gmatch(buffer, "[^/]+") do + if cnt > 0 then + html("/") + end + path = path .. i .. "/" + html(string.format("<a href=\"%s%s\" class=reposection>%s</a>", + os.getenv("SCRIPT_NAME"), path, i)) + cnt = cnt + 1 + end + return 0 +end + +function filter_write(str) + buffer = buffer .. str +end diff --git a/ui-repolist.c b/ui-repolist.c index 30915df..7af77e0 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -322,7 +322,11 @@ void cgit_print_repolist(void) strcmp(section, last_section)))) { htmlf("<tr class='nohover'><td colspan='%d' class='reposection'>", 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("</td></tr>"); last_section = section; } -- 2.4.9 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] ui-repolist: Add section filter 2016-02-26 20:58 ` [PATCH 1/3] ui-repolist: Add section filter tim.nordell @ 2016-02-28 12:51 ` john 0 siblings, 0 replies; 16+ messages in thread From: john @ 2016-02-28 12:51 UTC (permalink / raw) On Fri, Feb 26, 2016 at 02:58:57PM -0600, Tim Nordell wrote: > This allows custom links to be added into the section headers by > configuring a filter to be applied in the repository list. > > Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> > > create mode 100644 filters/section-example.lua > > diff --git a/cgit.c b/cgit.c > index 87ba811..7dac332 100644 > --- a/cgit.c > +++ b/cgit.c > @@ -234,6 +234,8 @@ static void config_cb(const char *name, const char *value) > ctx.cfg.email_filter = cgit_new_filter(value, EMAIL); > else if (!strcmp(name, "owner-filter")) > ctx.cfg.owner_filter = cgit_new_filter(value, OWNER); > + else if (!strcmp(name, "section-filter")) > + ctx.cfg.section_filter = cgit_new_filter(value, SECTION); > else if (!strcmp(name, "auth-filter")) > ctx.cfg.auth_filter = cgit_new_filter(value, AUTH); > else if (!strcmp(name, "embedded")) > diff --git a/cgit.h b/cgit.h > index b2b2ae9..f0ac36b 100644 > --- a/cgit.h > +++ b/cgit.h > @@ -55,7 +55,7 @@ typedef enum { > } diff_type; > > typedef enum { > - ABOUT, COMMIT, SOURCE, EMAIL, AUTH, OWNER > + ABOUT, COMMIT, SOURCE, EMAIL, AUTH, OWNER, SECTION > } filter_type; > > struct cgit_filter { > @@ -272,6 +272,7 @@ struct cgit_config { > struct cgit_filter *source_filter; > struct cgit_filter *email_filter; > struct cgit_filter *owner_filter; > + struct cgit_filter *section_filter; > struct cgit_filter *auth_filter; > }; > > diff --git a/cgitrc.5.txt b/cgitrc.5.txt > index 3bca0ab..113b7a0 100644 > --- a/cgitrc.5.txt > +++ b/cgitrc.5.txt > @@ -404,6 +404,13 @@ section:: > after this option will inherit the current section name. Default value: > none. > > +section-filter:: > + Specifies a command which will be invoked to format section headings. > + The command will get the section on its STDIN, and the STDOUT from the > + command will be included verbatim as the section heading. > + Default value: none. > + See also: "FILTER API". > + > 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 > @@ -695,6 +702,11 @@ owner filter:: > standard input and the filter is expected to write to standard > output. The output is included in the Owner column. > > +section filter:: > + This filter is given no arguments. The section text is avilable on > + standard input and the filter is expected to write to standard > + output. The output is included in the section heading. > + > source filter:: > This filter is given a single parameter: the filename of the source > file to filter. The filter can use the filename to determine (for > diff --git a/filter.c b/filter.c > index 949c931..952336f 100644 > --- a/filter.c > +++ b/filter.c > @@ -436,6 +436,7 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype) > argument_count = 1; > break; > > + case SECTION: > case COMMIT: > default: > argument_count = 0; > diff --git a/filters/section-example.lua b/filters/section-example.lua > new file mode 100644 > index 0000000..76076b2 > --- /dev/null > +++ b/filters/section-example.lua > @@ -0,0 +1,33 @@ > +-- This script is an example of an section-filter. It replaces the > +-- section title with several links for each subdirectory represented > +-- within the section title. (It's intended to be used where the section > +-- title is also the same as the path to the repository, similar to > +-- the output from the "section-from-path" option.) This script may > +-- be used with the section-filter setting in cgitrc with the `lua:` > +-- prefix. > + > +function gen_link(name, path) > +end > + > +function filter_open() > + buffer = "" > +end > + > +function filter_close() > + path = "/" > + cnt = 0 > + for i in string.gmatch(buffer, "[^/]+") do > + if cnt > 0 then > + html("/") > + end > + path = path .. i .. "/" > + html(string.format("<a href=\"%s%s\" class=reposection>%s</a>", > + os.getenv("SCRIPT_NAME"), path, i)) > + cnt = cnt + 1 > + end > + return 0 > +end > + > +function filter_write(str) > + buffer = buffer .. str > +end > diff --git a/ui-repolist.c b/ui-repolist.c > index 30915df..7af77e0 100644 > --- a/ui-repolist.c > +++ b/ui-repolist.c > @@ -322,7 +322,11 @@ void cgit_print_repolist(void) > strcmp(section, last_section)))) { > htmlf("<tr class='nohover'><td colspan='%d' class='reposection'>", > columns); > + if (ctx.cfg.section_filter) No need for the if here, cgit_{open,close}_filter already do the right thing if given a null filter. > + cgit_open_filter(ctx.cfg.section_filter); > html_txt(section); > + if (ctx.cfg.section_filter) > + cgit_close_filter(ctx.cfg.section_filter); > html("</td></tr>"); > last_section = section; > } > -- > 2.4.9 > > _______________________________________________ > CGit mailing list > CGit at lists.zx2c4.com > http://lists.zx2c4.com/mailman/listinfo/cgit ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] ui-repolist: Restructure internal logic to be more extensible 2016-02-26 20:58 [PATCH 0/3] ui-repolist: Support additional formatting for sections tim.nordell 2016-02-26 20:58 ` [PATCH 1/3] ui-repolist: Add section filter tim.nordell @ 2016-02-26 20:58 ` tim.nordell 2016-02-28 13:02 ` john 2016-02-26 20:58 ` [PATCH 3/3] ui-repolist: Support a trimmed view when several sections are present tim.nordell 2016-03-04 23:24 ` [PATCH v2 0/8] ui-repolist: Support additional formatting for sections tim.nordell 3 siblings, 1 reply; 16+ messages in thread From: tim.nordell @ 2016-02-26 20:58 UTC (permalink / raw) 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. Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> 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("<tr class='nohover'><td colspan='%d' class='reposection'>", + 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("</td></tr>"); +} + +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("<tr><td class='%s'>", is_toplevel ? "sublevel-repo" : "toplevel-repo"); + cgit_summary_link(repo->name, repo->name, NULL, NULL); + html("</td><td>"); + html_link_open(cgit_repourl(repo->url), NULL, NULL); + html_ntxt(ctx.cfg.max_repodesc_len, repo->desc); + html_link_close(); + html("</td><td>"); + 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("<a href='"); + html_attr(cgit_currenturl()); + html("?q="); + html_url_arg(repo->owner); + html("'>"); + html_txt(repo->owner); + html("</a>"); + } + html("</td><td>"); + } + print_modtime(repo); + html("</td>"); + if (ctx.cfg.enable_index_links) { + html("<td>"); + 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("</td>"); + } + html("</tr>\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("<table summary='repository list' class='list nowrap'>"); - 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("<tr class='nohover'><td colspan='%d' class='reposection'>", - 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("</td></tr>"); - last_section = section; - } - htmlf("<tr><td class='%s'>", - !sorted && section ? "sublevel-repo" : "toplevel-repo"); - cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL); - html("</td><td>"); - html_link_open(cgit_repourl(ctx.repo->url), NULL, NULL); - html_ntxt(ctx.cfg.max_repodesc_len, ctx.repo->desc); - html_link_close(); - html("</td><td>"); - 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("<a href='"); - html_attr(cgit_currenturl()); - html("?q="); - html_url_arg(ctx.repo->owner); - html("'>"); - html_txt(ctx.repo->owner); - html("</a>"); - } - html("</td><td>"); - } - print_modtime(ctx.repo); - html("</td>"); - if (ctx.cfg.enable_index_links) { - html("<td>"); - 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("</td>"); - } - html("</tr>\n"); - } + print_header(); + + /* Count visible repositories */ + repolist_walk_visible(generate_repolist, &repolist_ctx); + html("</table>"); - 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] ui-repolist: Restructure internal logic to be more extensible 2016-02-26 20:58 ` [PATCH 2/3] ui-repolist: Restructure internal logic to be more extensible tim.nordell @ 2016-02-28 13:02 ` john 2016-02-29 20:51 ` tim.nordell 0 siblings, 1 reply; 16+ messages in thread From: john @ 2016-02-28 13:02 UTC (permalink / raw) 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 <tim.nordell at logicpd.com> > > 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("<tr class='nohover'><td colspan='%d' class='reposection'>", > + 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("</td></tr>"); > +} > + > +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("<tr><td class='%s'>", is_toplevel ? "sublevel-repo" : "toplevel-repo"); > + cgit_summary_link(repo->name, repo->name, NULL, NULL); > + html("</td><td>"); > + html_link_open(cgit_repourl(repo->url), NULL, NULL); > + html_ntxt(ctx.cfg.max_repodesc_len, repo->desc); > + html_link_close(); > + html("</td><td>"); > + 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("<a href='"); > + html_attr(cgit_currenturl()); > + html("?q="); > + html_url_arg(repo->owner); > + html("'>"); > + html_txt(repo->owner); > + html("</a>"); > + } > + html("</td><td>"); > + } > + print_modtime(repo); > + html("</td>"); > + if (ctx.cfg.enable_index_links) { > + html("<td>"); > + 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("</td>"); > + } > + html("</tr>\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("<table summary='repository list' class='list nowrap'>"); > - 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("<tr class='nohover'><td colspan='%d' class='reposection'>", > - 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("</td></tr>"); > - last_section = section; > - } > - htmlf("<tr><td class='%s'>", > - !sorted && section ? "sublevel-repo" : "toplevel-repo"); > - cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL); > - html("</td><td>"); > - html_link_open(cgit_repourl(ctx.repo->url), NULL, NULL); > - html_ntxt(ctx.cfg.max_repodesc_len, ctx.repo->desc); > - html_link_close(); > - html("</td><td>"); > - 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("<a href='"); > - html_attr(cgit_currenturl()); > - html("?q="); > - html_url_arg(ctx.repo->owner); > - html("'>"); > - html_txt(ctx.repo->owner); > - html("</a>"); > - } > - html("</td><td>"); > - } > - print_modtime(ctx.repo); > - html("</td>"); > - if (ctx.cfg.enable_index_links) { > - html("<td>"); > - 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("</td>"); > - } > - html("</tr>\n"); > - } > + print_header(); > + > + /* Count visible repositories */ > + repolist_walk_visible(generate_repolist, &repolist_ctx); > + > html("</table>"); > - 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] ui-repolist: Restructure internal logic to be more extensible 2016-02-28 13:02 ` john @ 2016-02-29 20:51 ` tim.nordell 0 siblings, 0 replies; 16+ messages in thread From: tim.nordell @ 2016-02-29 20:51 UTC (permalink / raw) On 02/28/16 07:02, John Keeping wrote: > 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() > Thanks for the initial glance and suggestion. I'll break it apart along these lines. > 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. Noted. > 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. Yeah, I didn't realize that CGIT was following the kernel's style guide. I looked in CGIT's folder, but I didn't expect it to be following documentation inside the submodule. I'll grab the kernel's style checker and vet the changes against that. - Tim ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] ui-repolist: Support a trimmed view when several sections are present 2016-02-26 20:58 [PATCH 0/3] ui-repolist: Support additional formatting for sections tim.nordell 2016-02-26 20:58 ` [PATCH 1/3] ui-repolist: Add section filter tim.nordell 2016-02-26 20:58 ` [PATCH 2/3] ui-repolist: Restructure internal logic to be more extensible tim.nordell @ 2016-02-26 20:58 ` tim.nordell 2016-03-04 23:24 ` [PATCH v2 0/8] ui-repolist: Support additional formatting for sections tim.nordell 3 siblings, 0 replies; 16+ messages in thread From: tim.nordell @ 2016-02-26 20:58 UTC (permalink / raw) Trim the view to just a section list if there are multiple sections present in the output view. This is only really useful if one also has a script placing links on each section heading to trim the view by path. Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> diff --git a/cgitrc.5.txt b/cgitrc.5.txt index 113b7a0..2658e89 100644 --- a/cgitrc.5.txt +++ b/cgitrc.5.txt @@ -413,9 +413,14 @@ section-filter:: 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 - be preserved. Default value: "1". See also: section, - case-sensitive-sort, repository-sort. + listing by name. When set to "2", this will collapse the listing of + repositories (when over the size max-repo-count) into just the list + of sections. This is only really useful if section filters are also + enabled, if the filter makes links into the URL that the section + comprises, and if all section headings are paths. Set this flag to + "0" if the order in the cgitrc file should be preserved. + Default value: "1". See also: section, case-sensitive-sort, + repository-sort, section-filter section-from-path:: A number which, if defined prior to scan-path, specifies how many diff --git a/ui-repolist.c b/ui-repolist.c index 6b06a1a..9045203 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -278,6 +278,8 @@ struct repolist_ctx /* Used in interior context; should be reset in repolist_walk_visible() */ int hits; const char *last_section; + int section_cnt; + int section_nested_cnt; }; static void html_section(struct cgit_repo *repo, int columns) @@ -374,6 +376,57 @@ static int generate_repolist(struct cgit_repo *repo, struct repolist_ctx *c) return 0; } +static int generate_sectionlist(struct cgit_repo *repo, struct repolist_ctx *c) +{ + bool is_toplevel; + + is_toplevel = (NULL == repo->section || repo->section[0] == '\0'); + + if(!should_emit_section(repo, c) && !is_toplevel) + return 0; + + c->hits++; + + if (c->hits <= ctx.qry.ofs) + return 0; + if (c->hits > ctx.qry.ofs + ctx.cfg.max_repo_count) + return 0; + + if(is_toplevel) + html_repository(repo, c->sorted); + else + html_section(repo, c->columns); + + return 0; +} + +static int count_sections(struct cgit_repo *repo, struct repolist_ctx *c) +{ + const char *last_section; + + last_section = c->last_section; + if(should_emit_section(repo, c)) + { + c->section_cnt++; + + /* Determine if one section is nested within the other. This + * is only accurate if this is a sorted list. + */ + if(NULL != last_section && NULL != repo->section) + { + int spn = strspn(last_section, repo->section); + if(last_section[spn] == '\0') + { + c->section_nested_cnt++; + } + } + } + + c->hits++; + + 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) { @@ -382,6 +435,8 @@ static int repolist_walk_visible(repolist_walk_callback_t callback, struct repol c->hits = 0; c->last_section = NULL; + c->section_cnt = 0; + c->section_nested_cnt = 0; for (i = 0; i < cgit_repolist.count; i++) { repo = &cgit_repolist.repos[i]; @@ -395,6 +450,7 @@ static int repolist_walk_visible(repolist_walk_callback_t callback, struct repol void cgit_print_repolist(void) { + bool section_pages = false; struct repolist_ctx repolist_ctx = {0}; repolist_ctx.columns = 3; @@ -420,13 +476,24 @@ void cgit_print_repolist(void) if (ctx.qry.sort) repolist_ctx.sorted = sort_repolist(ctx.qry.sort); else if (ctx.cfg.section_sort) + { sort_repolist("section"); + section_pages = (2 == ctx.cfg.section_sort); + } html("<table summary='repository list' class='list nowrap'>"); print_header(); - /* Count visible repositories */ - repolist_walk_visible(generate_repolist, &repolist_ctx); + if(section_pages) + repolist_walk_visible(count_sections, &repolist_ctx); + + if(section_pages && repolist_ctx.hits > ctx.cfg.max_repo_count && + repolist_ctx.section_cnt - repolist_ctx.section_nested_cnt > 1) + { + repolist_walk_visible(generate_sectionlist, &repolist_ctx); + } else { + repolist_walk_visible(generate_repolist, &repolist_ctx); + } html("</table>"); if (repolist_ctx.hits > ctx.cfg.max_repo_count) -- 2.4.9 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 0/8] ui-repolist: Support additional formatting for sections 2016-02-26 20:58 [PATCH 0/3] ui-repolist: Support additional formatting for sections tim.nordell ` (2 preceding siblings ...) 2016-02-26 20:58 ` [PATCH 3/3] ui-repolist: Support a trimmed view when several sections are present tim.nordell @ 2016-03-04 23:24 ` tim.nordell 2016-03-04 23:24 ` [PATCH v2 1/8] ui-repolist: Add section filter tim.nordell ` (7 more replies) 3 siblings, 8 replies; 16+ messages in thread From: tim.nordell @ 2016-03-04 23:24 UTC (permalink / raw) On our internal server we have over 2,000 repositories, primarily due to mirroring external repositories locally. We patched cgit to make navigation a little easier by doing the following: - Make sections clickable (via a section filter) - Collapse repository listing to just sections when the repository listing is over 1 page worth of repositories The patchset here contains this modification and allows it to be enabled (or disabled) via configuration. You can see an example of this UI over here: http://git.logicpd.com (Note: That is running an older patched version not containing the changes here presently but has the same presentation.) Changes since v1: - Split apart patches into smaller discrete patches - Removed NULL check around open and close filter - Misc. style issues - Made parameter orders for repolist_ctx vs cgit_repo more consistent across routines Tim Nordell (8): ui-repolist: Add section filter ui-repolist: Remove conditional around print_header() ui-repolist: Create context structure for cgit_print_repolist() ui-repolist: Move HTML generation into helper functions ui-repolist: Factor out logic for emitting a new section heading ui-repolist: Remove assignment of section to repolist_ctx ui-repolist: Restructure internal logic to be more extensible ui-repolist: Support a trimmed view when several sections are present cgit.c | 2 + cgit.h | 3 +- cgitrc.5.txt | 12 ++ filter.c | 1 + filters/section-example.lua | 33 ++++++ ui-repolist.c | 268 ++++++++++++++++++++++++++++++++------------ 6 files changed, 247 insertions(+), 72 deletions(-) create mode 100644 filters/section-example.lua -- 2.4.9 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/8] ui-repolist: Add section filter 2016-03-04 23:24 ` [PATCH v2 0/8] ui-repolist: Support additional formatting for sections tim.nordell @ 2016-03-04 23:24 ` tim.nordell 2016-03-04 23:24 ` [PATCH v2 2/8] ui-repolist: Remove conditional around print_header() tim.nordell ` (6 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: tim.nordell @ 2016-03-04 23:24 UTC (permalink / raw) This allows custom links to be added into the section headers by configuring a filter to be applied in the repository list. Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> create mode 100644 filters/section-example.lua diff --git a/cgit.c b/cgit.c index fc482be..3ef01cd 100644 --- a/cgit.c +++ b/cgit.c @@ -210,6 +210,8 @@ static void config_cb(const char *name, const char *value) ctx.cfg.email_filter = cgit_new_filter(value, EMAIL); else if (!strcmp(name, "owner-filter")) ctx.cfg.owner_filter = cgit_new_filter(value, OWNER); + else if (!strcmp(name, "section-filter")) + ctx.cfg.section_filter = cgit_new_filter(value, SECTION); else if (!strcmp(name, "auth-filter")) ctx.cfg.auth_filter = cgit_new_filter(value, AUTH); else if (!strcmp(name, "embedded")) diff --git a/cgit.h b/cgit.h index 325432b..74ef01c 100644 --- a/cgit.h +++ b/cgit.h @@ -55,7 +55,7 @@ typedef enum { } diff_type; typedef enum { - ABOUT, COMMIT, SOURCE, EMAIL, AUTH, OWNER + ABOUT, COMMIT, SOURCE, EMAIL, AUTH, OWNER, SECTION } filter_type; struct cgit_filter { @@ -266,6 +266,7 @@ struct cgit_config { struct cgit_filter *source_filter; struct cgit_filter *email_filter; struct cgit_filter *owner_filter; + struct cgit_filter *section_filter; struct cgit_filter *auth_filter; }; diff --git a/cgitrc.5.txt b/cgitrc.5.txt index 94901bd..de5ceab 100644 --- a/cgitrc.5.txt +++ b/cgitrc.5.txt @@ -404,6 +404,13 @@ section:: after this option will inherit the current section name. Default value: none. +section-filter:: + Specifies a command which will be invoked to format section headings. + The command will get the section on its STDIN, and the STDOUT from the + command will be included verbatim as the section heading. + Default value: none. + See also: "FILTER API". + 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 @@ -687,6 +694,11 @@ owner filter:: standard input and the filter is expected to write to standard output. The output is included in the Owner column. +section filter:: + This filter is given no arguments. The section text is avilable on + standard input and the filter is expected to write to standard + output. The output is included in the section heading. + source filter:: This filter is given a single parameter: the filename of the source file to filter. The filter can use the filename to determine (for diff --git a/filter.c b/filter.c index 949c931..952336f 100644 --- a/filter.c +++ b/filter.c @@ -436,6 +436,7 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype) argument_count = 1; break; + case SECTION: case COMMIT: default: argument_count = 0; diff --git a/filters/section-example.lua b/filters/section-example.lua new file mode 100644 index 0000000..76076b2 --- /dev/null +++ b/filters/section-example.lua @@ -0,0 +1,33 @@ +-- This script is an example of an section-filter. It replaces the +-- section title with several links for each subdirectory represented +-- within the section title. (It's intended to be used where the section +-- title is also the same as the path to the repository, similar to +-- the output from the "section-from-path" option.) This script may +-- be used with the section-filter setting in cgitrc with the `lua:` +-- prefix. + +function gen_link(name, path) +end + +function filter_open() + buffer = "" +end + +function filter_close() + path = "/" + cnt = 0 + for i in string.gmatch(buffer, "[^/]+") do + if cnt > 0 then + html("/") + end + path = path .. i .. "/" + html(string.format("<a href=\"%s%s\" class=reposection>%s</a>", + os.getenv("SCRIPT_NAME"), path, i)) + cnt = cnt + 1 + end + return 0 +end + +function filter_write(str) + buffer = buffer .. str +end diff --git a/ui-repolist.c b/ui-repolist.c index 30915df..771b330 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -322,7 +322,9 @@ void cgit_print_repolist(void) strcmp(section, last_section)))) { htmlf("<tr class='nohover'><td colspan='%d' class='reposection'>", columns); + cgit_open_filter(ctx.cfg.section_filter); html_txt(section); + cgit_close_filter(ctx.cfg.section_filter); html("</td></tr>"); last_section = section; } -- 2.4.9 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 2/8] ui-repolist: Remove conditional around print_header() 2016-03-04 23:24 ` [PATCH v2 0/8] ui-repolist: Support additional formatting for sections tim.nordell 2016-03-04 23:24 ` [PATCH v2 1/8] ui-repolist: Add section filter tim.nordell @ 2016-03-04 23:24 ` tim.nordell 2016-03-04 23:24 ` [PATCH v2 3/8] ui-repolist: Create context structure for cgit_print_repolist() tim.nordell ` (5 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: tim.nordell @ 2016-03-04 23:24 UTC (permalink / raw) The code has since morphed from the original use case of the conditional for deciding whether or not to print the header. This is guaranteed to always pass due to the earlier check in the function "any_repos_visible()" which emits a 404 page. Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> diff --git a/ui-repolist.c b/ui-repolist.c index 771b330..c240af3 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -272,7 +272,7 @@ static int sort_repolist(char *field) void cgit_print_repolist(void) { - int i, columns = 3, hits = 0, header = 0; + int i, columns = 3, hits = 0; char *last_section = NULL; char *section; int sorted = 0; @@ -301,6 +301,7 @@ void cgit_print_repolist(void) sort_repolist("section"); html("<table summary='repository list' class='list nowrap'>"); + print_header(); for (i = 0; i < cgit_repolist.count; i++) { ctx.repo = &cgit_repolist.repos[i]; if (!is_visible(ctx.repo)) @@ -310,8 +311,6 @@ void cgit_print_repolist(void) 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; -- 2.4.9 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 3/8] ui-repolist: Create context structure for cgit_print_repolist() 2016-03-04 23:24 ` [PATCH v2 0/8] ui-repolist: Support additional formatting for sections tim.nordell 2016-03-04 23:24 ` [PATCH v2 1/8] ui-repolist: Add section filter tim.nordell 2016-03-04 23:24 ` [PATCH v2 2/8] ui-repolist: Remove conditional around print_header() tim.nordell @ 2016-03-04 23:24 ` tim.nordell 2016-03-04 23:24 ` [PATCH v2 4/8] ui-repolist: Move HTML generation into helper functions tim.nordell ` (4 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: tim.nordell @ 2016-03-04 23:24 UTC (permalink / raw) This is in preparation of moving the code that iterates over the repository list into a walk pattern. This walk pattern will permit other views on the repository list more easily, such as a section only list. Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> diff --git a/ui-repolist.c b/ui-repolist.c index c240af3..6b751d2 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -269,13 +269,24 @@ static int sort_repolist(char *field) return 0; } +struct repolist_ctx { + int columns; + int sorted; + int hits; + const char *section; + const char *last_section; +}; void cgit_print_repolist(void) { - int i, columns = 3, hits = 0; - char *last_section = NULL; - char *section; - int sorted = 0; + struct repolist_ctx repolist_ctx; + struct repolist_ctx *c = &repolist_ctx; + int i; + + repolist_ctx.columns = 3; + repolist_ctx.hits = 0; + repolist_ctx.last_section = NULL; + repolist_ctx.sorted = 0; if (!any_repos_visible()) { cgit_print_error_page(404, "Not found", "No repositories found"); @@ -283,9 +294,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,7 +307,7 @@ 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"); @@ -306,29 +317,29 @@ void cgit_print_repolist(void) ctx.repo = &cgit_repolist.repos[i]; if (!is_visible(ctx.repo)) continue; - hits++; - if (hits <= ctx.qry.ofs) + c->hits++; + if (c->hits <= ctx.qry.ofs) continue; - if (hits > ctx.qry.ofs + ctx.cfg.max_repo_count) + if (c->hits > ctx.qry.ofs + ctx.cfg.max_repo_count) continue; - 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)))) { + c->section = ctx.repo->section; + if (c->section && !strcmp(c->section, "")) + c->section = NULL; + if (!c->sorted && + ((c->last_section == NULL && c->section != NULL) || + (repolist_ctx.last_section != NULL && c->section == NULL) || + (repolist_ctx.last_section != NULL && c->section != NULL && + strcmp(c->section, c->last_section)))) { htmlf("<tr class='nohover'><td colspan='%d' class='reposection'>", - columns); + c->columns); cgit_open_filter(ctx.cfg.section_filter); - html_txt(section); + html_txt(c->section); cgit_close_filter(ctx.cfg.section_filter); html("</td></tr>"); - last_section = section; + c->last_section = c->section; } htmlf("<tr><td class='%s'>", - !sorted && section ? "sublevel-repo" : "toplevel-repo"); + !c->sorted && c->section ? "sublevel-repo" : "toplevel-repo"); cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL); html("</td><td>"); html_link_open(cgit_repourl(ctx.repo->url), NULL, NULL); @@ -364,8 +375,8 @@ void cgit_print_repolist(void) html("</tr>\n"); } html("</table>"); - 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 4/8] ui-repolist: Move HTML generation into helper functions 2016-03-04 23:24 ` [PATCH v2 0/8] ui-repolist: Support additional formatting for sections tim.nordell ` (2 preceding siblings ...) 2016-03-04 23:24 ` [PATCH v2 3/8] ui-repolist: Create context structure for cgit_print_repolist() tim.nordell @ 2016-03-04 23:24 ` tim.nordell 2016-03-04 23:29 ` [PATCH v2 5/8] ui-repolist: Factor out logic for emitting a new section heading tim.nordell ` (3 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: tim.nordell @ 2016-03-04 23:24 UTC (permalink / raw) Move the code that generates the repository and section snippets of html into their own helper functions. This is for code reuse in a later patch in this series. This also moves some dependencies from the context out into the caller instead of in the section of code moved. Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> diff --git a/ui-repolist.c b/ui-repolist.c index 6b751d2..38a42c7 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -277,6 +277,58 @@ struct repolist_ctx { const char *last_section; }; +static void html_section(struct cgit_repo *repo, int columns) +{ + htmlf("<tr class='nohover'><td colspan='%d' class='reposection'>", + columns); + cgit_open_filter(ctx.cfg.section_filter); + html_txt(repo->section); + cgit_close_filter(ctx.cfg.section_filter); + html("</td></tr>"); +} + +static void html_repository(struct cgit_repo *repo, bool sorted) +{ + bool is_toplevel; + + is_toplevel = (NULL != repo->section && '\0' != repo->section[0]); + htmlf("<tr><td class='%s'>", + (!sorted && is_toplevel) ? "sublevel-repo" : "toplevel-repo"); + cgit_summary_link(repo->name, repo->name, NULL, NULL); + html("</td><td>"); + html_link_open(cgit_repourl(repo->url), NULL, NULL); + html_ntxt(ctx.cfg.max_repodesc_len, repo->desc); + html_link_close(); + html("</td><td>"); + 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("<a href='"); + html_attr(cgit_currenturl()); + html("?q="); + html_url_arg(repo->owner); + html("'>"); + html_txt(repo->owner); + html("</a>"); + } + html("</td><td>"); + } + print_modtime(repo); + html("</td>"); + if (ctx.cfg.enable_index_links) { + html("<td>"); + 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("</td>"); + } + html("</tr>\n"); +} + void cgit_print_repolist(void) { struct repolist_ctx repolist_ctx; @@ -330,49 +382,10 @@ void cgit_print_repolist(void) (repolist_ctx.last_section != NULL && c->section == NULL) || (repolist_ctx.last_section != NULL && c->section != NULL && strcmp(c->section, c->last_section)))) { - htmlf("<tr class='nohover'><td colspan='%d' class='reposection'>", - c->columns); - cgit_open_filter(ctx.cfg.section_filter); - html_txt(c->section); - cgit_close_filter(ctx.cfg.section_filter); - html("</td></tr>"); + html_section(ctx.repo, c->columns); c->last_section = c->section; } - htmlf("<tr><td class='%s'>", - !c->sorted && c->section ? "sublevel-repo" : "toplevel-repo"); - cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL); - html("</td><td>"); - html_link_open(cgit_repourl(ctx.repo->url), NULL, NULL); - html_ntxt(ctx.cfg.max_repodesc_len, ctx.repo->desc); - html_link_close(); - html("</td><td>"); - 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("<a href='"); - html_attr(cgit_currenturl()); - html("?q="); - html_url_arg(ctx.repo->owner); - html("'>"); - html_txt(ctx.repo->owner); - html("</a>"); - } - html("</td><td>"); - } - print_modtime(ctx.repo); - html("</td>"); - if (ctx.cfg.enable_index_links) { - html("<td>"); - 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("</td>"); - } - html("</tr>\n"); + html_repository(ctx.repo, repolist_ctx.sorted); } html("</table>"); if (repolist_ctx.hits > ctx.cfg.max_repo_count) -- 2.4.9 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 5/8] ui-repolist: Factor out logic for emitting a new section heading 2016-03-04 23:24 ` [PATCH v2 0/8] ui-repolist: Support additional formatting for sections tim.nordell ` (3 preceding siblings ...) 2016-03-04 23:24 ` [PATCH v2 4/8] ui-repolist: Move HTML generation into helper functions tim.nordell @ 2016-03-04 23:29 ` tim.nordell 2016-03-04 23:30 ` [PATCH v2 6/8] ui-repolist: Remove assignment of section to repolist_ctx tim.nordell ` (2 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: tim.nordell @ 2016-03-04 23:29 UTC (permalink / raw) For a later commit in this series, this logic is common in a couple of places. This simplifies the logic a bit instead of having several nested checks. Additionally, this patch removes the need for using "!strcmp(c->section, "")" as this is equivalent to comparing "'\0' == repo->section[0]". Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> diff --git a/ui-repolist.c b/ui-repolist.c index 38a42c7..df2dc92 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -329,6 +329,30 @@ static void html_repository(struct cgit_repo *repo, bool sorted) html("</tr>\n"); } +static inline bool should_emit_section(struct repolist_ctx *c, struct cgit_repo *repo) +{ + /* 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; +} + void cgit_print_repolist(void) { struct repolist_ctx repolist_ctx; @@ -375,16 +399,8 @@ void cgit_print_repolist(void) if (c->hits > ctx.qry.ofs + ctx.cfg.max_repo_count) continue; c->section = ctx.repo->section; - if (c->section && !strcmp(c->section, "")) - c->section = NULL; - if (!c->sorted && - ((c->last_section == NULL && c->section != NULL) || - (repolist_ctx.last_section != NULL && c->section == NULL) || - (repolist_ctx.last_section != NULL && c->section != NULL && - strcmp(c->section, c->last_section)))) { + if (should_emit_section(&repolist_ctx, ctx.repo)) html_section(ctx.repo, c->columns); - c->last_section = c->section; - } html_repository(ctx.repo, repolist_ctx.sorted); } html("</table>"); -- 2.4.9 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 6/8] ui-repolist: Remove assignment of section to repolist_ctx 2016-03-04 23:24 ` [PATCH v2 0/8] ui-repolist: Support additional formatting for sections tim.nordell ` (4 preceding siblings ...) 2016-03-04 23:29 ` [PATCH v2 5/8] ui-repolist: Factor out logic for emitting a new section heading tim.nordell @ 2016-03-04 23:30 ` tim.nordell 2016-03-04 23:30 ` [PATCH v2 7/8] ui-repolist: Restructure internal logic to be more extensible tim.nordell 2016-03-04 23:30 ` [PATCH v2 8/8] ui-repolist: Support a trimmed view when several sections are present tim.nordell 7 siblings, 0 replies; 16+ messages in thread From: tim.nordell @ 2016-03-04 23:30 UTC (permalink / raw) There is now only one user of this, emitting a html_section, and this gets the repository passed in (which also contains a link to the section heading). Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> diff --git a/ui-repolist.c b/ui-repolist.c index df2dc92..fab589e 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -273,7 +273,6 @@ struct repolist_ctx { int columns; int sorted; int hits; - const char *section; const char *last_section; }; @@ -398,7 +397,6 @@ void cgit_print_repolist(void) continue; if (c->hits > ctx.qry.ofs + ctx.cfg.max_repo_count) continue; - c->section = ctx.repo->section; if (should_emit_section(&repolist_ctx, ctx.repo)) html_section(ctx.repo, c->columns); html_repository(ctx.repo, repolist_ctx.sorted); -- 2.4.9 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 7/8] ui-repolist: Restructure internal logic to be more extensible 2016-03-04 23:24 ` [PATCH v2 0/8] ui-repolist: Support additional formatting for sections tim.nordell ` (5 preceding siblings ...) 2016-03-04 23:30 ` [PATCH v2 6/8] ui-repolist: Remove assignment of section to repolist_ctx tim.nordell @ 2016-03-04 23:30 ` tim.nordell 2016-03-04 23:30 ` [PATCH v2 8/8] ui-repolist: Support a trimmed view when several sections are present tim.nordell 7 siblings, 0 replies; 16+ messages in thread From: tim.nordell @ 2016-03-04 23:30 UTC (permalink / raw) 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 is for a future patch in this series. Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> diff --git a/ui-repolist.c b/ui-repolist.c index fab589e..93655d4 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -270,8 +270,12 @@ static int sort_repolist(char *field) } 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; }; @@ -290,6 +294,10 @@ static void html_repository(struct cgit_repo *repo, bool sorted) { bool is_toplevel; + /* Note: cgit_summary_link() ultimately calls repolink(), which is + * dependent on ctx.repo pointing to the repo we're working on */ + ctx.repo = repo; + is_toplevel = (NULL != repo->section && '\0' != repo->section[0]); htmlf("<tr><td class='%s'>", (!sorted && is_toplevel) ? "sublevel-repo" : "toplevel-repo"); @@ -352,15 +360,45 @@ static inline bool should_emit_section(struct repolist_ctx *c, struct cgit_repo return true; } +static int generate_repolist(struct repolist_ctx *c, struct cgit_repo *repo) +{ + 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(c, repo)) + html_section(repo, c->columns); + html_repository(repo, c->sorted); + + return 0; +} + +typedef int (*repolist_walk_callback_t)(struct repolist_ctx *c, struct cgit_repo *repo); +static int repolist_walk_visible(struct repolist_ctx *c, repolist_walk_callback_t callback) +{ + 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(c, repo); + } + return 0; +} + void cgit_print_repolist(void) { struct repolist_ctx repolist_ctx; - struct repolist_ctx *c = &repolist_ctx; - int i; repolist_ctx.columns = 3; - repolist_ctx.hits = 0; - repolist_ctx.last_section = NULL; repolist_ctx.sorted = 0; if (!any_repos_visible()) { @@ -388,19 +426,7 @@ void cgit_print_repolist(void) html("<table summary='repository list' class='list nowrap'>"); print_header(); - for (i = 0; i < cgit_repolist.count; i++) { - ctx.repo = &cgit_repolist.repos[i]; - if (!is_visible(ctx.repo)) - continue; - c->hits++; - if (c->hits <= ctx.qry.ofs) - continue; - if (c->hits > ctx.qry.ofs + ctx.cfg.max_repo_count) - continue; - if (should_emit_section(&repolist_ctx, ctx.repo)) - html_section(ctx.repo, c->columns); - html_repository(ctx.repo, repolist_ctx.sorted); - } + repolist_walk_visible(&repolist_ctx, generate_repolist); html("</table>"); 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); -- 2.4.9 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 8/8] ui-repolist: Support a trimmed view when several sections are present 2016-03-04 23:24 ` [PATCH v2 0/8] ui-repolist: Support additional formatting for sections tim.nordell ` (6 preceding siblings ...) 2016-03-04 23:30 ` [PATCH v2 7/8] ui-repolist: Restructure internal logic to be more extensible tim.nordell @ 2016-03-04 23:30 ` tim.nordell 7 siblings, 0 replies; 16+ messages in thread From: tim.nordell @ 2016-03-04 23:30 UTC (permalink / raw) Trim the view to just a section list if there are multiple sections present in the output view. This is only really useful if one also has a script placing links on each section heading to trim the view by path. Signed-off-by: Tim Nordell <tim.nordell at logicpd.com> diff --git a/ui-repolist.c b/ui-repolist.c index 93655d4..ff70ddc 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -278,6 +278,8 @@ struct repolist_ctx { * (Should be reset in repolist_walk_visible()) */ int hits; const char *last_section; + int section_cnt; + int section_nested_cnt; }; static void html_section(struct cgit_repo *repo, int columns) @@ -375,6 +377,51 @@ static int generate_repolist(struct repolist_ctx *c, struct cgit_repo *repo) return 0; } +static int generate_sectionlist(struct repolist_ctx *c, struct cgit_repo *repo) +{ + bool is_toplevel; + + is_toplevel = (NULL == repo->section || repo->section[0] == '\0'); + + if (!should_emit_section(c, repo) && !is_toplevel) + return 0; + + c->hits++; + + if (c->hits <= ctx.qry.ofs) + return 0; + if (c->hits > ctx.qry.ofs + ctx.cfg.max_repo_count) + return 0; + + if (is_toplevel) + html_repository(repo, c->sorted); + else + html_section(repo, c->columns); + + return 0; +} + +static int count_sections(struct repolist_ctx *c, struct cgit_repo *repo) +{ + const char *last_section; + + last_section = c->last_section; + if (should_emit_section(c, repo)) { + c->section_cnt++; + + /* Determine if one section is nested within the other. This + * is only accurate if this is a sorted list. + */ + if (NULL != last_section && NULL != repo->section && + last_section[strspn(last_section, repo->section)] == '\0') + c->section_nested_cnt++; + } + + c->hits++; + + return 0; +} + typedef int (*repolist_walk_callback_t)(struct repolist_ctx *c, struct cgit_repo *repo); static int repolist_walk_visible(struct repolist_ctx *c, repolist_walk_callback_t callback) { @@ -383,6 +430,8 @@ static int repolist_walk_visible(struct repolist_ctx *c, repolist_walk_callback_ c->hits = 0; c->last_section = NULL; + c->section_cnt = 0; + c->section_nested_cnt = 0; for (i = 0; i < cgit_repolist.count; i++) { repo = &cgit_repolist.repos[i]; @@ -396,6 +445,7 @@ static int repolist_walk_visible(struct repolist_ctx *c, repolist_walk_callback_ void cgit_print_repolist(void) { + bool section_pages = false; struct repolist_ctx repolist_ctx; repolist_ctx.columns = 3; @@ -421,12 +471,23 @@ void cgit_print_repolist(void) if (ctx.qry.sort) repolist_ctx.sorted = sort_repolist(ctx.qry.sort); - else if (ctx.cfg.section_sort) + else if (ctx.cfg.section_sort) { sort_repolist("section"); + section_pages = (2 == ctx.cfg.section_sort); + } html("<table summary='repository list' class='list nowrap'>"); print_header(); - repolist_walk_visible(&repolist_ctx, generate_repolist); + + if (section_pages) + repolist_walk_visible(&repolist_ctx, count_sections); + + if (section_pages && repolist_ctx.hits > ctx.cfg.max_repo_count && + repolist_ctx.section_cnt - repolist_ctx.section_nested_cnt > 1) + repolist_walk_visible(&repolist_ctx, generate_sectionlist); + else + repolist_walk_visible(&repolist_ctx, generate_repolist); + html("</table>"); 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); -- 2.4.9 ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-03-04 23:30 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-02-26 20:58 [PATCH 0/3] ui-repolist: Support additional formatting for sections tim.nordell 2016-02-26 20:58 ` [PATCH 1/3] ui-repolist: Add section filter tim.nordell 2016-02-28 12:51 ` john 2016-02-26 20:58 ` [PATCH 2/3] ui-repolist: Restructure internal logic to be more extensible tim.nordell 2016-02-28 13:02 ` john 2016-02-29 20:51 ` tim.nordell 2016-02-26 20:58 ` [PATCH 3/3] ui-repolist: Support a trimmed view when several sections are present tim.nordell 2016-03-04 23:24 ` [PATCH v2 0/8] ui-repolist: Support additional formatting for sections tim.nordell 2016-03-04 23:24 ` [PATCH v2 1/8] ui-repolist: Add section filter tim.nordell 2016-03-04 23:24 ` [PATCH v2 2/8] ui-repolist: Remove conditional around print_header() tim.nordell 2016-03-04 23:24 ` [PATCH v2 3/8] ui-repolist: Create context structure for cgit_print_repolist() tim.nordell 2016-03-04 23:24 ` [PATCH v2 4/8] ui-repolist: Move HTML generation into helper functions tim.nordell 2016-03-04 23:29 ` [PATCH v2 5/8] ui-repolist: Factor out logic for emitting a new section heading tim.nordell 2016-03-04 23:30 ` [PATCH v2 6/8] ui-repolist: Remove assignment of section to repolist_ctx tim.nordell 2016-03-04 23:30 ` [PATCH v2 7/8] ui-repolist: Restructure internal logic to be more extensible tim.nordell 2016-03-04 23:30 ` [PATCH v2 8/8] ui-repolist: Support a trimmed view when several sections are present tim.nordell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).