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

Amended Pull Request #14

Closed
wants to merge 5 commits into from
Closed

Conversation

craniumslows
Copy link

I changed it up to use the more greedy .* and that will match anything inside of reference which is what we want anyway.

Apologies for making a little mess. Confirmed it works with FF and Chrome isn't happy with WebKit Nightly

Marcus Wilson added 3 commits April 30, 2013 18:37
The regex wasn't escaping the parenthesis and it was only looking for a
left not the right.  Confirmed working in

Chrome  26.0.1410.65
Project broken for Firefox 20 and Webkit nightly 149402
Confirmed working with Firefox 20 + Firebug & Chrome 26. Couldn't get
it to run properly on Webkit nightly.

Regex isn't escaping the ) parenthesis or the bracket which is why it
wouldn't match.  After thinking about it and submitting a req for an
altered regex I realized that for these functions they don't care
what's in the backreference we just want to style it all.
I tested this in FF 20 with Firebug and Chrome 26 and it worked as
expected.  Issue provided the JS used http://js2coffee.org/ to just
translate it and it worked just fine.
@craniumslows
Copy link
Author

Let me know if I'm failing at this. 👊

Marcus Wilson and others added 2 commits April 30, 2013 19:34
I changed up the regex so that it will match with an opening single '
or double "  quote.

confirmed this works in Chrome 26 and FF 20 with Firebug
@adamschwartz
Copy link
Owner

Hey @craniumslows, thanks for all of your help so far. I'd like to merge these changes, particularly the parts which address #11 and #12. However, I'm seeing several issues when I test locally in Chrome 27.0.1453.93 on Mac OS. (See the screenshot below.)

If you can address these regressions I'd be happy to merge.

Also, note that the clear trick seems to be triggering an infinite recursive call.

log

@craniumslows
Copy link
Author

Hi

​Sorry about that ill be able to fix this late Sunday or early Monday. Thanks for emailing / messaging me! 

-craniumslows 


Sent from Mailbox for iPhone

On Wed, Jun 5, 2013 at 7:56 PM, Adam Schwartz notifications@github.com
wrote:

Hey @craniumslows, thanks for all of your help so far. I'd like to merge these changes, particularly the parts which address #11 and #12. However, I'm seeing several issues when I test locally in Chrome 27.0.1453.93 on Mac OS. (See the screenshot below.)
If you can address these regressions I'd be happy to merge.
Also, note that the clear trick seems to be triggering an infinite recursive call.

log

Reply to this email directly or view it on GitHub:
#14 (comment)

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

2 participants