The built-in language list for Drupal 8 is in http://api.drupal.org/api/drupal/core%21includes%21standard.inc/function.... As we discussed in the last update patch for it (http://drupal.org/node/1295696#comment-5143816), we want to convert this to CMI storage and eventually keep it up to date from localize.drupal.org. The initial version can be generated based on languages from localize.drupal.org and then updated from there. So sounds like tasks here would be:

1. Figure out a CMI (currently YAML) format for this.
2. Generate a file of that format on localize.drupal.org.
3. Use that script to generate a shipped file for Drupal 8.
4. Replace the current core list with the config file based implementation.
5. Work on actually updating the shipped list from localize.drupal.org in a followup, NOT in this issue.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

disasm’s picture

Assigned: Unassigned » disasm
alexpott’s picture

Status: Active » Needs review
FileSize
15.69 KB

Here's a first stab at this.

The Yaml file was generated using the existing standard_language_list() and saving a new config.

The installer represents an interesting challenge as config will not be in the db at this point so I've added a new function to config.inc to load module config directly.

alexpott’s picture

Added a test for the new function and improved code documentation.

gdd’s picture

I'm not really thrilled with config_load_and_parse_module_config(). If we consider the active store (or possibly the files in the active config directory) as canonical, then we are creating an exception to that here. We already have another exception ($conf) and the more and more of these we pile on, the more confused things will get over time.

That said, I'm not sure I have a better solution other than possibly this doesn't belong in CMI. Is it ever used outside of this use case in the installer?

alexpott’s picture

I guess a possible solution to this would be put the function in install.inc but I was thinking that we could do a follow up patch to but config_install_default_config() to use the same function as this is exactly what it does too.

Gábor Hojtsy’s picture

@heyrocker: yes, it is used later to provide you a list of quick language options in locale module. At that point it should use the active store, but in the installer, that is not really possible. As per the linked issue based on comments from @catch, the idea is that

1. we'd ship a version of this file in core
2. in the installer, we'd try and update the file from localize.drupal.org (online fed CMI update :)
3. later possibly update it from localize.drupal.org too

So the idea is that Drupal ships with a list of languages, that we can update from l.d.o. So we cannot really put this in a PHP array. We can put this in some kind of file that is writable, and really CMI is that type of thing. This is not human edited / managed data, this is a "suggested language list" for language module, then language module has its configuration on its own.

disasm’s picture

Assigned: disasm » Unassigned
Status: Needs review » Active

Unassigning issue since others have made much more progress than I have.

gdd’s picture

I guess I can live with this solution. I agree that if it lands we should add a followup to make config_install_default_config() use it.

Gábor Hojtsy’s picture

Status: Active » Needs work

Ok then if you are fine with it, then a little code review point:

+++ b/core/includes/config.incundefined
@@ -86,3 +86,23 @@ function config_get_storage_names_with_prefix($prefix = '') {
+/**
+ * Parse a config file directly from a module's config directory.
+ *
+ * @param $name
+ *   The name of the configuration object to retrieve. The name corresponds to
+ *   a configuration file. For @code config(book.admin) @endcode, the config
+ *   object returned will contain the contents of book.admin configuration file.
+ * @param $module
+ *   The name of a module that provides the configuration file.
+ *
+ * @return
+ *   An array containing the configuration data.
+ */

Let's document that this is useful for data needed at installation, but does not work with the current config, it works with shipped config only.

Otherwise it is looking good to me :)

alexpott’s picture

Improved code documentation as suggested in (hopefully) the most succinct way.

alexpott’s picture

Status: Needs work » Needs review
alexpott’s picture

I've created a new module in my sandbox (http://drupal.org/sandbox/alexpott/1636130) that allows a site to become a config_server or a config_client... and allows import from the server to the client.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Do you think/believe that that would be slated for core inclusion?

Sounds like as a first step here, this patch looks good, since Greg is fine and I am fine with the changes.

webchick’s picture

Assigned: Unassigned » sun
Status: Reviewed & tested by the community » Needs review

sun said he wanted to comment on this.

webchick’s picture

Oh, but in the meantime:

Just two small things:

