[patch] improved ignore pattern matching (#57637)
John A Meinel
john at arbash-meinel.com
Thu Dec 7 21:01:01 GMT 2006
John Arbash Meinel has voted +1 (conditional).
Status is now: Semi-approved
Comment:
I think you can do the extra help as something like "bzr help ignore-patterns". But certainly, having the basic changes installed are much better to start with.
I also prefer to have case-sensitive patterns on all platforms. That way if it works in one place it works everywhere.
One big thing, is that we shouldn't call the module 'glob.py'. Because that conflicts with the python builtin module 'glob.py'.
There is a typo here:
+ warning(u"'%s' not allowed withing regexp. Replacing with '%s'" %
+ (m, repl))
'not allowed within a regular expression.'
I think it might be easier to follow if we broke:
+ def test_char_groups(self)
So that it was multiple calls to 'self.assertMatches' rather than a giant list, that you try to decipher before you find out what is being done with it.
Imports are done in sorted order so:
from bzrlib.benchmarks import Benchmark
from bzrlib.workingtree import WorkingTree
+from bzrlib import ignores
'bzrlib' comes before anything else, so it should be:
+from bzrlib import ignores
from bzrlib.benchmarks import Benchmark
from bzrlib.workingtree import WorkingTree
If you are only using this in the test suite, I would make it private:
+ def flush_ignore_list_cache(self):
I don't think you intend to have it as a generally used function. And making it private gives us more flexibility to change it if we need to.
So you should rename glob.py to something else (globbing? glob_matcher?, I don't have any strong preference, we just need to avoid collisions with the standard lib).
And a couple of small cleanups. But it looks very close to me. I think it can be fast reviewed next time if someone else gives it a +1.
For details, see: http://bundlebuggy.aaronbentley.com/request/%3C456C786A.2060303%40gmail.com%3E
More information about the bazaar
mailing list