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

Fix #4183 - exclude pattern containing delimiter used throws error #4193

Merged
merged 2 commits into from Jun 30, 2015

Conversation

alcohol
Copy link
Member

@alcohol alcohol commented Jun 29, 2015

Added test to reproduce #4183

Not sure on what the best way to fix it would be..

  • Escape any delimiter characters used in the pattern?
  • Use a different delimiter?

/cc @Seldaek @stof

@staabm
Copy link
Contributor

staabm commented Jun 29, 2015

Using a different delimiter does not sound like a good idea. someone will collide with it again ;-).

could we do a preg_quote instead?

@alcohol
Copy link
Member Author

alcohol commented Jun 29, 2015

I believe preg_quote would escape too much (I did consider it for a moment). I think escaping the delimiter is the most elegant solution. However, I'm not sure if my fix catches all scenarios.

@Seldaek
Copy link
Member

Seldaek commented Jun 30, 2015

That's why I use {} as delimiters always.. because they are already special chars that you need to escape either way, and don't cause fuckups like this. Just convert the # delimiter to {/} and it should fix this issue as far as I can tell.

@staabm
Copy link
Contributor

staabm commented Jun 30, 2015

👍

Seldaek added a commit that referenced this pull request Jun 30, 2015
Fix #4183 - exclude pattern containing delimiter used throws error
@Seldaek Seldaek merged commit 05c4911 into composer:master Jun 30, 2015
@alcohol alcohol deleted the fix-gitignore-parsing branch September 13, 2016 06:06
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.

None yet

3 participants