t() missing for some labels : this make some labels not being translatable

Comments

brdwor’s picture

please submit a .patch for these and I will add them to cvs

Freso’s picture

Title: t() missing for some labels » Not all strings are translatable
Assigned: Unassigned » Freso
Status: Active » Needs review
StatusFileSize
new8.58 KB

I've fiddled about a bit with this, and here's the result!

Some comments and questions though:
Should the HTML output at the end of the module be translatable (like RecipeML)?
There's a possible problem at line 1457: I'm not sure how well recipe_ingredient_quantity_from_decimal() plays along with format_plural(). If someone will test this and let me know the result (I've pretty much only got Drupal 6 sites around, ATM), I'll make a work-around.
Also, I might not have caught all strings. Let me know if you see something I missed. I haven't tested the patch either, so all sorts of breakage could, theoretically, occur, though I find this quite unlikely as it's a simple update of some text strings.

Freso’s picture

Oh, and this was done against HEAD. If there's a difference between DRUPAL-5 and HEAD, I might have to re-roll. I won't bother though, if there isn't. :)

Freso’s picture

StatusFileSize
new8.37 KB

Re-rolled patch against latest HEAD, as a bug was fixed that removed a string to be made translatable.
Comments previously mentioned still apply, except that the possible problem on line 1457 is now on line 1454.

brdwor’s picture

Sorry for no response, my computer died. I will commit this patch asap. Thank you

Freso’s picture

Before committing, please check my questions regarding the possible problem on line 1454 (hasn't been tested) and the translations in the HTML. It should be safe, though, to use my patch excluding the part on line 1454 (ie., keeping $preptime = t("!n hours", array ("!n" => recipe_ingredient_quantity_from_decimal($node->preptime / 60)));), and I would then be able to make another patch with a possible solution to that problem as well as having the text in the HTML translated if so desired.

But it's great to know that the patch wasn't just being ignored! :D

brdwor’s picture

I'll test it before committing

Freso’s picture

StatusFileSize
new9.09 KB

I've gone through the file again, and this is a bit more clean of a patch (ie., some non-related double quote to single quote conversions are sorted out). I've also made the most elegant solution to the "recipe_ingredient_quantity_from_decimal() vs. format_plural()" problem I could think of:

if ($node->preptime < 60) {
  $preptime = format_plural($node->preptime, '1 minute', '@count minutes');
}
elseif ($node->preptime % 60 == 0) {
  $preptime = format_plural($node->preptime / 60, '1 hour', '@count hours');
}
else {
  $preptime = t('!time hours', array('!time' => recipe_ingredient_quantity_from_decimal($node->preptime / 60)));
}

It basically says "if there are less than 60 minutes, describe it in minutes; else, if $node->preptime is cleanly dividable by 60, describe it in hours; else use regular string". I couldn't and can't think of any way to cleanly apply recipe_ingredient_quantity_from_decimal() within a format_plural(), so I found this to be the best way to deal with it. Please let me know if you have other suggestions on how to achieve this. (I have also tested that this algorithm works.)

t('!time hours', array('!time' => recipe_ingredient_quantity_from_decimal($node->preptime / 60))); could possibly be replaced with something like t(recipe_ingredient_quantity_from_decimal($node->preptime / 60).' hours'); to take advantage of the translations made earlier in the file (line 165 in the patched version).

brdwor:
Please let me know if there are any problems with the patch ASAP, so that I may soon fix them. I have a few other clean-ups I'd like to do to this code after having combed through it for strings, but I don't want to do it before this patch has gone in, as I don't want conflicting patches hanging about.

brdwor’s picture

Status: Needs review » Fixed

I tested the patch and found no problems. Patch in Comment 8 was committed to HEAD and DRUPAL-5.

Anonymous’s picture

Status: Fixed » Closed (fixed)