[MERGE] is_ignored improvements...

[Robert Collins]

On Thu, 2006-05-18 at 19:18 +0200, Jan Hudec wrote:
> On Fri, May 19, 2006 at 00:14:08 +1000, Robert Collins wrote:
> > Heres my alternative patch, finished up:
> > 
> > I currently prefer this over the original patch you prepared because of
> > the preservation of API. I dont think many users will have more than 100
> > regexs, and it should perform just fine.
> > 
> > What do you think? Does this make the future improvements you are
> > working on harder? Is it a problem to take this variation?
> 
> I certainly isn't a problem. I can combine the patches later when/as needed.

Ok

> However I have some comments, below in the patch. The most important is
> probably that you should use the fact, that fnmatch.translate will never
> generate capturing (nor non-capturing) groups, and simplify the combiner.

Thanks for the review. With respect to the complication translation of
regex's - I did look in fnmatch.translate and see they dont ever use
capturing groups. However, its not part of the published api that it
wont *ever* do that - so I wrote the translator to the published
interface which is simply
--
fnmatch.translate = translate(pat)
    Translate a shell PATTERN to a regular expression.
    
    There is no way to quote meta-characters.
--

I felt it was risky to assume that the fnmatch translation internals
would never change. We could change it to do a compile, assert that
groups is 0, and after that do a slice, which will make the return value
from _combine_ignore_rules a little simpler (list rather than regex),
but at the cost of forcing a dpendence on the internals of translate.

> Also I think you should add a blackbox test that actually creates
> a 1000-lines long .bzrignore and checks some files matched by patterns
> towards the end are ignored and proper pattern is reported for them. I had
> such test there simply creating patterns '*.0' through '*.999' and matching
> a few files like 'foo.333' and 'bar.987' or somesuch. This will test that the
> regexp engine will actually handle the patterns as split and report correct
> group matches.

Well, I have a smoke test for 198 patterns, I can certainly modify that
to be more in-depth. I dont agree about it being black box though -
theres nothing in the ui that relates to the # of ignore rules.

Thanks for the review - you caught a bunch of cruft I should have known
better than to leave .... and I'll go fix that now. [I shouldn't send in
patches at 1am :)].

Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060519/2a0516e7/attachment.pgp