[MERGE] Rule-based preferences (EOL part 1 of 3)

[Martin Pool]

On Fri, May 16, 2008 at 6:02 PM, Ian Clatworthy
<ian.clatworthy at internode.on.net> wrote:
> I'm now happy enough with my end-of-line conversion work to begin
> putting the pieces up for review. There are 3 altogether:
>
> 1. Rule-based preferences
> 2. Content filtering
> 3. The eol plugin

I think that's a good way to split it.

The documentation of globs and ini file syntax in the help topic is
good but maybe either redundant or not in quite the right place
because the same syntax is used in other places, and really the
example may be enough to give people the idea.

+# The cache of branch-specific rule searchers
+_branch_searchers = {}
+
+
+def iter_search_rules(branch, path_names, pref_names=None,
+    _default_searcher=_per_user_searcher):
+    """Find the preferences for filenames in a branch.

Having a cache in a global variable seems like a something we would
generally try to avoid, because it's a bit uncontrolled and likely to
either retain things you don't want, or not cache things you do want.
In this case it seems like it should be held by the Branch?

And this method too seems like it would happily be a Branch method?

+                ini_file = branch.control_files.get('branch.rules')

Code not in a Branch implementation should rarely if ever be relying
on the existence of a particular file inside the Branch directory, or
even that the Branch has control files at all.  This is just the kind
of code I've been trying to kill in the controlfiles patches.  If you
are going to do it, the Transport is the right way.

More importantly - this seems like the first mention of having rules
inside the branch/ directory (unless I just missed a previous thread?)
and since it's quite substantial you should call it out more in your
review.

I don't think putting a rules file in there is a good idea; in general
everything in there should be determined solely by the branch format.
Adding a new one would require a branch format bump, and also code to
move them around, merge them, diff them, etc.  I think it would be
much more appropriate to put it in the working tree as .bzrrules.
(I'm happy to explain more about why this is so either in mail or
elsewhere if you want but won't flog it for now.)

The actual searchers and their tests look basically reasonable.

bb: resubmit

-- 
Martin <http://launchpad.net/~mbp/>