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

Consider replacing regexs with actual parsers #33

Open
hamtie opened this issue Nov 25, 2013 · 1 comment
Open

Consider replacing regexs with actual parsers #33

hamtie opened this issue Nov 25, 2013 · 1 comment

Comments

@hamtie
Copy link
Contributor

hamtie commented Nov 25, 2013

Don't know how much time you want to invest in this idea, but the cdn replacements should really be done with proper tokenizers, not regexs.

See this popular stackoverflow post about the issue:
http://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags

This change however would require adding parsers for each html-like template types, like ejs, but presumably those already exist somewhere and maybe could be bootstrapped? Otherwise, this is just going to get gnarly the more formats you try to support.

I might try to help out on this idea when I have more time, but pretty slammed atm. Splitting on elements and replacing the first attribute seems to be working safely now (if you pull my latest request), but I don't really trust it fully.

@johnnyhalife
Copy link
Contributor

Hey @fouasnon,

Thanks for all your contributions, I think that before going any futher with the development we should start working on unit-test for the whole package and then think of improving it.

We reached a point that bugs are introduced and there're many people contributing to this, so let's start adding tests =)

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

No branches or pull requests

2 participants