t() should work regardless of the bootstrap phase

I encountered the same messages, and found out that the error-reporting code from Blank page - The White Screen of Death I added to index.php was causing them. Removing ini_set('display_errors', TRUE); resolved the problem.

At least it's possible to use Drupal with enabled simpletests... but you're right, of course. Line 879 is inside the t() function... all I found out so far is that the global $language variable is sometimes NULL.

Here is the scenario that fails:
- variable_init() fails to load the variable cache, rebuilds it and call cache_set()
- cache_set() generate a MergeQuery
- the MergeQuery requires the schema in Postgres, so calls drupal_get_schema
- drupal_get_schema() fails to load the schema from cache, so rebuilds it
- during that process, it module_invoke the hook_schema() implementations, that contains t()-ed descriptions
- t() fails because the language sub-system has not be initialized at that point

#304924: Extend drupal_error_handler to manage exceptions changed the way we handle error management: we used to enforce E_ALL only when the error handler is loaded (in BOOTSTRAP_FULL), while we now enforce E_ALL much earlier in the bootstrap process, in initialize_variables(), in BOOTSTRAP_CONFIGURATION.

So the process that failed silently before now output errors all over the place and prevent the HTTP redirection to take place (note that Simpletest *is* enabled correctly anyway).

So the process that failed silently before now output errors all over the place and prevent the HTTP redirection to take place (note that Simpletest *is* enabled correctly anyway).

The notices are, however, only displayed if you use ini_set('display_errors', TRUE); So it still fails silently.

In several situations, t() is called on a not fully bootstrapped Drupal. Those situations include the building of the database schema required for Merge queries in PostgreSQL.

Those t()-calls currently fail because the language sub-system has not been initialized yet, with odd-looking errors like:

Notice: Trying to get property of non-object in /var/www/projects/drupal/drupal-7.x-dev/includes/common.inc on line 879

t() is a versatile function. We should make it work regardless of the bootstrap phase. This also paves the way to reuniting t() and st() again.

chx was noting that t() should be a fallback while the language subsystem is not bootstrapped.

Once we fix up t() to really work -- ie. to do the replacements -- regardless of bootstrap phase -- it almost completely took over st's role. The rest can be fitted nicely in t(). So st is dead.

Per Goba's request, I added an $install_time parameter to install time strings. And fixed a variable in t itself.

- t($string, $args = array(), $langcode = NULL, $install_time) does not look like a valid signature

- Also, I see by looking at this patch that TRUE will not be really meaningful in identifying what the hell is happening there, so I'd do a define('T_INSTALL', TRUE), and use that instead. That would mean an str_replace('NULL, TRUE', 'NULL, 'T_INSTALL', $patch_code) on the patch.

Just for clarification, we do need to have a marker on strings also used in the installer or only used in the installer, since we read in translations for the installer from one .po file on each page request (we do not have a database set up yet). We should have a relatively small amount of strings in that file, to avoid memory problems and ensure a timely parsing process. So we just cannot go forgetting about marking strings only used in translations unless an SQLlite database is always available to fall back and we ship translations as SQLite databases that is :) Which would be kind of out of the scope for this issue. Heh :)

If we have T_INSTALL, doesn't that render the 'eliminate st()' goal mostly moot? Or will the set of T_INSTALL strings be smaller than the st() strings? The localization of the installer seems to be a royal pain again. ;-)

@Dries: the main idea behind "remove st()" is "remove the hurdle of choosing between st() or t(), having to use $t = get_t(), and such". We still need to "mark" the string used in the installer, at least until the SQLite implementation matured enough to serve as a bootstrap install environment.

Dries: one gain is that people can actually use other API functions, like format_plural() (although that requires an additional T_INSTALL parameter) as well as format_interval, format_date, etc in the installer. These all reuse t() and therefore useless in the installer. Because t() falls back on the installer strings if called in the install phase, it would "automagically" work for every t() call in the install phase, even when t() is reused from some other function.

So this patch makes t() work in the installer as it is without any additional parameter. We only need the T_INSTALL parameter because the installer reads in translations from a text file (as long as there is no database set up yet), and we need to somehow keep the size of that text file small. So we need to identify the actual t() and format_plural() calls where the string should be exported to that textfile. We could keep st() for that marker role, but that rules out format_plural() from being used in the installer, while this patch otherwise would enable it. Then we either need to provide an alias for format_plural() or add a parameter there. From there, it gets to the point that we better standardize on the parameter for API consistency.

I think adding this new parameter is still less of a mess then limiting the installer localization API to one function, instead of all the options available.

Ps. Of course if we could bootstrap the installer with a full SQLite backend AND ship translations as an SQLite DB later imported to the live database, then we would not need to be bothered with shipping all strings in the SQLite DB and looking the required ones up in the installer. As long as we use a textfile (PO file) for the installer strings, we need to keep reasonable limits.

Also note that we do not even necessarily need to add a parameter to the definition t() and format* -- you can use more parameters than specified by a function to call it. Also, instead of changing the function signature, we could add a ; // used by install too comment to the relevant lines.

chx: to my knowledge Dries ruled out magic comments like this in previous issues with localization, but we will see what he has to say now.

Marking as CNR -- we will reroll once we know which what to do but right now this is the best issue status as I do not have "architecture needs opinion".

Would it help avoid the T_INSTALL param if we used a mode constant such as define('MAINTENANCE_MODE', 'install')

Moshe: t() itself already knows that it runs in the installer or not based on a defined('MAINTENANCE_MODE') check (see patch). We need a "flag" on t() and format_plural() calls for a **static code parser** to identify the translatable strings. Otherwise you'd need to run all possible code paths of the installer (including all error conditions) and save all parameters to t() calls to a file to be able to identify all possible strings which might need translation in the installer. That sounds pretty hard, right?

Thanks for the clarifications.

Before I commit this patch, I'd like to see us do a couple of benchmarks with and without this patch. t() is called a _LOT_ so I want to understand the performance implications of this change.

This would still be great to do, it is a DX WTF/inconsistency that would be solved here. I agree benchmarking is crucial.

Unless I am mistaken, we already did what the title of this issue says. I think the title might be misleading tho.

Well, the language check is now proper, so t() will work with an uninitialized language. However, it still does variable_get() regardless, and the above patches suggested to avoid that for the possibly low bootstrap level.

Adding UI language translation tag.

Needs reroll

#1813762: Introduce unified interfaces, use dependency injection for interface translation actually implemented this.