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

Squiz.Strings.DoubleQuoteUsage not unescaping dollar sign when fixing #1496

Closed

Conversation

michalbundyra
Copy link
Contributor

When double quotes are changed to single in content character escaped
dolar sign (\$) should be replaced to dolar sign ($) only.

Let me know if you need separate PR with this fix for v2.

When double quotes are changed to single in content character escaped
dolar sign (`\$`) should be replaced to dolar sign (`$`) only.
@gsherwood gsherwood changed the title [3.0] Hotfix DoubleQuoteUsageSniff - escaped dolar sign \$ Squiz.Strings.DoubleQuoteUsage not unescaping dollar sign when fixing Jun 6, 2017
@gsherwood
Copy link
Member

Thanks a lot for fixing this.

@gsherwood gsherwood closed this Jun 6, 2017
gsherwood added a commit that referenced this pull request Jun 6, 2017
@gsherwood gsherwood reopened this Jun 6, 2017
@gsherwood
Copy link
Member

I'm not sure why the status didn't change to merged, but I have merged it in.

@gsherwood gsherwood closed this Jun 6, 2017
@michalbundyra michalbundyra deleted the hotfix/double-quote-usage branch June 6, 2017 22:17
@pento
Copy link

pento commented Jun 24, 2017

@gsherwood: Would it be possible to get this fix backported to 2.9? I've been helping @jrfnl with the WordPress Core work, I've run into this bug during testing.

@jrfnl
Copy link
Contributor

jrfnl commented Jul 13, 2017

@gsherwood Regarding @pento's request - would it help if I created a PR for this for the 2.9 branch ?

gsherwood added a commit that referenced this pull request Jul 13, 2017
@gsherwood
Copy link
Member

Sorry. I did see this but didn't get around to it. I've backported it now.

@jrfnl
Copy link
Contributor

jrfnl commented Jul 13, 2017

@gsherwood Much appreciated! 👍

@pento
Copy link

pento commented Jul 13, 2017

Thanks @gsherwood, this works perfectly! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants