On Wed, Jul 30, 2014 at 02:53:21PM -0400, Jamie Couture wrote: > When parsing cgitrc users that place 'source-filter' setting after > 'scan-path' find their source filter is never run. > > scan-path=/home/git/repositories > source-filter=/usr/local/lib/cgit/filters/syntax-highlighting.sh > > ui-tree.c will print out our repository objects, but will only consider > ctx.repo->source_filter struct and not checking ctx.cfg.source_filter > > The value would have been set in shared.c, via cgit_add_repo() but this > isn't the case when using scan-path, because we have not yet processed > the source-filter line in our cgitrc, and thus the source_filter struct > will not be initialized. > > Checking the global configuration in ui-tree.c is necessary to avoid an > issue where users declare the source filter after scan-path.
I think this is OK because we only fall back if repo->source_filter is unset and there is no way to unset the source filter for a repository AFAICT. But if we do this for source-filter, why not also about-filter, email-filter and commit-filter? The documentation already makes it clear that settings should be configured before "scan-path", and scan-path isn't special here you could equally well demonstrate the same effect with: repo.url=my-repository source-filter=/path/to/source-filter.sh I'm not convinced that this change makes things less confusing, it just means that "everything except source-filter must be configured before adding repositories", which seems more confusing. The full list of captured settings is: ret->section = ctx.cfg.section; ret->snapshots = ctx.cfg.snapshots; ret->enable_commit_graph = ctx.cfg.enable_commit_graph; ret->enable_log_filecount = ctx.cfg.enable_log_filecount; ret->enable_log_linecount = ctx.cfg.enable_log_linecount; ret->enable_remote_branches = ctx.cfg.enable_remote_branches; ret->enable_subject_links = ctx.cfg.enable_subject_links; ret->max_stats = ctx.cfg.max_stats; ret->branch_sort = ctx.cfg.branch_sort; ret->commit_sort = ctx.cfg.commit_sort; ret->module_link = ctx.cfg.module_link; ret->readme = ctx.cfg.readme; ret->about_filter = ctx.cfg.about_filter; ret->commit_filter = ctx.cfg.commit_filter; ret->source_filter = ctx.cfg.source_filter; ret->email_filter = ctx.cfg.email_filter; ret->clone_url = ctx.cfg.clone_url; And I don't think we want to change all of these to allow the config file to be out-of-order. Perhaps we would be better off making the documentation clearer rather than trying to fix some particular cases? > Signed-off-by: Jamie Couture <jamie.cout...@gmail.com> > --- > ui-tree.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/ui-tree.c b/ui-tree.c > index e4c3d22..f52f15c 100644 > --- a/ui-tree.c > +++ b/ui-tree.c > @@ -22,6 +22,7 @@ static void print_text_buffer(const char *name, char *buf, > unsigned long size) > { > unsigned long lineno, idx; > const char *numberfmt = "<a id='n%1$d' href='#n%1$d'>%1$d</a>\n"; > + struct cgit_filter *source_filter = NULL; > > html("<table summary='blob content' class='blob'>\n"); > > @@ -45,11 +46,17 @@ static void print_text_buffer(const char *name, char > *buf, unsigned long size) > } > > if (ctx.repo->source_filter) { > + source_filter = ctx.repo->source_filter; > + } else if (ctx.cfg.source_filter) { > + source_filter = ctx.cfg.source_filter; > + } > + > + if (source_filter) { > char *filter_arg = xstrdup(name); > html("<td class='lines'><pre><code>"); > - cgit_open_filter(ctx.repo->source_filter, filter_arg); > + cgit_open_filter(source_filter, filter_arg); > html_raw(buf, size); > - cgit_close_filter(ctx.repo->source_filter); > + cgit_close_filter(source_filter); > free(filter_arg); > html("</code></pre></td></tr></table>\n"); > return; > -- > 1.9.1.377.g96e67c8 > > _______________________________________________ > CGit mailing list > CGit@lists.zx2c4.com > http://lists.zx2c4.com/mailman/listinfo/cgit _______________________________________________ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit