locale_inc_callback() is a thing of the past

again 1 :

+          if (drupal_function_exists('_locale_parse_js_file')) {
+            locale_inc_callback('_locale_parse_js_file', $filepath);
+            watchdog('locale', 'Parsed JavaScript file %file.', array('%file' => $filepath));
+            $parsed[] = $filepath;
+            $new_files = TRUE;
+          }

A closely related issue is #187398: Re-split locale module which should be followed up, fixed and committed to Drupal 7 as well. Locale.inc is basically all locale.module subfunctionality, and is not at all shared in the whole system. Locale functions like t() are elsewhere, not in locale.inc. So locale.inc should move under locale.module and named like other Drupal sub-module includes are now named.

See: #335122: Test clean HEAD after every commit and http://pastebin.ca/1258476

Looks like we could remove this nested if:

         if (substr($filepath, 0, strlen($dir)) != $dir) {
+          if (drupal_function_exists('_locale_parse_js_file')) {

This patch includes other places where inclusion of locale.inc was explicit and not implicit through locale_inc_callback. These should now also make use of drupal_function_exists(). This gets us much closer to execute a simple patch at #187398: Re-split locale module (which requires these changes or would need to modify incldue paths from includes/locale.inc to modules/locale/locale.admin.inc or so).

Please review this patch since it holds up #187398: Re-split locale module which holds up further nice developments (at least I have some cool ideas :) on locale functionality.

I ran through the install in English and it worked fine but after grabbing the Traditional Chinese D6 language set (there aren't any D7 ones yet) and then re-running the installation I got the following warning:

Notice: Undefined index:  locale in /Users/amorton/Sites/dh/install.php on line 558

Call Stack:
    0.0049     375236   1. {main}() /Users/amorton/Sites/dh/install.php:0
    0.0081     597108   2. install_main() /Users/amorton/Sites/dh/install.php:1189
    0.1237    6350756   3. install_select_locale() /Users/amorton/Sites/dh/install.php:111

And no languages were displayed. I'm not sure if it was because there's a difference in D6 and D7 translations or a bug in the patch.

After reverting the patch I notice that there's more warnings but the form actually appears. It seems like the call to drupal_get_form('install_select_locale_form', $locales); at the bottom of install_select_locale() isn't finding the form.

Most probably because the registry has no idea of Locale module / locale.inc during installation. #310467: Slimmer hook_theme() contains a fix for this, which allows us to load locale.module + locale.inc in maintenance mode.

Uhm, well #187398: Re-split locale module is quite important for any future development on locale module to happen, so it would be good to bring this forward, so we can get that committed too and get over the cleanups with actual feature development (eg. cleaning up the very dated locale UI).

Turns out that #310467: Slimmer hook_theme() would not help solve this issue, because locale.inc is not at all in the locale module's location and is not referenceable through its info file. Ironically, for that to happen, we would need to fix #187398: Re-split locale module first, which was marked postponed on this issue. So looks like our circle is back to fixing #187398: Re-split locale module first and mark this postponed on that.

Make it active 'cos fixed #187398: Re-split locale module

Now all form-related staff is moved to locale.admin.inc

Because locale.inc is loaded in locale_init() and used only in locale_js_alter() there's no reason to save locale_inc_callback()

Forget to include changed tests

Added comment about locale_js_alter() could be called from simpletest after installing locale.module so locale_init() is not called

include_once() changed to require_once() to unify with other tests

+++ modules/locale/locale.module	22 Feb 2010 03:02:56 -0000
@@ -766,6 +756,9 @@ function locale_js_alter(&$javascript) {
+  // Could be called from simpletest without locale_init().
+  require_once DRUPAL_ROOT . '/includes/locale.inc';

I think it is odd that we specifically reference Simpletest here.

Changed comment, locale_init() sometimes could be skiped

Good! Committed to CVS HEAD. Thanks.