Mathieu Bridon <boche...@fedoraproject.org> added the comment:
Wow, I certainly didn't expect to generate so much controversy. :-/
First of all, thanks for the comments on the patch Antoine and David.
> I don't get what the purpose of these two lines is. Forbid empty patterns?
I copy-pasted the handling of the '[' character in the same file, and adapted
it. This test was to properly handle sequences like '[]]', and you are right,
it has nothing to do in this patch, I just forgot to remove it.
> You probably mean "while j < n" instead of "while i < n".
Yes, that's a typo. :-/
> Regardless, it's simpler to use "j = pat.find('}', j)".
I know, I just thought I would try to remain consistent with the way the '['
char was handled.
> You should also add a test for unmatched braces. Currently:
I realised that after submitting the patch, yes. Actually, there are several
other cases that I didn't properly handle, like a closing brace without a
matching opening brace, or even nested braces (which are perfectly acceptable
in the context of a shell like Bash).
I'm working on an improved patch that would correctly handle those cases (with
much more unit tests!), I guess I just hit the submit button too quickly. :)
---
Now, about whether or not this is appropriate in fnmatch, I agree with David
that if we want to remain really consistent with shell implementations, then
fnmatch probably isn't the appropriate place to do so.
In this case, I guess the correct way to implement it would be to expand the
braces and generate several patterns that would all be fed to different fnmatch
calls?
Implementing it in fnmatch just seemed so convenient, replacing the braces with
'(...|...)' constructs in a regex.
People seem to agree that a thread on python-ideas would be good to discuss
this change, but this ticket already generated some discussion. Should I start
the thread on the mailing-list anyway or is this ticket an appropriate forum
for further discussion?
----------
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue9584>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
