locale_inc_callback() is a thing of the past
Damien Tournoud - November 11, 2008 - 00:45
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | locale.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
With the registry, there is no need for this old beast anymore.
#1
And here's the patch.
#2
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;
+ }
#3
#4
Well spotted lilou, but we need to keep the function call!
Here is a reroll. Ran the test suite, 100% pass.
#5
The last submitted patch failed testing.
#6
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.
#7
See: #335122: Test clean HEAD after every commit and http://pastebin.ca/1258476
#8
Looks like we could remove this nested if:
<?phpif (substr($filepath, 0, strlen($dir)) != $dir) {
+ if (drupal_function_exists('_locale_parse_js_file')) {
?>
#9
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.
#10
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.
#11
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 ofinstall_select_locale()isn't finding the form.#12
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.
#13
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).
#14
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.
#15
Make it active 'cos fixed #187398: Re-split locale module
Now all form-related staff is moved to locale.admin.inc
#16
Because locale.inc is loaded in locale_init() and used only in locale_js_alter() there's no reason to save locale_inc_callback()
#17
The last submitted patch, 332725-locale_inc_callback_d7.patch, failed testing.
#18
Forget to include changed tests
#19
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
#20
+++ 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.
#21
Changed comment, locale_init() sometimes could be skiped
#22
Good! Committed to CVS HEAD. Thanks.
#23
Automatically closed -- issue fixed for 2 weeks with no activity.