source-filter isn't getting applied

source-filter isn't getting applied

[njn2118]

Hi,

I have cgit installed and the source-filter isn't working on any of my
source files. I have Python and Pygments installed. I tried manually
running the script on some files, and it's giving back html
correctly.

Here's my /etc/cgitrc:

    scan-path=/home/git

    branch-sort=age
    repository-sort=age

    source-filter=/usr/local/lib/cgit/filters/syntax-highlighting.py
    about-filter=/usr/local/lib/cgit/filters/about-formatting.sh


My cgit is installed in /var/www/cgit, and I'm using cgit v0.10.2.

Let me know if you have any suggestions.

Nik

source-filter isn't getting applied

[john]

On Mon, Jul 28, 2014 at 05:14:43PM -0400, Nik Nyby wrote:
> I have cgit installed and the source-filter isn't working on any of my
> source files. I have Python and Pygments installed. I tried manually
> running the script on some files, and it's giving back html
> correctly.
> 
> Here's my /etc/cgitrc:
> 
>     scan-path=/home/git
> 
>     branch-sort=age
>     repository-sort=age
> 
>     source-filter=/usr/local/lib/cgit/filters/syntax-highlighting.py
>     about-filter=/usr/local/lib/cgit/filters/about-formatting.sh
> 
> 
> My cgit is installed in /var/www/cgit, and I'm using cgit v0.10.2.
> 
> Let me know if you have any suggestions.

What are the permissions on the script files?  How do you run them
manually?

Assuming that CGit is definitely reading that `cgitrc` file, I'd guess
that the permissions are not set correctly for the user `cgit` runs as
when called from your web server.

If all else fails, you can try moving CGit out of the way and replacing
it with something like this:

-- >8 --
#!/bin/sh
strace -o /tmp/cgit.strace /path/to/real/cgit "$@"
-- 8< --

Fwd: source-filter isn't getting applied

[njn2118]

The permissions on the script files are set to be executable by everyone:
-rwxr-xr-x

Thanks for the strace idea. I'm looking through the strace, but I haven't
seen any helpful mention of the filter scripts yet. I've attached an
abridged version of my strace with the middle of the file taken out. I
don't know if this is helpful or not. I'll let you know if I solve the
problem.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20140728/ebb6a45c/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cgit.strace
Type: application/octet-stream
Size: 30016 bytes
Desc: not available
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20140728/ebb6a45c/attachment-0001.obj>

Fwd: source-filter isn't getting applied

[john]

On Mon, Jul 28, 2014 at 11:59:10PM -0400, Nik Nyby wrote:
> The permissions on the script files are set to be executable by everyone:
> -rwxr-xr-x
> 
> Thanks for the strace idea. I'm looking through the strace, but I haven't
> seen any helpful mention of the filter scripts yet. I've attached an
> abridged version of my strace with the middle of the file taken out. I
> don't know if this is helpful or not. I'll let you know if I solve the
> problem.

If CGit was going to open a filter, it would do so after this line:

	write(1, "<td class='lines'><pre><code>", 29) = 29

So it looks like something in the configuration isn't being read
correctly, but I think you snipped the strace log before we could see
which config file it reads.

Fwd: source-filter isn't getting applied

[njn2118]

Great, this helped me debug the error. I looked at the part of the strace
where it was reading the /etc/cgitrc, and it looked like it was only
reading the first setting, even though the following settings are getting
applied. I'm not sure exactly why yet, but putting the source-filter
setting at the top of the /etc/cgitrc fixes the problem, and my source
files are now highlighted.

Now I'm dealing with the same problem with the about-filter, because
putting that setting at the top isn't giving me markdown highlighting.


On Tue, Jul 29, 2014 at 3:37 AM, John Keeping <john at keeping.me.uk> wrote:

> On Mon, Jul 28, 2014 at 11:59:10PM -0400, Nik Nyby wrote:
> > The permissions on the script files are set to be executable by everyone:
> > -rwxr-xr-x
> >
> > Thanks for the strace idea. I'm looking through the strace, but I haven't
> > seen any helpful mention of the filter scripts yet. I've attached an
> > abridged version of my strace with the middle of the file taken out. I
> > don't know if this is helpful or not. I'll let you know if I solve the
> > problem.
>
> If CGit was going to open a filter, it would do so after this line:
>
>         write(1, "<td class='lines'><pre><code>", 29) = 29
>
> So it looks like something in the configuration isn't being read
> correctly, but I think you snipped the strace log before we could see
> which config file it reads.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20140730/6c6b353d/attachment.html>

Fwd: source-filter isn't getting applied

[njn2118]

Nevermind about the about-filter. I'm reading the about-formatting.sh
script and my questions are all answered now.


On Wed, Jul 30, 2014 at 12:17 AM, Nik Nyby <njn2118 at columbia.edu> wrote:

> Great, this helped me debug the error. I looked at the part of the strace
> where it was reading the /etc/cgitrc, and it looked like it was only
> reading the first setting, even though the following settings are getting
> applied. I'm not sure exactly why yet, but putting the source-filter
> setting at the top of the /etc/cgitrc fixes the problem, and my source
> files are now highlighted.
>
> Now I'm dealing with the same problem with the about-filter, because
> putting that setting at the top isn't giving me markdown highlighting.
>
>
> On Tue, Jul 29, 2014 at 3:37 AM, John Keeping <john at keeping.me.uk> wrote:
>
>> On Mon, Jul 28, 2014 at 11:59:10PM -0400, Nik Nyby wrote:
>> > The permissions on the script files are set to be executable by
>> everyone:
>> > -rwxr-xr-x
>> >
>> > Thanks for the strace idea. I'm looking through the strace, but I
>> haven't
>> > seen any helpful mention of the filter scripts yet. I've attached an
>> > abridged version of my strace with the middle of the file taken out. I
>> > don't know if this is helpful or not. I'll let you know if I solve the
>> > problem.
>>
>> If CGit was going to open a filter, it would do so after this line:
>>
>>         write(1, "<td class='lines'><pre><code>", 29) = 29
>>
>> So it looks like something in the configuration isn't being read
>> correctly, but I think you snipped the strace log before we could see
>> which config file it reads.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20140730/e7dfc1d7/attachment.html>

