[PATCH] ui-tree.c: check source filter if set globally

[John Keeping]

On Wed, Jul 30, 2014 at 04:47:17PM -0400, Jamie Couture wrote:
> On Wed, Jul 30, 2014 at 08:23:21PM +0100, John Keeping wrote:
> > On Wed, Jul 30, 2014 at 02:53:21PM -0400, Jamie Couture wrote:
> > 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.
> > 
> 
> Well, 'everything except source-filter' isn't really the case.

It is the case that all repo-specific settings except source-filter
would be ignored if they come after the repository is loaded.

> To be clear, the behaviour is that we setup the repo as you pointed
> out below, but because we didn't process the appropriate filter
> lines yet we did not instantiate the stuct. A full pass of the 
> configuration has not happened.  We would have to make two passes to
> know about all filter options, and then copy those settings during
> scan-path if that is also defined. 
> 
> I think better wording would be 'everything should to be declared
> before scan-path and scan-tree', which we know well enough, but
> users still run into that issue.
> 
> Users should not have to worry about where they define things.

Why not?  They can define different settings for different repositories
by interleaving the settings and repositories.

> > 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.
> > 
> 
> Maybe we do if users are having issues?  

Currently we support something like this:

	scan-path=/path/to/world-a
	source-filter=/path/to/source-filter.sh
	scan-path=/path/to/world-b

Where the source filter isn't applied to "world-a".  I don't know if
anyone makes use of this, but the current scheme does give quite a lot
of flexibility.

> > Perhaps we would be better off making the documentation clearer rather
> > than trying to fix some particular cases?
> > 
> 
> I'm okay with this change, as long as it avoids problems for the
> user. I don't think it introdces any sort of unwanted or unexpected
> behaviour.  However, it should try to be more complete with the
> remaining filter options.
> 
> I'm fine with this being turfed, but we do get a lot of questions
> about scan-path problems.

I tend to think that shows that the documentation is lacking rather than
the implementation, but I dunno.  Maybe I just don't want to deal with
making sure that all repo variables fall back to the global one if it's
set after scan-path or repo.url.