Use the same language consistently in generated comments and strings

No one is interested in this one?

It is making diffs imposible when working with people from around the world :)

Definitely interested :) I have my doubts whether the translatable string language selection is really needed, it is a best practice to use English in t(). So, it probably should default to English wherever available, and only revert to the current language if no English original is available (possibly marking the calls to t() with a comment stating an English original could not be found). Maybe in case there is no English original the setting will be used as a default..? Then again, how is the system ever going to be able to find a language other than the current one if it must be different than English and there is no English original to use as the connection between translations of the same language..?

It should be a really good start to always use english language for exportations, whatever the default language is.
Being able to choose the language of comments and translatable strings is just a nice-to-have imho.

Here is a simple patch which does the following: Everytime a feature export is rendered, it sets the current language to english.
This works both in drush as well as in the UI (manually tested that).

Just to be clear, this problem is kinda a major pain.

Excellent!
Just to know, why did you wrap this code in a function?

+++ b/features.module
@@ -1200,3 +1200,10 @@ function features_feature_unlock($feature, $component = NULL) {
+  $GLOBALS['language'] = 'en';

I'm afraid we need to extend this a bit since changing the language mid-request could lead to some strange behaviour related to (static) caches.
E.g. the entity info cache: The persistent cache is cached per language but the static cache isn't (which seems natural) - so I suppose we need to do a drupal_static_reset() here, unfortunately for stuff that doesn't use drupal_static() there is no hope.

unfortunately for stuff that doesn't use drupal_static() there is no hope.

That is certainly a general statement.

Here is a new patch

Ups, that was the wrong patch.

Patch from #11 gives me lots of Notice: Trying to get property of non-object in entity_get_info() (line 7743 of /home/pascalmartineau/Web/localhost/testing.dev/includes/common.inc). errors.

Am I doing something wrong?

+++ b/features.module
@@ -1200,3 +1200,13 @@ function features_feature_unlock($feature, $component = NULL) {
+  $GLOBALS['language'] = 'en';

Erm, language should be a language object :)
We could do

  // Create the language object as language_default() does.
  $GLOBALS['language'] = (object) array(
    'language' => 'en',
    'name' => 'English',
    'native' => 'English',
    'direction' => 0,
    'enabled' => 1,
    'plurals' => 0,
    'formula' => '',
    'domain' => '',
    'prefix' => '',
    'weight' => 0,
    'javascript' => '',
  );

Updated patch. It also ensures that the language and static cache handling only is processed once.

patch #14 works
thanks

Resetting the whole static cache can lead to a broken UI as drupal_add_js() and other functions use the static cache.
I've updated the patch to keep important pieces of the drupal static cache.

Patch #16 works like a charm!

I'd recommend one more minor change to this patch:

Can you check to see if the default language is already set to English to bypass all of this in that case? All the extra cache clearing is still going to impact performance so I'd like to only do this if the site is non-English.

@mpotter Good idea to do the evil stuff just if really necessary ;)

Patch update. Slightly adjusted the comment on why parts of drupal static are restored as well.

Committed to 9746b22.

When using the latest features (version 7.x-2.6) with an interface language other than english, the features are marked as being overridden but export correctly in english. When checking the overrides using the diff module, I get "No changes have been made to this feature" yet the feature is still marked as being overridden.

Should we reopen this issue?

Update: It seems to only affect features that have fields in them.

I'm having the same issue as PascalAnimateur. Also, views field description in the export are in Dutch (my default language). So when re-creating the feature, it results in the following:

<?php
-  /* Field: Content: Title */
+  /* Veld: Content: Title */

-  /* No results behavior: Global: Text area */
+  /* Gedrag bij ontbreken van resultaten: Global: Text area */
?>

Any thoughts?

I'm experiencing the same behavior as #23 but also with view that have labels that are going to be translated.

I only experience this issue in this case:

• The site default language is not English
• For the Views module only
• Using drush (with the command drush fu).

When I recreate a feature with a View through the UI the code comments are correctly in English.

The reason that the code comments for a View get translated when using drush is because in _drush_features_export() caches are flushed before the exports are generated (it is in the export generation phase that the active language is set to English). A cache flush triggers (among other) a menu cache flush which loads View objects, which calls views_object_types(). views_object_types() statically caches (and translates) strings that are used in the export.

So, for drush the solution would be to set the language to English before flushing caches. See attached patch. I wonder though if the language should be set to the original after the export is done. This would only be relevant if code ran after this can be dependent on the active language. The patch that was committed didn't set the language back later on, so maybe this is far fetched.

I haven't looked into the issue of features getting marked as overriden, but I think that has a similar cause (the active language is set to English to late in the process).

I arrived to a similar patch.
I experience the issue with drush (workarounded forcing drush to always run in English) and the ajax call features make to get the storage of features.

