Remove HTML from __() in load.php and wp_set_wpdb_vars()

[mordauk]

When WP checks to see if there is an error with the DB prefix, it outputs a localized error with HTML in it:

wp_die( __( '<strong>ERROR</strong>: <code>$table_prefix</code> in <code>wp-config.php</code> can only contain numbers, letters, and underscores.' ) );

It'd be better to use sprintf() and add the HTML after localization via placeholders:

wp_die( sprintf( __( '%sERROR%s: %s$table_prefix%s in %swp-config.php%s can only contain numbers, letters, and underscores.', '<strong>', '</strong>', '<code>', '</code>', '<code>', '</code>' ) ) );

[mordauk]

Replace HTML with placeholders

[SergeyBiryukov]

Why would that be better?

<strong>ERROR</strong> pattern is used in 11 more files. Replacing the tags with placeholders in this one string would be inconsistent and seems less readable to me.

[mordauk]

My main reason was following what Otto says here: ​http://ottopress.com/2012/internationalization-youre-probably-doing-it-wrong/

If it gets replaced here, I would replace it everywhere to be consistent, I just happened to notice it here while I was browsing the file.

[scribu]

Otto says:

Markup should be eliminated from your translated strings wherever possible.

It's always possible, technically. I'm pretty sure he meant "wherever it makes sense".

I don't think it makes sense here.

[mordauk]

Replying to scribu:

Otto says:

Markup should be eliminated from your translated strings wherever possible.

It's always possible, technically. I'm pretty sure he meant "wherever it makes sense".

I don't think it makes sense here.

Okay I can see that. I suppose it comes down to a matter of preference as well, I just prefer to never have HTML in a translated string.

[SergeyBiryukov]

I think it makes more sense to leave out HTML tags when there's no need for sprintf(), e.g. like <p> tags in help text (which is not the case here):
​http://core.trac.wordpress.org/browser/tags/3.4.1/wp-admin/custom-header.php#L104

Otto's example is essentially the same. It uses sprintf() for $number argument, not for HTML tags.

[mordauk]

Line 139 of custom-header.php is:

'<p>' . __( '<a href="http://codex.wordpress.org/Appearance_Header_Screen" target="_blank">Documentation on Custom Header</a>' ) . '</p>'

That seems like a perfect place to use sprintf(). I can definitely agree that my original patch might be unnecessary, but how do you feel about this?

[scribu]

Yes, that makes more sense. And if we want to enable translators to change the Codex URL, we can add it as a separate string.

[scribu]

We don't even need sprintf():

'<p><a href="' . __( 'http://codex.wordpress.org/Appearance_Header_Screen' ) . '" target="_blank">' . __( 'Documentation on Custom Header' ) . </a></p>'

but it does seem cleaner:

sprintf( '<p><a href="%s" target="blank">%s</a></p>',
  __( 'http://codex.wordpress.org/Appearance_Header_Screen' ),
  __( 'Documentation on Custom Header' )
);

[SergeyBiryukov]

The Codex links are intended to be localizable.

Splitting those strings in two would result in more strings to translate and would require more memory. Localized installs are already close to 32 MB memory limit (ticket:19831:14, ticket:19852:21, #19832).

I'd prefer to leave those strings as is, at least until gettext performance is improved (#17128, #17268).

If the links were to some standard non-localizable location (like the one in line 498), it would be fine:
​http://core.trac.wordpress.org/browser/tags/3.4.1/wp-admin/custom-header.php#L498

[mordauk]

Makes sense to me.