Fix incorrect calls to t()
wmostrey - September 19, 2008 - 08:55
| Project: | Node comments |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
CivicActions is reviewing and upgrading multiple modules for use on client sites. Part of this work is a coding standards and i18n review. Attached you will find a patch based on a review with the coder module and a careful examination of the code.
The title and description of a hook_menu item don't need to be t()ed. More information can be found here: http://drupal.org/node/140311
Thanks!
| Attachment | Size |
|---|---|
| nodecomment.patch | 616 bytes |
#1
#2
Patch looks good and applies cleanly.
#3
Not quite, there's also an unnecessary call to t() in _node_comment_delete_thread() - watchdog descriptions aren't passed through t() either now.
#4
Here's the updated patch.
#5
This looks great now.
#6
I've taken rc1 and applied that to HEAD so it can be worked on from there. Haven't had a lot of time to spend on this, but I applied this patch to HEAD as well. Thanks.
#7
I'm not sure those changes are being included in the 6.x-1.x-dev tarball. The changes were committed to HEAD, but the tarball appears to be generated from DRUPAL-6--1 branch, where other changes have also been made since. I'm pretty sure these i18n changes aren't included.
I've re-rolled the patch, so it includes the above 2 changes and one additional internationalization issue.
Cheers,
Stella
#8
small error in the patch:
<?php- 'description' => t("Administer your site's comment settings."),
+ 'description' =>("Administer your site's comment settings.",
?>
should be:
<?php- 'description' => t("Administer your site's comment settings."),
+ 'description' => "Administer your site's comment settings.",
?>
committed to DRUPAL-6--1 branch.
http://drupal.org/cvs?commit=144468
#9
That's great, thanks! However, I just noticed one other small error - one instance introduced by the patch and one that already existed. The
t()function can't be used in install files as it may not always be available. Instead theget_t()function should be used to determine whethert()orst()should be used. The attached patch fixes this.Cheers,
Stella
#10
ah, interesting. good to know, thanks!
#11
Automatically closed -- issue fixed for two weeks with no activity.