Drush provides dt() as a wrapper to t() and if t is not available it fallbacks to a simple but enough call to strtr(). So the only advantage of this is to allow for a localized drush --only when it runs on a bootstrapped site.

I don't think localizing drush has any utility, moreover it can be an obstacle for scripting purposes. Also, some argument-symbol is rendered as an undesirable <em>message</em>.

Well, although t() is a bunch of code including several db accesses (if using locale.module) it seems that removing it is not much of a performance gain, but I still think it worth it.

CommentFileSizeAuthor
#20 drush-1159146.patch17.7 KBjonhattan
#19 drush-1159146.patch17.7 KBjonhattan
#17 drush-1159146.patch17.5 KBjonhattan
#12 drush-1159146.patch1.17 KBjonhattan

Comments

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented

As a side node, some of drush commands do 'description' => dt(...)in hook_command() while others don't. The standard is to not do it.

greg.1.anderson’s picture

greg.1.anderson commented

If the standard is to not use dt() in some places, are there any other places where it should be used? It does not seem to make sense to me to use dt() for output if it is not used for command descriptions. If we are not to use dt() in command descriptions, should we perhaps just eliminate the dt() function completely, and make all drush strings 'raw'? (That is, is the dt substitution capability better than simple concatenation if we are never going to localize?)

moshe weitzman’s picture

moshe weitzman commented

What do commands like `git` and `mysql` do for localization of help texts, error messages, etc?

greg.1.anderson’s picture

greg.1.anderson commented

gettext, mostly. Although I cannot say for certain what any particular project is using, this is the most common answer for Linux tools written in C. There are gettext equivalents for many other languages, including php.

Not sure that drush should change, but there is your answer.

moshe weitzman’s picture

moshe weitzman commented

OK, the take away is that they offer localization. Then drush should do so also. Am open to more discussion, but thats my first take. I'm not too clear on what the 'scripting purposes' problem is. I guess you are saying that a calling script can no longer depend on any given string as output because of localization. Surely there are solutions for that.

greg.1.anderson’s picture

greg.1.anderson commented

So we are clear then that the standard is to use dt in command descriptions? At least in an ideal world they should all be there, right?

moshe weitzman’s picture

moshe weitzman commented

Yes, I agree that command descriptions should have dt().

FYI, there are complete translations of Drush in Spanish and Danish. Partial translations in many other languages. See http://localize.drupal.org/translate/projects/drush

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented

spanish translation! it was not me! to use this translation it is needed to import it manually per site... l10n_update is not of help here.

For the scripting purposes I mean to parse the output. With 'svn', 'mysql', etc you can prepend LANG=C to the command and obtain untranslated output. Perhaps we could pass a flag to drush to do so and force lang=en in drupal.

Also, we only benefit from t() translations when drupal is bootstrapped. Switching to gettext will fix this but we loose the translations already present in modules integrating with drush. We can complex the logic and use both gettext and t() when available. Also, the option for gettext will be to ship a bunch of .po/.mo files with drush? I see this unpractical.

On hook_command(): it is analogous to hook_menu() and t() is not used there for items definition. Strings are passed through t() in the calling function. In drush some commandfiles do use dt() others no.

In case I was not clear my proposal, in summary, is to not support translations and reduce dt() to strtr() --only useful for strings with placeholders.

DjebbZ’s picture

DjebbZ commented

I'm in favor of keeping the translation possibility (the dt() function), and if scripting is a problem an option like lang=en may solve the problem.

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented

Re-reading #8 I realize I was confusing in my writting. Sorry for that, I was in a hurry and did not review.

1. some drush commands are using dt() for its description while others no. If drush is supporting localization this needs to be fixed. I propose to let drush_get_commands() do the translations instead of filling the commands definitions with dt(). That is similar to what drupal does for hook_menu() in http://api.drupal.org/api/drupal/includes--menu.inc/function/_menu_item_... .

Attached is a patch for that. To complete this patch all calls to dt() in command definitions must be removed.

2. OTOH for drush to be fully translatable the solution can't rely on t() as it only works when there's a bootstrapped site and also it needs the drush translation to be imported per site (and l10n-update won't help as it is based on installed modules --drush isn't one).

A remedy to this is to use gettext as mentioned. We can use a script to rip translations off localize.drupal.org periodically and push them to drush repo. A counterpart of going this way and removing t() is that translation of modules integrating with drush will not be available, perhaps a minor thing.

Also consider drush can obtain the cli user's language from a env variable (as 'mysql', 'git', etc does) but it may be different from a site's language.

So I think that in practice users will use drush in english and doesn't worth it to implement gettext support or mix gettext with t() to completely cover all translated strings.

DjebbZ’s picture

DjebbZ commented

Ok I understand your point now. I thought you wanted to drop all kind "translatability". I'm not a core Drush developer, but including t() calls as you say in 1 seems a good idea. But where is your patch ?
As of 2, I have no idea what's the best. Maybe a drush command like drush self-translate --lang=[language code from localize.d.o] could do the heavy work for us ?

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
StatusFileSize
new drush-1159146.patch1.17 KB

the patch.

moshe weitzman’s picture

moshe weitzman commented
Status: Active » Needs work

I think this patch will work fine. Lets send along an array to dt() if an array is provided (i.e. description with placeholders).

anarcat’s picture

anarcat commented
Issue tags: +i18n

I would object to dropping t() from dt(). A bunch of stuff is already translated in the frontend and while it depends on the bootstrapped site, it's better than nothing - reinventing the wheel there and re-translating everything just seems like a real bad idea...

I agree with the side not of making the commands descriptions translation more standard however, but - that is a side note here. :)

As for worries about scripting, if you script relies on translatable content or plain strings, you are doing something wrong. You should:

* use --pipe, or;
* use --backend

Furthermore, you can always resent the translation through the environment, or at least that how gettext() works and it's mostly standard now:

env LC_ALL=C LANG=C ls
greg.1.anderson’s picture

greg.1.anderson commented

Would it be possible to add some code to dt() that, instead of calling through to t() would instead use a option (e.g. $options['translation-files'] in drushrc.php) to point at some drupal root where translation files could be found? Would that add too much code bloat to drush?

anarcat’s picture

anarcat commented

You would probably want to use gettext() directly at that point, in my opinion. The trick would be to find the files, but once it's done, since Drupal was smart enough to use that standard format, we could just use the builtin gettext() extensions instead of doing our crazy parsing:

http://php.net/manual/fr/function.gettext.php

This way drush itself could provide .po files the Drupal community knows and loves, and they would be useable even if drush wouldn't bootstrap a database.

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
Title: Drop usage of t() in dt() » Clarify usage of dt() in hook_command()
Status: Needs work » Needs review
StatusFileSize
new drush-1159146.patch17.5 KB

Same patch of #12 with the suggestion from #13, as:
'description' => array('a !text with placeholders', array('!text' => 'string'));

and also remove all occurences of dt() in the several hook_command() implementation.

anarcat’s picture

anarcat commented
Status: Needs review » Needs work

API docs on the two new functions would be nice...

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
Version: » All-versions-4.x-dev
Assigned: Unassigned » msonnabaum
Status: Needs work » Patch (to be ported)
StatusFileSize
new drush-1159146.patch17.7 KB

Commited with docstrings. Attached the patch for backport to 4.x.

I've opened a new issue for the gettext(): #1162694: Switch from t() to gettext().

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
StatusFileSize
new drush-1159146.patch17.7 KB

There was a bug 'in a 'string''.

msonnabaum’s picture

msonnabaum commented
Status: Patch (to be ported) » Fixed

Backported.

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented

for the record, about scripting:

$ drush up --pipe
admin_menu 6.x-1.5 6.x-1.6 Actualización-disponible
adminrole 6.x-1.2 6.x-1.3 Actualización-disponible
drupal 6.16 6.22 SECURITY-UPDATE-available

this output is half localized.

Automatically closed -- issue fixed for 2 weeks with no activity.