Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Escape filenames which looks like regexp #9

Merged
merged 2 commits into from
Oct 24, 2019

Conversation

JanBednarik
Copy link
Contributor

Hi, we have issue with files containing square braces in name. glob is reading them as regular expressions and crashes when expression is invalid. Filename like relint "foo[a-b].yml" is fine and relint "foo[b-a].yml" is crashing. To fix this filenames has to be escaped: https://docs.python.org/3/library/glob.html#glob.escape

When implementing fix, I had to improve tests a bit. First change is proper patching of sys.argv and sys.stdin so tests does not have side effects on other tests.

And second change is rewrite of test_main_execution_with_diff to be reliable and doesn't break on changes in test_relint.py.

Copy link
Owner

@codingjoe codingjoe left a comment

Choose a reason for hiding this comment

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

@JanBednarik thanks for contributing. Interesting bug you found, good catch.
I believe the bugfix you proposed is perfectly fine, but please read my comment below. I'd like to split your improvement into a separate commit.
Thanks Joe 👍

test_relint.py Show resolved Hide resolved
Copy link
Owner

@codingjoe codingjoe left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @JanBednarik great contribution!

@codingjoe codingjoe merged commit c670676 into codingjoe:master Oct 24, 2019
@codingjoe
Copy link
Owner

Released in 1.1.1 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants