I've translated this version to Danish but several strings are not translatable. I'm no expert but I suppose it's because they are missing their t(). How do you want me to move forward with this? Should I look through the code and report all these strings, or are you going to do that, or is there another way?
Marcus
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | luceneapi.zip | 14.6 KB | marcushenningsen |
| #6 | luceneapi.module.patch | 435 bytes | marcushenningsen |
Comments
Comment #1
cpliakas commentedHi Marcus.
I am marking this as a bug report, because I think that it is important to fix. Let me know what strings you find, and I will correct them. Anyone else is welcome to submit patches as well. I have tried to wrap everything in t(), but it is well within the realm of possibility that I missed some.
Thanks very much for bringing this to my attention, and thanks in advance for any help you can give.
~Chris
Comment #2
marcushenningsen commentedOk, the problem is that you're putting a lot of strings in variables which you later wrap with t(). This is not allowed, Drupal even throws errors for every one of these strings when visiting the relevant pages in a non-english language, e.g. da/admin/settings/luceneapi_node/facets (for Danish langauge). Can you reproduce this?
Instances of these strings can be found but is not limited to these lines of code:
luceneapi_node.module, line 250
luceneapi_node.module, line 270
luceneapi_node.module, line 283
luceneapi.admin.inc, line 241
luceneapi.admin.inc, line 323
luceneapi.admin.inc, line 339
luceneapi.admin.inc, line 351
luceneapi.admin.inc, line 394
luceneapi.admin.inc, line 408
luceneapi_node.admin.inc, line 20
luceneapi_node.admin.inc, line 97
luceneapi_node.admin.inc, line 104
luceneapi_node.admin.inc, line 125
luceneapi_node.admin.inc, line 132
I'm willing to supply you with patches for every file, if you tell me how to go about the problem.
Marcus
Comment #3
cpliakas commentedHi Marcus.
Thanks for being willing to help. Let me clean up my code and remove the strings from variables, and then we can start going over the errors. I have installed a site in German (a language which I semi-understand), so I am going to try and reproduce the errors you are getting. Beta 3 will be released soon (most likely sometime this week), so let's shoot to get this resolved in Beta 4.
Again, thanks for your help in discovering the translation bugs,
Chris
Comment #4
marcushenningsen commentedNo problem. Sounds good, I'll await the next release.
Marcus
Comment #5
cpliakas commentedBeta 3 has been released. Please feel free to post any issues you have against that version.
Thanks!
Comment #6
marcushenningsen commentedI finally got to review this, and it's almost perfect now.
I only found two instances of missing t()'s. One is corrected in the attached patch (which btw is my first patch submitted ever!!!).
The other I'm not sure how to correct: In the tabs at admin/settings/luceneapi_node 'Performance' and 'Index Statistics' are not translated.
Otherwise you nailed them all. Thanks.
Comment #7
cpliakas commentedCongrats on your first patch!!! I am glad it was on my module :-). Let me just make sure that t() isn't called somewhere down the road so that the string isn't double-translated and I will commit the patch.
In terms of the tabs, according to the documentation at http://api.drupal.org/api/function/hook_menu the title should not be passed to t(). Menu items are cached, so maybe a cache clear is required to get the translations you need. Just in case you are curious, the code that builds the 'Performance', 'Index Statistics', and 'General settings' menu local tasks (tabs) is located on line 215 in the luceneapi.module file If the translations don't work after the cache clear please let me know and we can debug further.
Thanks for your great attention to detail on this issue. Also, any translations you wish to share would be appreciated so that Search Lucene API can speak Danish out of the box.
~Chris
Comment #8
marcushenningsen commentedThanks. I attached the translation files.
Clearing the caches was not enough. The problem is that the translation strings for the menu tabs don't show up in any of your module's translation templates. I suppose they're showing up somewhere else, but I'm not sure where. According to the info at the link you sent me, I understand that they're automatically passed to t(). That's probably why e.g. 'Generel settings' is already translated to Danish but 'Performance' isn't, they're inherited from some other template.
Comment #9
cpliakas commentedThanks for the translation! You rock. Also, thanks for the explanation about the tabs. This is definitely something I will look into over weekend.
Thanks again,
Chris
Comment #10
cpliakas commentedPatch committed in #282470.
Comment #11
cpliakas commentedMarking this issue as fixed, although a task has been set up at #620340: Review best practices for translations, adhere to best practices regarding calls to t(). to adhere to best practices regarding translations.
Comment #12
marcushenningsen commentedSounds good. On a side note, the problem with the untranslatable strings also applies the DoYouMean module, which is the only of the submodules I've checked. I've also made preliminary Danish translations for that module, but could you please check the module's code for the same bug that you corrected in Lucene Search beta3?
Marcus
Comment #13
cpliakas commentedI will do so. You have sparked a lot of interest regarding localization, and I have been doing a ton of research regarding best practices. As you can probably see form the issue queue, translations are a huge focus of the beta4 release. I plan on applying these changes to the sub modules within the next couple of weeks.
Thanks for all your work,
Chris