This is the path module counterpart for #540294: Move node language settings from Locale to Node module, to move language related code to their respective modules. The same argument stands here. Path module handles the storage of language information for paths, its just the input of the language that is in locale.module. Path module is otherwise full of locale support, such as in path_admin_overview(): http://api.drupal.org/api/drupal/modules--path--path.admin.inc/function/...

We'd want to move the tests related to language path support to path module, as well I guess, not done yet.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

Status: Active » Needs work

Could we do #access => module_exists('locale'); maybe? That way any other form alter on this doesn't need to take into account whether locale module is enabled or not - structure will be the same.

Gábor Hojtsy’s picture

@catch: how do you call locale_language_list('name') then? module_invoke()? That sounds like would present a separate language list if locale is not there, and could result in validation failures (the possible options would not have the selected language, but you cannot change it)?

catch’s picture

Status: Needs work » Needs review

hmm, fair enough. I had meant to mark this CNR from active anyway.

It'd be good if we could come up with a better pattern than module_exists() for stuff like this, really dislike that function, but obviously not for this issue.

Status: Needs review » Needs work

The last submitted patch, decouple-path-and-locale.patch, failed testing.

franz’s picture

+++ b/modules/path/path.admin.inc
@@ -130,11 +130,23 @@ function path_admin_form($form, &$form_state, $path = array('source' => '', 'ali
+  if (module_exists('locale')) {
+    $form['language'] = array(
+      '#type' => 'select',
+      '#title' => t('Language'),
+      '#options' => array(LANGUAGE_NONE => t('All languages')) + locale_language_list('name'),
+      '#default_value' => $form['language']['#value'],

You probably meant '#default_value' => $path['language'], is that so?

Gábor Hojtsy’s picture

Yes, that seems to be bad-copy-paste. Can you help with a fixed patch? :)

franz’s picture

Status: Needs work » Needs review
FileSize
2.26 KB

Just wanted to check. That should do it.

Gábor Hojtsy’s picture

Looks better, thanks :)

David Lesieur’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +montreal

Checked the code, played with it on a test site, looks good.

tarmstrong’s picture

Just wanted to link this issue to #1280996: New language_select element type for form API which is hopefully using the general idea of this patch to create a language_select element in more places in Core.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks. Marking 'fixed'.

Gábor Hojtsy’s picture

Thanks, we'll continue abstracting this out in #1280996: New language_select element type for form API. People interested please go there.

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