Fwd: source-filter isn't getting applied

[john]

On Wed, Jul 30, 2014 at 12:17:26AM -0400, Nik Nyby wrote:
> Great, this helped me debug the error. I looked at the part of the strace
> where it was reading the /etc/cgitrc, and it looked like it was only
> reading the first setting, even though the following settings are getting
> applied. I'm not sure exactly why yet, but putting the source-filter
> setting at the top of the /etc/cgitrc fixes the problem, and my source
> files are now highlighted.

I suspect that was just strace truncating the data, but at least it
pointed to the answer!

Global settings get frozen for each repository when it is added, so you
should generally do all the configuration near the top of the file and
then add repositories (either explicity or with "scan-path=...") near
the bottom.

> Now I'm dealing with the same problem with the about-filter, because
> putting that setting at the top isn't giving me markdown highlighting.
> 
> 
> On Tue, Jul 29, 2014 at 3:37 AM, John Keeping <john at keeping.me.uk> wrote:
> 
> > On Mon, Jul 28, 2014 at 11:59:10PM -0400, Nik Nyby wrote:
> > > The permissions on the script files are set to be executable by everyone:
> > > -rwxr-xr-x
> > >
> > > Thanks for the strace idea. I'm looking through the strace, but I haven't
> > > seen any helpful mention of the filter scripts yet. I've attached an
> > > abridged version of my strace with the middle of the file taken out. I
> > > don't know if this is helpful or not. I'll let you know if I solve the
> > > problem.
> >
> > If CGit was going to open a filter, it would do so after this line:
> >
> >         write(1, "<td class='lines'><pre><code>", 29) = 29
> >
> > So it looks like something in the configuration isn't being read
> > correctly, but I think you snipped the strace log before we could see
> > which config file it reads.

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

[jamie.couture]

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.

Signed-off-by: Jamie Couture <jamie.couture at 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

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

[john]

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.couture at 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 at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit

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

[jamie.couture]

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:
> > 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?
> 

Yes, I'm unfairly catering to one setting and not the remaining
filters: most people have this issue with source-filter.  I've seen
this question asked enough times I thought it was time something was
done to avoid the subtly.

> 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:
> 

Yes, yet people still make this error.  We can try harder to make it
less error prone.  This patch doesn't try harder for the other
options, only to the one solution so a more thoughtful patch is
needed.

> 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.

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.

> 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?  

> 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.

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

[jamie.couture]

Actually I have another idea.  Why not defer scan path until the rest of
the config file is processed?  Thus cgit_add_repo() can do its setup
properly.  I'll give it another shot in a bit.

couture
On Jul 30, 2014 4:47 PM, "Jamie Couture" <jamie.couture at gmail.com> 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:
> > > 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?
> >
>
> Yes, I'm unfairly catering to one setting and not the remaining
> filters: most people have this issue with source-filter.  I've seen
> this question asked enough times I thought it was time something was
> done to avoid the subtly.
>
> > 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:
> >
>
> Yes, yet people still make this error.  We can try harder to make it
> less error prone.  This patch doesn't try harder for the other
> options, only to the one solution so a more thoughtful patch is
> needed.
>
> > 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.
>
> 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.
>
> > 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?
>
> > 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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20140730/c7179c32/attachment-0001.html>

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

[john]

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.

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

[jamie.couture]

On Wed, Jul 30, 2014 at 10:14:17PM +0100, John Keeping wrote:
> > 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.
> 

Okay.

> > 
> > 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.
> 

You got me.

This is a situation I have not considered to be a use-case others
may actually *want*.  I am not aware of anyone who does this in
practise, I know I do not do anything like that, but in
consideration of being most flexible we would lose that ability.

> > > 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.

Indeed.  A solution, even to try and defer processing of
scan-path/scan-tree after we do parse_configfile() is awkward and
would defeat the use-case you mention above.

I'm not sure if the following adds much more value to the docs,
because it assumes people are going to configure cgitrc in a
specific way.

Maybe it would be better to include a _troubleshooting_ section to
README?  There we could repeat and assert that any global setting
defined after scan-path will not be defined.

-- 8< --
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -378,6 +378,10 @@ scan-path::
        before the scan-path directive will be applied to each repository.
        Default value: none. See also: cache-scanrc-ttl, project-list,
        "MACRO EXPANSION".
++
+[NOTE]
+    It is general practise to define scan-path at the end of a cgitrc
+    file to ensure the global settings are used.

 section::
        The name of the current repository section - all repositories defined
-- 8< --

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

[jamie.couture]

On Wed, Jul 30, 2014 at 09:05:38PM -0400, Jamie Couture wrote:
> You got me.
> 
> This is a situation I have not considered to be a use-case others
> may actually *want*.  I am not aware of anyone who does this in
> practise, I know I do not do anything like that, but in
> consideration of being most flexible we would lose that ability.
> 

*in consideration of being most flexible we would lose that ability
with this patch, and can turf it.*

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

[Jason]

I would be okay with both scan-path deferment or simply augmenting the
documentation. I don't like the commit of this thread though.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20140801/23e46a07/attachment.html>