Ensure matchers work without attribute quotes.

OverZealous / cdnizer

Node module for replacing local links with CDN links, includes fallbacks and customization

MIT License

52 stars 24 forks source link

Ensure matchers work without attribute quotes. #4

Closed kasperisager closed 10 years ago

kasperisager commented 10 years ago

The matchers currently completely ignore minified HTML where quotes are stripped from the attributes. This PR adjusts the matchers to make quotes optional and also adds some test cases to make sure everything works.

OverZealous commented 10 years ago

I'm not sure I agree with the need for this PR.

It would be extremely simple to run cdnizer before minifying the HTML, which is more the correct answer.
Even without that, you can customize the matchers by adding your own via options.matchers.

I still might be willing to accept the change, but for some reason this PR modifies all of the tests, rather than adding new tests to test for a lack of quotes. I really don't like that. There's no reason to change the existing tests in this case.

I set the tests up to be very modular, it should be easy to add new fixtures and expected outputs and add new tests to run over those separately.

kasperisager commented 10 years ago

The reason why you need this is pretty simple: The regular expressions are now as solid as they should be. They no longer depend on quotes and will match both minified and non-minified HTML. Defensive programming and all that.

I do agree that I should make a new set of tests rather than extend the existing ones. They're on their way!

kasperisager commented 10 years ago

All set. Would you prefer a squashed commit?

OverZealous commented 10 years ago

Yeah, that would be better. Thanks for the changes.

kasperisager commented 10 years ago

Pleasure! :+1:

OverZealous commented 10 years ago

All set, the updated version is published to npm.