1) Let's rename that function to config_install_load_from_module() (name Gábor and I came up with, along the lines of config_install_default_config()). Right now it sounds way to fun and friendly and just BEGGING me to call it, and in actual fact you pretty much NEVER want to call this as a module developer because you will get stale values.

2) @code config(book.admin) @endcode was confusing because:

a) this is documenting the config_install_load_from_module() function, not the config() function
b) if that's actual code, it's a parse error. ;)

Could we just reword that slightly to say something like "For example, the name 'book.admin' would blah blah..."

alexpott’s picture

Function name and documentation improved as suggested by #15.

Status: Needs review » Needs work
Issue tags: -Configuration system, -D8MI, -sprint, -language-base

The last submitted patch, 1632236-16-language_list.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review

#16: 1632236-16-language_list.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Configuration system, +D8MI, +sprint, +language-base

The last submitted patch, 1632236-16-language_list.patch, failed testing.

Gábor Hojtsy’s picture

Does not seem to work on testbot.

To summarize the discussion we had here with @sun and @webchick:

- the list of predefined languages informs the form for setting up your languages, so the chain is
"predefined languages" => "configure site languages" => "site languages"
- while this list has no UI to configure in core, it has the UI on localize.drupal.org and a default is shipped with core
- if the installer figures out it has internet connection, it will try to update this list eventually and therefore Drupal 8 would get new languages without new releases
- distributions will be able to ship with different language list config, need to figure out how do we want to allow for that; have an issue for that for a *long* time at #1351352: Distribution installation profiles are no longer able to override the early installer screens
- if there is no internet connection, the system can use the built-in list
- if there is internet connection, the system will also download .po files for the selected language, this is being developed in #1191488: META: Integrate l10n_update functionality in core

So the conclusion was that this file is either config or not. It would be config of the distro (I think). It behaves the same way as if you have settings that are strongarmed out. It is just a hardwired setting for how a config form will work. We have config forms based on other configs in core (eg. the primary menu select list is based on all your menus), so this is just an example of that too.

alexpott’s picture

Status: Needs work » Needs review
FileSize
17.12 KB

Bah missed a place to change the function name

sun’s picture

Status: Needs review » Needs work

What I also mentioned:

  1. The list of predefined languages is used for much more than the language selector. It is essentially a copy of the W3C standard definition. Whenever any code in Drupal needs to handle languages in any way, it will use this list. I will therefore say that it is fundamentally wrong to allow anyone to adjust this list (which is why it is called "predefined").
  2. The list needs to be properly maintained in case of updates. Languages cannot be removed from the predefined list, if they have been installed on the site. As such, the list of predefined languages has the same maintenance challenges as #1068840: core/includes/standard.inc contains inaccurate country data, for which we will introduce a dedicated update script.

I'm strongly opposed to this change.

It looks like what you actually want is to allow distros/profiles and/or modules to "limit" or customize the list of available languages. That however, means something like this:

core/modules/system/config/system.standard.yml:
# Obviously, only one of both:
language:
    remove: [xx-lolspeak, en-gb]
    only: [en, de, fr]

Thus:

function language_get_customized_predefined_list() {
  $predefined = standard_language_list();
  $config = config('system.standard');
  if ($only = $config->get('language.only')) {
    $predefined = array_intersect_key($predefined, array_flip($only));
  }
  elseif ($remove = $config->get('language.remove')) {
    $predefined = array_diff_key($predefined, array_flip($remove));
  }
  return $predefined;
}

Lastly, for the localize.d.o update/mirror, we should use a similar mechanism as for the list of predefined countries.

(I do not see any need for updating this definition live/online directly from l.d.o. Fetching translations from there makes sense, but these predefined lists really belong into static files in the code base.)

sun’s picture

Assigned: sun » Unassigned
Gábor Hojtsy’s picture

Status: Needs work » Closed (won't fix)

Ok, this is such a small drop in my task list, that I don't care for spending more time trying to convince people. If we are better off storing this list in a PHP array dump in an include file, if that is the best for Drupal 8, then that is it.

Gábor Hojtsy’s picture

Issue tags: -sprint