I've seen so many issues in the German translation that made me reviewing the strings and translations and to get some issues fixed I've created this patch.

Files: 
CommentFileSizeAuthor
#6 captcha_Translatable+string+review+context+sensitive+bugfixes3.patch16.2 KBhass
PASSED: [[SimpleTest]]: [MySQL] 731 pass(es).
[ View ]
#4 captcha_Translatable+string+review+context+sensitive+bugfixes2.patch15.52 KBhass
FAILED: [[SimpleTest]]: [MySQL] 728 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#3 captcha_Translatable+string+review%2C+context+sensitive+bugfixes2.patch15.52 KBhass
FAILED: [[SimpleTest]]: [MySQL] Fetch test patch: failed to retrieve [captcha_Translatable+string+review%2C+context+sensitive+bugfixes2.patch] from [drupal.org].
[ View ]
captcha_Translatable+string+review%2C+context+sensitive+bugfixes.patch15.52 KBhass
FAILED: [[SimpleTest]]: [MySQL] Fetch test patch: failed to retrieve [captcha_Translatable+string+review%2C+context+sensitive+bugfixes.patch] from [drupal.org].
[ View ]

Comments

Status:Active» Needs review

Status:Needs review» Needs work

Status:Needs work» Needs review
StatusFileSize
new15.52 KB
FAILED: [[SimpleTest]]: [MySQL] Fetch test patch: failed to retrieve [captcha_Translatable+string+review%2C+context+sensitive+bugfixes2.patch] from [drupal.org].
[ View ]

Changed one string to upcase first.

StatusFileSize
new15.52 KB
FAILED: [[SimpleTest]]: [MySQL] 728 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Buggy robot don't like percents in file names.

Status:Needs review» Needs work

Status:Needs work» Needs review
StatusFileSize
new16.2 KB
PASSED: [[SimpleTest]]: [MySQL] 731 pass(es).
[ View ]

Challenging tests.

Status:Needs review» Needs work

Hi, thanks a lot for these suggestions.

I've already committed some parts in http://drupalcode.org/project/captcha.git/commit/bdfc108

I have some questions/doubts about others:

-    $captcha_types['none'] = '[' . t('none') . ']';
-    $captcha_types['default'] = '[' . t('default challenge type') . ']';
+    $captcha_types['none'] = t('- None -');
+    $captcha_types['default'] = t('Default challenge type');

I agree with the capitalization change. Did you change "none" to "- None -", because 'none' is too short to be unambiguously translated (e.g. in German)?. Why did you remove the square brackets? I've used these to indicate that it are special cases. Compromise proposal: '[No challenge]' and '[Default challenge type]'.

-        $captcha_types["$module/$type"] = t('@type (from module @module)', array('@type' => $type, '@module' => $module));
+        $captcha_types["$module/$type"] = t('@type (from module @module)', array('@type' => t($type), '@module' => $module));

I've always learned that the Drupal translation system is not made to do t($variable) and you should always do t('a hardcoded string'). Why do you want to translate it anyway? The challenge types are shown untranslated in a lot of other places.

In a lot of places you suggest things like:

-    t(' ... the help of the \'%CAPTCHA_admin_links\' option below.',
-      array('%CAPTCHA_admin_links' => t('Add CAPTCHA administration links to forms'))
-      ),
+    t(" ... the help of the <em>Add CAPTCHA administration links to forms</em> below."),

I don't think I agree here: if you have incomplete translations (e.g 'Add CAPTCHA administration links to forms' is translated, but the long string not), with the original construction you have consistent translations: 'Add CAPTCHA administration links to forms' is always translated, whether it is embedded in a translated string or not. With your construction, the user will see the string translated and untranslated in different places, which makes the UI harder to understand IMHO.

Flush versus clear cache: is there a guideline for this? I found usage of both in Drupal core.

-            '#title' => t('Challenge "%challenge" by module "%module"', array('%challenge' => $challenge, '%module' => $module)),
+            '#title' => t('Challenge %challenge by module %module', array('%challenge' => $challenge, '%module' => $module)),

I would keep the quotes around %challenge because this strings can come from other modules and can be a complete sentence, so dropping the quotes can be bad for readability. I have less firm feelings about the quotes around %module :)

