Closed (fixed)
Project:
Project issue tracking
Version:
5.x-2.x-dev
Component:
Issues
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
7 Jul 2007 at 12:19 UTC
Updated:
30 Jul 2007 at 07:08 UTC
Jump to comment: Most recent file
Comments
Comment #1
hass commentedComment #2
hass commentedsmall mistake in the above patch. review this one, please.
Comment #3
hass commentedfound another
\r\nissue like in project module... rerolled patchComment #4
hass commentedThis patch fixes some translation bugs in the "issue.inc" file.
Comment #5
dwwHelp text with complex HTML shouldn't be 1 giant t() like this. Instead, please fix it like my latest patches at http://drupal.org/node/57667 and http://drupal.org/node/159334.
Also, if you're *just* fixing translation problems, please include all the changes in a single patch. That makes it easier to review, instead of separate patches for each file in a given module's directory. Putting other changes unrelated to translation problems in a patch/issue about translation problems makes it harder to review. Does that make sense?
For example, this isn't a translation change, you're proposing a change to the default UI (e.g. on drupal.org):
That belongs in a separate issue and patch. You're not "fixing" the translatability of the module, you're proposing to change the UI for English sites. Yes, "Priority" is easier to translate than just "Pri", but it might also make the UI look worse and require some additional .css changes or other things to get the spacing right.
Also, please verify that the fields in the mailhandler input could be translated. Otherwise, those keywords inside the help text should be handled outside of t() and passed in via placeholders or something, since people shouldn't translate them. See what I mean? Actually, since I was curious, I just looked myself and they can be translated. Though that raises weird questions about how mailhandler works on a multilingual site -- users send in email with the keywords in their own language, but how does hook_mailhandler() know what language to use for looking up the keywords? It's running via cron with the site's default language, it seems. Evil -- however, this belongs in a separate issue.
Comment #6
hass commentedUpdated patch
Comment #7
hass commentedAside, let me say, Gabor haven't said we must split *all* strings. He only said it might be better for translators, but he missed to say that some stings are context sensitive and it is MORE difficult to translate them if i cannot read the context of the string.
Therefor i thing what we have done here is no good idea in general. Partly it makes sense here, but partly not.
Comment #8
dwwGreat, almost there. Now, for readability and ease of maintaining this in the future, and since it's not inside a t() anymore where it was causing problems, let's keep some of the newlines in between the various parts. This way, if we ever need to change anything in the future, we only have to modify the appropriate line, not the entire message (from the CVS or patch/diff perspective -- the separate t() already defends translators from having to retranslate the whole thing if we change a few words). For example:
Note, also, your
</p>is in the wrong place in the patch, should be as I list here. (Though, I see that's just a bug in the original code, not your fault, but you shouldn't propagate bugs like this).Comment #9
hass commentedhopefully this patch gets your love. Additional i found and added the untranslatable "priorities".
Comment #10
dwwAdditional i found and added the untranslatable "priorities".
That's going to require much more careful review and testing, since it's entirely possible those strings are being stored in the db, where they need to be English to keep consistent behavior. That should go to a new issue, since it requires more thought and care. The other stuff is basically RTBC now.
Comment #11
dwwCommitted a modified version of the 2 changes not include the "priorities" thing to HEAD, and backported the help text change to DRUPAL-4-7--2 and DRUPAL-4-7.
Updated the translation template to find you had committed a new copy based on your hacked workspace, not the official code. Please, never do this, or I'll make sure you don't have permission to commit to the po directories in any of my modules. It's nice if you help keep the .pot files up to date, but you should *never* commit changes based on your local modifications.
Thankfully, soon this won't involve CVS at all, and the translation templates will always be extracted automatically.
-Derek
Comment #12
(not verified) commented