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

Update from backtick quote to single quote #9332

Merged
merged 1 commit into from
Aug 24, 2018

Conversation

Rahmon
Copy link
Contributor

@Rahmon Rahmon commented Aug 24, 2018

Description

The string "Change the block type after adding a new paragraph" doesn't show for translate and the only diference is the backtick quote use instead of single quote. But I don't know if that is the real cause. Anyway for codding standards I changed to single quote.

Types of changes

Bug fix.

@@ -101,7 +101,7 @@ const blockShortcuts = {
},
{
keyCombination: '/',
description: __( `Change the block type after adding a new paragraph.` ),
description: __( 'Change the block type after adding a new paragraph.' ),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say that's probably the cause of the translation issue and we might want to fix it in the babel plugin cc @aduth

But I agree that there's no real reason to use backticks here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering that the template strings are targeted specifically at variable interpolation, something we explicitly cannot support in the i18n string extraction, it's not entirely surprising to me that I'd not have included support for them in the original implementation.

Usage like this seems valid to parse, but also as pointed out, the template string is not necessary here. Easy to overlook though. Maybe the other option is to add a lint rule to forbid template strings as argument to __ ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense if the eslint rule is easy to add, I'm all for it 👍

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this change, I'm approving the PR and merging regardless of the way we'd fix the i18n extraction issue.

@youknowriad youknowriad merged commit 65f6072 into WordPress:master Aug 24, 2018
@Rahmon Rahmon deleted the patch-2 branch August 24, 2018 23:32
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