-    drupal_set_message(t('Failed to set a CAPTCHA type for form %form_id: could not interpret value "@captcha_type"',
+    drupal_set_message(t('Failed to set a CAPTCHA type for form %form_id: could not interpret value %captcha_type.',
...
-          '%form_id post blocked by CAPTCHA module: challenge "%challenge" (by module "%module"), user answered "%response", but the solution was "%solution".',
+          '%form_id post blocked by CAPTCHA module: challenge %challenge (by module %module), user answered %response, but the solution was %solution.',

I prefer quotes in this kind of error handling cases, because if you have an empty string for some reason, you can clearly see this, while without quotes, it looks like your sentence is chopped off.

+  // "Level:" is a translatable string "context" for the options.
   $form['image_captcha_color_settings']['image_captcha_foreground_color_randomness'] = array(
     '#type' => 'select',
     '#title' => t('Additional variation of text color'),
     '#options' => array(
-      0 => t('none'),
-      50 => t('small'),
-      100 => t('moderate'),
-      150 => t('high'),
-      200 => t('very high'),
+      0 => t('Level: none'),
+      50 => t('Level: small'),
+      100 => t('Level: moderate'),
+      150 => t('Level: high'),
+      200 => t('Level: very high'),

I don't like the 'Level:' prefix here. Why not use real translation context as introduced in Drupal 7?. Or alternatively: 'No variation', 'Little variation', 'Moderate variation', ...

I agree with the capitalization change. Did you change "none" to "- None -", because 'none' is too short to be unambiguously translated (e.g. in German)?. Why did you remove the square brackets? I've used these to indicate that it are special cases. Compromise proposal: '[No challenge]' and '[Default challenge type]'.

I used the None string as we are using them everywhere in Drupal for selectboxes with a none option. In past many used "none" and <none> and this is extremely difficult to translate, The minus are just the way how it's done in all translations since D6. I would suggest to used dashes not brackets. - No challenge - sound also much better to me than "None". I don't like the brackets... it's uncommon.

I've always learned that the Drupal translation system is not made to do t($variable) and you should always do t('a hardcoded string'). Why do you want to translate it anyway? The challenge types are shown untranslated in a lot of other places.

Than I missed the others... In general you are absolutely correct, but there are a few exceptions. I made this because you have the strings already somewhere else wrapped in t(). As these strings are safe you can do the wrapping of a variable. You should not surround custom (unstrusted) text with t(). The custom captcha label is also one exception, as only admins have permission to change and you need to trust admins. :-)

I don't think I agree here: if you have incomplete translations (e.g 'Add CAPTCHA administration links to forms' is translated, but the long string not), with the original construction you have consistent translations: 'Add CAPTCHA administration links to forms' is always translated, whether it is embedded in a translated string or not. With your construction, the user will see the string translated and untranslated in different places, which makes the UI harder to understand IMHO.

I should better write documentation on d.o... you should never have any strings that use l() to create links. Translators cannot translate such strings in context. Very often the placeholder text is conditional to the surrounding text. Additional if there is only a placeholder - translators can only *guess* what is inside the placeholder. We don't do this since D6/7 any longer. You should always write the text in the translatable strings and keep only the links outside as they could change in D8, but the text may not. I understand that this is additional work for translators, but it's better than a placeholder as I have no idea if I need a verb before the placeholder starts of not (context sensitive issues). In localization server I have no clue about the context or lines of code.

Aside, something that is already a % placeholder don't need double quotes again. The % placeholder is perfect.

Flush versus clear cache: is there a guideline for this? I found usage of both in Drupal core.

The main idea was to bring it in one line with core core... I believe there is no written rule :-)

I would keep the quotes around %challenge because this strings can come from other modules and can be a complete sentence, so dropping the quotes can be bad for readability. I have less firm feelings about the quotes around %module :)

Hm... I just made it like everywhere; if we expose something we make it EM... if this are sentences, than ok, maybe better to say with double quotes, but than please don't use % placeholders... it's somewhat "double" exposed.

I prefer quotes in this kind of error handling cases, because if you have an empty string for some reason, you can clearly see this, while without quotes, it looks like your sentence is chopped off.

Your personal preference is not like the most common used. I would go with core and we use % placeholders and that's it. Doubles quotes in other languages are sometimes like nightmare... see http://en.wikipedia.org/wiki/Quotation_mark_glyphs... and now try to type them with a keyboard in a website textarea. In German we mostly use copy and paste as they are so difficult to type... :-)

I don't like the 'Level:' prefix here. Why not use real translation context as introduced in Drupal 7?. Or alternatively: 'No variation', 'Little variation', 'Moderate variation', ...

Funny... I know... contexts are cool, we can use them here, but how should be name the context? I tried to find a good name, but was not sure. It should be as generic as possible. Contexts should be used very exceptional. The list of contexts is still in heavy discussion as it's very difficult to reuse. The words are not really "sizes"... I don't know... please don't use "Variations" as context... this is what we could name - abuse of contexts. A context name should describe the content as good as possible and as generic as possible. I hope I explained this correctly...

Status:Needs work» Closed (fixed)

Hi, sorry for the delay

I think I handled most of the issues now (in separate commits). Please reopen for remaining/additional issues.

Thanks for you work

Have you comitted the remaining bugs, too?

Ah, I see... This are all good cleanup changes. Great! THX