Fix many translatable strings

As I said over in #157694: Upgrade project module to 6.x, let's get this fixed in the D5 version first.

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

", <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

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. ;)

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.

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

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.

@Gabor: thanks for the input, that makes sense.

I'll apply the last hunk to both branches when I have the chance.

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.