Skip to content

Conversation

chuckhendo
Copy link

Fixes #20

This is a first attempt at this, and it appears to be working fine. The biggest downside here is that I've got it checking for an extension in order to know when to look for a closing parentheses. Therefore, it will fail in the following situations:

  1. url('test.jpg(1)') - parentheses in the extension. This one would have failed prior to this PR as well
  2. url('test') - no extension. This is a new situation that it would fail in

There were not any tests to test for scenario 2, so I don't know how important that is (I'm not even sure if it's allowed in the spec...). If we really need it to pass in scenario 2, we'll need to look into matching pairs or recursive regex, neither of which JS supports out of the box.

//cc @s0ph1e

@coveralls
Copy link

coveralls commented Jan 7, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 1cd6689 on chuckhendo:parentheses-support into 686983f on website-scraper:master.

@s0ph1e
Copy link
Member

s0ph1e commented Jan 9, 2018

Hi @chuckhendo
I think we need to find a solution which will correctly handle both scenarios (with parentheses in the extension and without extension)
Despite there are no tests for second one, it is quite common in css

@s0ph1e
Copy link
Member

s0ph1e commented Jan 10, 2018

Hey @chuckhendo
I found solution how to handle both scenarios (see #22), will go with it for now.
so I'm closing this PR.

Thanks for contribution!

@s0ph1e s0ph1e closed this Jan 10, 2018
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.

3 participants