The attached patch solves both.

Thanks! This patch in #27 fixes the problem with overridden features. Please commit.

Patch at #27 solved the issue for me too, thanks!

You also get my vote for committing #27 !

I use Seven as admin theme but get the default theme in diff's "Review overrides" page.

kevinn: Can you explain why the language we are discusing in this issue is interacting with the admin theme?
Maybe it needs a new issue.

+1 for #27

#27 works for me.

Could someone please commit patch #27. This also works for us and we want to stop patching the features module over and over again.

I'll try to get this in soon, but it would have been a lot better if this had been posted to a new issue instead of re-opening the previous issue and adding another patch. Just makes the patch maintenance and testing more complicated and slows down the process.

Definitely working for me, what I nice find. I was now working features in english in order not to have them as overridden all the times. All seems good now!

#27 was OK for me too!

#27 works for me also...

Hello,

Thanks for the patches.

#27 works for me, avoiding features to be marked as overridden for comments not in the good language.

Tested #27 against version 7.x-2.6 and works!

Looks like it is RTBC so please commit this.

For me #27 did not fix the whole issue.

It fixed the following:

• Features status: a feature that contains a View is marked as overridden in the UI (and also when using drush features-list (fl)) when the sites default language is not English. When visiting the "diff" page (admin/structure/features/[feature-name]/diff) nothing appears to be changed.

It did not fix this problem for me:

• drush features-update (fu): when recreating a feature that contains a View using drush, code comments in the View are exported to the sites default language instead of English. Example:
  /* Filtercriterium: Content: Published */
  instead of:
  /* Filter criterion: Content: Published */

I've combined the patches from #25 and #27 to fix both issues. For details about the fix for drush, see #25.

I did not test drush fu last time, tested #43 with both drush and trough the UI and it works!

Thanks for the work on this. Committed to 4c7f9fc.

This completely breaks search and several other core features, as per #2603578: _features_set_export_language is clearing drupal static cache too aggressively breaking search and possible many other modules :(

OK, because of #2603578: _features_set_export_language is clearing drupal static cache too aggressively breaking search and possible many other modules I am tempted to revert *all* of this language stuff. The way it is handling the static cache is just really really bad. I'd rather have comments in Feature exports in English like it always has been rather than deal with all the side-effects of this patch.

Welcome comments on this, but without a better solution that's where I'm heading.

I am all in with that. The caveat I found is that without this patch, when you are in a multilingual site and the admin UI is on another language, you get all of the features as overridden. Switching the UI to english sorted this out and I am somehow with this. I guess 99% of site devs that are working with features are Ok with an English UI.

What I would add is some kind of warning to the features admin page, in which if the UI is not the default english one, features could be wrongly stuck as overridden.

@mpotter, @hanoii,

I follow you too but the solution to switch to the English UI isn't always possible because in most of the websites my company wants, the English language is disabled.

Regards,
Michel.

Part of the problem is in Views, I think. The function views_object_types() statically caches translations without providing a method to reset that cache when the language is changed during the features export process. I think that the patch in #43 worked around this problem by changing the language earlier in the process. #43 also fixed having features be marked as overridden by changing the language to English in features_get_component_states(), so the language isn't changed only during export, but also when checking the state of the feature.

I'd rather have comments in Feature exports in English like it always has been

It hasn't been always in English (unless English is the site default language). At least after #21 in this issue there was a difference in behaviour when exporting features via the UI versus drush fu: exporting via the UI resulted into English comments, exporting via Drush into comments in the site default language. If I remember well, before the patch in #19, the comments were always in the site default current active language (site default language in case of Drush).

Part of the problem is in Views

Right, that's just the thing. *ANY* Module could potentially have trouble if they use the cache like this. So Features shouldn't be doing something like this trying to correct what other modules might be doing. There was no defined core way to handle caching translations, so each module does it differently. Some use drupal_static, but some might even still have just regular static variables that don't even get changed by this.

Sorry I mistated about the language. It's more what was said in #50 about overrides. But we all know that there are plenty of situations that result in overridden modules, and it's something we have lived with for years.

But to clarify: My issue isn't with the patch in #43. All it does it add calls to _features_set_export_language(). The *problem* is the code in _features_set_export_language() added back in #19. That whole way it is clearing the static cache with the array of exceptions is just bad. That's the part that needs reverting in my opinion. I should have reviewed that more carefully months ago before committing it.

OK, what I propose is going back to #13 where we change the language but do *NOT* clear the static cache. And then add #43 on top of that.

Would that help with overridden features without having all of these cache side-effects?

Moving the discussion and new patches for this over to issue #2603578: _features_set_export_language is clearing drupal static cache too aggressively breaking search and possible many other modules