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.

CommentFileSizeAuthor
#7 decouple-path-and-locale.patch2.26 KBfranz
decouple-path-and-locale.patch2.27 KBgábor hojtsy

Comments

catch’s picture

catch
he/him
Primary language English
commented
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

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

@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

catch
he/him
Primary language English
commented
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

franz
any
Location Montréal
commented 
+++ 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

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

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

franz’s picture

franz
any
Location Montréal
commented
Status: Needs work » Needs review
StatusFileSize
new decouple-path-and-locale.patch2.26 KB

Just wanted to check. That should do it.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Looks better, thanks :)

David Lesieur’s picture

David Lesieur commented
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

tarmstrong commented

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

dries commented
Status: Reviewed & tested by the community » Fixed

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

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

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.