Fix many translatable strings
hass - August 1, 2008 - 20:38
| Project: | Project |
| Version: | 5.x-1.x-dev |
| Component: | Projects |
| Category: | bug report |
| Priority: | normal |
| Assigned: | dww |
| Status: | closed |
Description
Here is patch to fix many translatable strings to be more consistent with core, more context sensitive (this part needs nevertheless more fixes - for e.g in class project_views_handler_filter_project_type_vid), fixes some missing periods and missing words, etc.
Both patches apply are against aclight's SYN.
| Attachment | Size |
|---|---|
| project_d6_translatable_strings.patch | 16.04 KB |
#1
#2
As I said over in #157694: Upgrade project module to 6.x, let's get this fixed in the D5 version first.
#3
Agreed, let's fix these strings in D5. Thanks.
One quick thing I noticed skimming the patch:
@@ -167,7 +167,7 @@// If set, the project_release_download_base must end with a '/'
if (!empty($form_state['values']['project_release_download_base'])) {
if (substr($form_state['values']['project_release_download_base'], -1) != '/') {
- form_set_error('project_release_download_base', t('The %download_base_setting should end with a slash.', array('%download_base_setting' => t('Download link base URL'))));
+ form_set_error('project_release_download_base', t('The <em>Download link base URL</em> should end with a slash.'));
}
}
}
A) that puts the look and feel of your site in the hands of the translations, not the themer, which seems wrong. % == theme('placeholder'). Maybe on my site, I like to make my placeholders bold, not italic. So, I implement my own theme_placeholder(), but now the German translation put some placeholders in em. I thought it was always a bad idea to have raw HTML in translatable strings.
B) Your way also means that you then have to translate "Download link base URL" both inside this string, and where it separately appears. That's why I'm in the habit of referring to UI elements that are already translated elsewhere via placeholders, so that you only have to translate the name once. Saves time for translators (I thought) and improves consistency (so the setting name is forced to always be translated the same way everywhere it appears).
But, I'm not a translator ;) so if you have another explanation, I'd be interested in hearing it.
Cheers,
-Derek
#4
We could write
t('The "Download link base URL" should end with a slash.')to solve the theme placeholder issue. No problem.See http://api.drupal.org/api/function/t/6 about example of incorrect usage of t(). It explains that the string shouldn't be moved outside the main string. If we do this we will end up with context insensitive strings what could make more headaches... nevertheless it looks like a "reusable" string. It could simply break other things...
#5
",<em>and<strong>(among others) are all possible placeholder delimiters a themer might want to use. I don't see how putting this in the hands of the translators is ever a good idea. That's part of the point of %foo being automatically run through theme('placeholder') for you.That API comment makes sense for links, since the link title wants a little context along with the rest of the sentence you're translating. However, it seems that if "Download link base URL" is already a t()'ed string when it appears directly in the UI, then the argument about "context insensitive strings" doesn't count. You already have to translate "Download link base URL" on its own, without any other context, since it's the title of some form element. Given that, I don't see why it's an improvement to have to translate it N times: every occurrence of the name of that form element in help text, etc. It really is just a placeholder as far as the help text is concerned.
Could you provide a little more evidence/explanation of "It could simply break other things..."? I don't see how.
Thanks,
-Derek
#6
<em>in strings seems to be ok, see http://drupal.org/node/194962#comment-1065964#7
To quote the comment you just linked:
3. If you'd use it like <em>@value</em> then it is indeed a bad idea. If you just emphasize some part of the string which is not a placeholder...This is markup to delimit a placeholder, the name of another setting, so this falls under Gabor's definition of "indeed a bad idea". And, as usual, you haven't replied to my other points or questions. ;)
#8
I have asked in the comment http://drupal.org/node/194962#comment-1065924 point 3 about the change
t('The <em>Download link base URL</em> should end with a slash.')and he answered that EMs are ok in such a case... let's aks Gabor about your above detailed questions... I understand well what you mean above, but I have no real solution how we should handle this for future... :-)#9
#B is easy to answer... the main problems coming up here are context sensitive strings. Yes we may need to translate one string twice or more often, but if you need to know what the meaning of the placeholder is you are lost if there is only a variable. You cannot say if the string is male/female/it and so... this is very important for translators. Different languages have different rules... :-)
#10
dww: re #3: I see your points in consistent placeholder markup/design and consistent translations for UI elements. Unfortunately some languages (might) need to add suffixes to the UI labels when they appear with a sentence. For example, in Hungarian "with something" translated to a "-val" or "-vel" suffix on the something, that is the corresponding form to the "with" word. So although we could translate in a style like "you can save it using the %name button" (so that we avoid "with" and therefore the suffix on the word), but it is twice as long and is not at all natural to Hungarian readers. The Localization API guide we recently put together documents that unnecessary markup should be removed from t(), but when you remove actual text at the same time, it could easily hurt translations. It might not make it impossible to translate (such as this case, when you refer to a button name, not moving out part of the sentence flow even in the English original), but it does make it harder and the translations more uglier.
#11
Roled patch for D5.
Please leave some time to update translation files before 1.3 release.
#12
Chasing... no code changes, only lines have changed.
#13
Any way to get the above D5 patch committed or the D6 first and then the D5?
Here is a re-role of the D6 patch.
#14
A) This breaks the spacing between sentences:
define('PROJECT_RELEASE_VERSION_FORMAT_HELP', t('Available variables are: %api, %major, %minor, %patch, %extra.') . t("The percent sign ('%') at the front of the variable name indicates that a period ('.') should be inserted as a delimiter before the value of the variable. The '%' can be replaced with a hash mark ('#') to use a hyphen ('-') delimiter, or with an exclaimation point ('!') to have the value printed without a delimiter. Any variable in the format string that has no value will be removed entirely from the final string.") .' '. PROJECT_RELEASE_VERSION_FORMAT_VALID_MSG);I replaced it with this (a single t() seems better now):
define('PROJECT_RELEASE_VERSION_FORMAT_HELP', t("Available variables are: %api, %major, %minor, %patch, %extra. The percent sign ('%') at the front of the variable name indicates that a period ('.') should be inserted as a delimiter before the value of the variable. The '%' can be replaced with a hash mark ('#') to use a hyphen ('-') delimiter, or with an exclaimation point ('!') to have the value printed without a delimiter. Any variable in the format string that has no value will be removed entirely from the final string.") .' '. PROJECT_RELEASE_VERSION_FORMAT_VALID_MSG);B) What's the problem here?
--- project.module 15 Jan 2009 00:29:29 -0000 1.329
+++ project.module 16 Jan 2009 05:55:51 -0000
@@ -713,7 +713,7 @@ function project_projects_select_options
function project_quick_navigate_form() {
$uris = NULL;
- $projects = array_merge(array(0 => t('<select a project>')), project_projects_select_options($uris, FALSE, 'node/'));
+ $projects = array_merge(array(0 => t('- Select a project -')), project_projects_select_options($uris, FALSE, 'node/'));
$form = array();
$form['project_goto'] = array(
'#type' => 'select',
I thought stuff like
<none>was the standard formatting for select elements like this... Are there examples in core of- None -like you've got it?C) The D5 patch broke some functionality by using the D6 API for url():
$form['version_login'] = array('#value' => t('<a href="@login">Login</a> or <a href="@register">register</a> to modify the filter.', array('@login' => url('user/login', array('query' => $destination)), '@register' => url('user/register', array('query' => $destination)))));In D5, that needs to be:
$form['version_login'] = array('#value' => t('<a href="@login">Login</a> or <a href="@register">register</a> to modify the filter.', array('@login' => url('user/login', $destination), '@register' => url('user/register', $destination))));D) The D6 patch didn't apply cleanly, so I had to resolve conflicts.
E) I know t("Don't mess") is better than t('Don\'t mess'), but why do you care if it's t("Something") or t('Something') when there's no escaping involved?
After reviewing and testing, fixing the above, and leaving out the hunk from (B), I committed to HEAD and DRUPAL-5.
Thanks for helping,
-Derek
#15
A) Ok.
B) Yes in core, CCK started using it, but I'm not sure if there is a general rule for this.
C) Sounds like a backporting bug as I applied the D6 patch to D5, fixed all hunks and forgotten about this API change. Sorry.
D) last time I made it - it worked for me. Maybe the last changes broke it.
E) This is a code style fix. It's written somewhere that we should not use double quotes - if not really required.
#16
On B: core does use
<none>as expected input for stuff, but is it not translated. Whatever language you are in, you are expected to enter<none>. Otherwise core uses- Please choose -and other similar constructs. Latest versions of Drupal 6 and 7 whitelist allowed HTML tags in translatable strings, and<none>is not among the allowed tags (neither is<select a project>). So if you are using HTML tag looking text in translatable strings, you should at least escape them. I'd say better standardize on stuff which does not look like HTML tags.#17
@Gabor: thanks for the input, that makes sense.
I'll apply the last hunk to both branches when I have the chance.
#18
There was a spot in project_release that was using
<none>for a similar project selector drop-down, which I fixed, too. Committed both to HEAD and DRUPAL-5.#19
Aside,
- Please choose -is also much easier to translate then single word strings like "none" - a least in German... THX.#20
Automatically closed -- issue fixed for 2 weeks with no activity.