This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author bochecha
Recipients bochecha, eric.araujo, eric.smith, pitrou, r.david.murray, tim.golden
Date 2010-08-14.11:29:23
SpamBayes Score 0.0
Marked as misclassified No
Message-id <1281785367.39.0.638466733307.issue9584@psf.upfronthosting.co.za>
In-reply-to
Content
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?
History
Date User Action Args
2010-08-14 11:29:27bochechasetrecipients: + bochecha, pitrou, eric.smith, tim.golden, eric.araujo, r.david.murray
2010-08-14 11:29:27bochechasetmessageid: <1281785367.39.0.638466733307.issue9584@psf.upfronthosting.co.za>
2010-08-14 11:29:25bochechalinkissue9584 messages
2010-08-14 11:29:23bochechacreate