Skip to content

Commit

Permalink
Add curly brace expansion support to fnmatch globs
Browse files Browse the repository at this point in the history
  • Loading branch information
treyhunner committed Oct 14, 2012
1 parent 6ed3400 commit e4807b1
Showing 1 changed file with 17 additions and 0 deletions.
17 changes: 17 additions & 0 deletions editorconfig/fnmatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def fnmatch(name, pat):
- ``?`` matches any single character
- ``[seq]`` matches any character in seq
- ``[!seq]`` matches any char not in seq
- ``{s1,s2}`` matches any of the strings given (separated by commas)

This comment has been minimized.

Copy link
@xuhdev

xuhdev Mar 10, 2013

Member

Do you think it's better to use something like {s1,s2,...,sn} or {s1,s2,...}? People may misunderstand that only two entries are allowed.

This comment has been minimized.

Copy link
@treyhunner

treyhunner Mar 10, 2013

Author Member

I think "matches any of the strings" should make it clear that any number of strings can be specified, but further clarification can't hurt.

Maybe {s1,s2,s3}? I think that's clear that any number of comma separated strings is supported.

This comment has been minimized.

Copy link
@xuhdev

xuhdev Mar 10, 2013

Member

I'm ok with this.

An initial period in FILENAME is not special.
Both FILENAME and PATTERN are first case-normalized
Expand Down Expand Up @@ -93,6 +94,22 @@ def translate(pat):
elif stuff[0] == '^':
stuff = '\\' + stuff
res = '%s[%s]' % (res, stuff)
elif c == '{':
j = i
groups = []
while j < n and pat[j] != '}':
k = j
while k < n and (pat[k] not in (',', '}') or pat[k-1] == '\\'):
k = k+1
groups.append(pat[j:k])
j = k
if j < n and pat[j] == ',':
j = j+1
if j >= n or j == i:
res = res + '{'
else:
res = '%s(%s)' % (res, '|'.join(map(re.escape, groups)))
i = j+1
else:
res = res + re.escape(c)
return res + '\Z(?ms)'

16 comments on commit e4807b1

@xuhdev
Copy link
Member

@xuhdev xuhdev commented on e4807b1 Oct 19, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we don't plan to create a new branch for this, right?

@treyhunner
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have created a new branch, but I did not. We can develop this in master. I've bumped the version to clarify that it is a development release.

I also added some fixes to v0.10.0 and created a v0.10.1 branch out of them so the Python core is now on PyPI.

@treyhunner
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot figure out how to translate this code to implement this behavior for the C core. The ec_fnmatch.c file is insurmountable for me right now.

This problem seems very non-trivial without the use of regular expressions.

@xuhdev
Copy link
Member

@xuhdev xuhdev commented on e4807b1 Mar 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is too hard for me, but it takes some time. I'll try to implement it as soon as possible. Maybe this week or next week.

@treyhunner
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xuhdev that would be awesome if you can manage it. Let me know if there's anything I can do to help.

@xuhdev
Copy link
Member

@xuhdev xuhdev commented on e4807b1 Mar 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that your code doesn't handle this case:

[a{\\,b}]
backslash = yes
editorconfig `pwd`/a\\

doesn't give backslash = yes.

@xuhdev
Copy link
Member

@xuhdev xuhdev commented on e4807b1 Mar 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@treyhunner
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And neither of our implementations seem to handle:

    [{a\,b,cd}]
    backslash  = yes
editorconfig $(pwd)/a,b

@treyhunner
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xuhdev I could make tests for these cases. Do you think both \ and , should be escapable with a \?

@xuhdev
Copy link
Member

@xuhdev xuhdev commented on e4807b1 Mar 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@treyhunner Yes, I think so. And I think } should also be escaped to match the character itself.

@xuhdev
Copy link
Member

@xuhdev xuhdev commented on e4807b1 Mar 10, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C core passes all new test cases now!

@treyhunner
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python core passes all tests now also. Should we update the versions to 0.11 and upgrade the plugins to use the new core libraries?

@xuhdev
Copy link
Member

@xuhdev xuhdev commented on e4807b1 Mar 10, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc of the C core hasn't update yet. Maybe after the update of the doc?

@xuhdev
Copy link
Member

@xuhdev xuhdev commented on e4807b1 Mar 10, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also an inconsistence between the descriptions of the meaning of entries in C core doc and the website. Which one we should use?

@treyhunner
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes let's update the C core docs to match the (now updated) Python ones. The website docs should probably be used since they probably more recent.

@xuhdev
Copy link
Member

@xuhdev xuhdev commented on e4807b1 Mar 10, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@treyhunner
Documentation updated.

Please sign in to comment.