Posted by bangpound on April 18, 2009 at 2:45am
| Project: | GeoNames |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | bangpound |
| Status: | closed (fixed) |
Issue Summary
Throughout the Geonames module, there are lines of code like this:
'#value' => t('Note: You must give credit to ') . l('GeoNames', 'http://www.geonames.org/') . t(' if you are not using the commercial services, for example by including a link to ') . l('their site', 'http://www.geonames.org/') . t(' on your pages. The GeoNames geographical database is released under the') .' '. l('Creative Commons Attribution Licence 3.0', 'http://creativecommons.org/licenses/by/3.0/') .'.',Making strings translatable by passing them through the t() function is the right thing to do, but this is the wrong way to do it. Moreover the link texts are not translatable, so the English words 'their site' will appear in the middle of properly translated text.
The right way (or close to the right way -- I am happy to be corrected!) to do this is:
'#value' => t('Note: You must give credit to !geonames_link if you are not using the commercial services, for example by including a link to !theirsite_link on your pages. The GeoNames geographical database is released under the !cc_link.', array(
'!geonames_link' => l('GeoNames', 'http://www.geonames.org/'),
'!theirsite_link' => l(t('their site'), 'http://www.geonames.org/'),
'!cc_link' => l(t('Creative Commons Attribution Licence 3.0'), 'http://creativecommons.org/licenses/by/3.0/'),
)),I'll work on this patch for DRUPAL-6--1.
Comments
#1
Any patch, bangpound?
#2
Started fixing t() issues: http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/geonames/ge...
#3
Lyricnz:
I worked on this, but I think I stopped when I realized I wasn't working on the leading dev branch of geonames module.
I'll catch up with it soon after my vacation and help out.
#4
Yeah, there was a bit of a disaster with multiple CVS branches. We've decided to move forward with this one - and SeroSero can roll his other changes (which provide a service-based API to geonames) in when he likes.
#5
Great. I've seen now that the main chunk of difficult-to-fix t() abuse has been separated into its own inc file. I'll feel better about fixing the t() in the module file alone first.
#6
Hahah, yes. I'm not too inclined to spend time on fixing the geonames docs, but the rest of it should be cleaner now. I'm getting ready to roll another release, so getting this fixed would be great.
#7
Here's the patch I promised.
It fixes the main t() problems and also some coding style. Coder tests pass except those related to drupal_ string functions.
The docs are well done, but I wonder if it would be more sensible to bring this into advanced help... or if it's even possible without hardcoding it.
#8
Looks good. Applied, thanks!
(I was about to point out "hey, you don't t() menu entries" - then noticed that you're undoing it, not doing it :)
#9
Automatically closed -- issue fixed for 2 weeks with no activity.