[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r919460 - filtering svn patch targets

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Wed, 10 Mar 2010 13:19:37 +0000

Stefan Sperling wrote:
> On Wed, Mar 10, 2010 at 10:36:13AM +0000, Julian Foad wrote:
> > This patch appears to have the filter reversed: it lets the user specify
> > paths that the filter should REMOVE, which IMO is not so useful.
>
> Why isn't that useful? I think either way (include and exclude) is useful.

I understand that TortoiseSVN wants the include/exclude functionality
available at the API. I wonder if "supply a list of patterns to
include" is really the best API. If I were writing a GUI that wanted to
show a list of what the patch is about to patch, I would want an API
that told me, first of all, what are the paths that this patch file is
going to patch. Maybe a callback that says: "I'm about to patch the
target file TARGET_ABSPATH. If you want, you can set TARGET_ABSPATH to a
different target, or set it to NULL if you don't want to apply this
file-patch at all." Using that, I could either get a list of all
targets in the patch (and make it a dry-run by returning TARGET_ABSPATH
= NULL on each callback), or control which targets are applied, by any
pattern- or list-matching scheme I like, or apply to different paths, or
just apply the patch normally by not providing a callback function at
all, all with the same simple API.

Is there a real practical need to have the include/exclude functionality
in "svn" as well? I can't think of a use that's important enough to
justify it. Can you? GNU patch doesn't have it (and it doesn't have an
"include-only" filter either).

> > (Using the word "filter" as a verb is ambiguous: it's never clear
> > whether you want to keep the big particles that the filter catches or
> > the liquid that passes through - or both, in two separate bowls.)
>
> Yes, we're aware of all this.
> See http://subversion.tigris.org/issues/show_bug.cgi?id=3599

I hadn't seen that.

> My plan is to add an --include-pattern option (at the CLI level as well
> as the API level), and if both --include-pattern and --exclude-pattern
> options have been specified, have the --exclude-pattern option operate on
> any files left after one or more --include-pattern options were used to
> limit the patch target list to a given list of files.

I just don't want us to make it more flexible than it needs to be.

Have you thought about consistency with other svn commands? How does
this relate to "--targets="? To the "ignore patterns"? Are the glob
patterns using the same syntax as auto-props?

> I'm also planning on renaming the 'glob_filter' parameter of the API
> to 'exclude_patterns' so it matches the terminology used by the CLI
> and does not say 'filter' anymore.

That sounds good.

Thanks.
- Julian
Received on 2010-03-10 14:20:11 CET

This is an archived mail posted to the Subversion Dev mailing list.