Rename Language module's LanguageInterface to ConfigurableLanguageInterface and Language to ConfigurableLanguage

There is lots of activity in this general area ... dont want to step on other toes! So I am working on this as we speak

Famous last words "This is a trival patch to write"
BUT
It should bring clarity to whole raft of other language module issues
So I consider this very useful. ( hence the QuickFix )

locally test :-

1) LanguageUILanguageNegotiationTest.php
2) LanguageListTest.php

Time for testbot to take a look.

PS This also will cause a flurry of rerolls .. feel free to call on me and I will help with that

After a conversation with YesCT I have "feature creeped" this issue to also rename
\Drupal\language\Entity\Language
to be
\Drupal\language\Entity\ConfigurableLanguage

I have tried a few tests locally ... but as always await testbot to return a more definitive answer
( LanguageConfigurationElementTest + NodeAccessLanguageAwareCombinationTest )

I have formed my git diff with a '-M' to ease the review process.

I am about done for the day and will respond to any issues in the morning.

To aid review I am including a list of all the times Language.php as used in core

find . -name 'Language.php'
./core/lib/Drupal/Core/Language/Language.php
./core/lib/Drupal/Core/TypedData/Plugin/DataType/Language.php
./core/modules/language/lib/Drupal/language/Entity/Language.php ( <---* )
./core/modules/language/lib/Drupal/language/Plugin/Condition/Language.php
./core/modules/node/lib/Drupal/node/Plugin/views/field/Language.php
./core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/views/field/Language.php
./core/modules/user/lib/Drupal/user/Plugin/views/field/Language.php
./core/vendor/symfony/validator/Symfony/Component/Validator/Constraints/Language.php

*changed by this patch
I found this useful

This should be rolled into #2271421: Rename Language module's Language to ConfigurableLanguage. It is small, it changes the same lines, and
class Language extends ConfigEntityBase implements ConfigurableLanguageInterface
is confusing, while
class Language extends ConfigEntityBase implements LanguageInterface
and
class ConfigurableLanguage extends ConfigEntityBase implements ConfigurableLanguageInterface
makes sense

Yup awesome.

We should fix those use Drupal\language\Entity\ConfigurableLanguage as LanguageEntity to just use ConfigurableLanguage directly in a follow-up.

Not marking RTBC myself as such a fundamental change - even if it's "just" naming - should be signed off by @Gábor Hojtsy. Assigning for that purpose.

Hehe, schizophrenic me...

I think this new terminology makes sense. Block entities are also CustomBlock and not Block to avoid the confusion in terminology duplicity. I think the ConfigurableLanguage as LanguageEntity removal would be better done here instead of splitting up even more.

Also I am wondering how the other terminology elements line up with this. Eg. this patch kleeps the machine name of the entity as "language_entity", which I guess would be better be "configurable_language" no? Also the config prefix of "entity", etc. So while the change looks good in itself, it leaves several things as-is that gets more mixed-up terminology wise AFAIS.

regarding #10

We should fix those use Drupal\language\Entity\ConfigurableLanguage as LanguageEntity to just use ConfigurableLanguage directly in a follow-up.

+1 That was on my mind as I converted things .... but erred on the size of minimal changes.

Follow up filed

sorry for noise ... closing duplicate

@GaborHojtsy ... Hi Can I ask for clarification regarding #12

looking at language.module language_save() as an example are you saying the local variable $language_entity should change!

  $language_entity = entity_load('language_entity', $language->id);
  if (!$language_entity) {
    $language_entity = entity_create('language_entity', array(
      'id' => $language->id,
    ));
  }

I have to say I disagree as at it root $language_entity is an entity
I would be grateful if you could clarify

Here's my attempt at fixing the part in comment #10.

+1 to #17 patch looks good

Yeah as I said above I think keeping the language_entity config entity name mixes up terminology quite a bit now that a configurable language is brought into the mix too. So I hoped we can align that terminology, eg. rename language_entity to configurable_language then, but I understand the scope may be too big for that. However that is very unlikely to happen after the beta, so I could see core committers would only accept this if there is some chance we can do the renames wholesale for the entity as well.

I don't want to tread on anybodies toes...

So I won't personally recommend this, I will leave it to reviewers, who know better than I to comment. This is just to see what it would look like.

The next change is far from the "Wholesale" solution but restricts itself to changes within the scope of the existing patch.

I am active in another (see related - 2271611 ) issue and this will cause a reroll That issue is talking about the difficulties of naming things .. So I am happy to reroll
if this were to land first...

I think 'configurable_language_entity' is redundant, as there is no language entity that is not configurable. I.e. that fact that is an entity makes it configurable. I would prefer 'configurable_language'.

This change makes Drupal\config_translation\Tests\ConfigTranslationOverviewTest pass locally

Patch no longer applied, this is just a reroll. No conflicts just merging.

I am just posting this as I can't run head at the moment and I am just off to bed ... I will try again in the morning

The entity_type needs changing the in the function of the form hook_ENTITY_TYPE_delete()

part of the solution, the interdiff from #32 I think but need to confirm

  *
  * Delete the potential block visibility settings of the deleted language.
  */
-function block_language_entity_delete(ConfigurableLanguage $language) {
+function block_configurable_language_delete(ConfigurableLanguage $language) {
   // Remove the block visibility settings for the deleted language.

The morning light shows a seg fault on my apache server aghhh... so here is the fix ( see #34 )

Locally Drupal\block\Tests\BlockLanguageTest now passes.

This is just a natural extension of the fix from #34/35.

1. +++ b/core/modules/config_translation/tests/src/ConfigEntityMapperTest.php
   @@ -57,14 +57,14 @@ public function setUp() {
          'names' => array(),
   -      'entity_type' => 'language_entity',
   +      'entity_type' => 'configurable_language',
   

   We do have to update the routes names, as introduced by 2314875 now.

2. +++ b/core/modules/config_translation/tests/src/ConfigEntityMapperTest.php
   @@ -101,7 +101,7 @@ public function testSetEntity() {
   -      ->with('language_entity')
   +      ->with('configurable_language')
   

   Isn't the relationship between language nad configurable_language similar to field_instance and field_instance_config. Did you thought about using that pattern?

3. +++ b/core/modules/locale/locale.module
   @@ -222,9 +222,9 @@ function locale_stream_wrappers() {
   -function locale_language_entity_insert(LanguageEntity $language) {
   +function locale_configurable_language_insert(ConfigurableLanguage $language) {
   
   @@ -233,9 +233,9 @@ function locale_language_entity_insert(LanguageEntity $language) {
   -function locale_language_entity_update(LanguageEntity $language) {
   +function locale_configurable_language_update(ConfigurableLanguage $language) {
   

   Note: These hooks should actually use the interface. Fill a follow up.

Straight reroll, first.

Yes, going back and doing a file diff....

The only change I was conscious off was :-

<        'route_name' => 'config_translation.item.overview.language.edit',
---
>        'route_name' => 'config_translation.item.overview.entity.language_entity.edit_form',

but I see this also was a change

<        ->with('language.edit')
---
>        ->with('entity.language_entity.edit_form')

Just coming back to look at this today ... all issues raised in #40 have been addressed as the patch was rerolled to match HEAD.

+++ b/core/modules/language/language.module
@@ -414,21 +414,21 @@ function language_get_default_langcode($entity_type, $bundle) {
+  $configurable_language = entity_load('configurable_language', $language->id);

It would be nice to use getId() here -there are some other-occurrences too-, but as this is blocking other issues, IMHO we should move forward and fill a follow-up.

The point from #50 will be addressed in the stalled issue https://www.drupal.org/node/2226533
which will one day get committed... honest!!!

 *   links = {
 *     "delete-form" = "entity.language_entity.delete_form",
 *     "edit-form" = "entity.language_entity.edit_form"
 *   }

We have new standards on route names here... these should be entity.configurable_language.* now. See #2285413: [Meta] Standardize entity route names

Applied fix from #52.

@martin107 You need to rename also in core/modules/language/language.routing.yml.

I renamed all instances of language_entity to configurable_language in core/modules/language/language.routing.yml.

+++ b/core/modules/language/language.routing.yml
@@ -25,34 +25,34 @@ language.negotiation_selected:
 entity.language_entity.edit_form:
-  path: '/admin/config/regional/language/edit/{language_entity}'
+  path: '/admin/config/regional/language/edit/{configurable_language}'

Missed this one.

Ah, yes. I found instances of language_entity routes in other places -- will post a patch and interdiff shortly.

OK. Here is the updated patch for core/modules/language/language.routing.yml.

I also used grep -r language_entity ./* --exclude='*.patch' to check if there were any other occurrences of language_entity routes:

./core/modules/config_translation/tests/src/ConfigEntityMapperTest.php:      ->with('entity.language_entity.edit_form')
./core/modules/config_translation/tests/src/ConfigEntityMapperTest.php:      'base_route_name' => 'entity.language_entity.edit_form',
./core/modules/config_translation/tests/src/ConfigEntityMapperTest.php:      'route_name' => 'config_translation.item.overview.entity.language_entity.edit_form',
./core/modules/language/language.links.task.yml:entity.language_entity.edit_form:
./core/modules/language/language.links.task.yml:  route_name: entity.language_entity.edit_form
./core/modules/language/language.links.task.yml:  base_route: entity.language_entity.edit_form
./core/modules/language/tests/src/Menu/LanguageLocalTasksTest.php:    $this->assertLocalTasks('entity.language_entity.edit_form', array(
./core/modules/language/tests/src/Menu/LanguageLocalTasksTest.php:      0 => array('entity.language_entity.edit_form'),

In phpStorm, I used Edit > Find > Replace in path to make the replacements.

Awesome, thanks!

Committed 8a0fa31 and pushed to 8.0.x. Thanks!

https://www.drupal.org/node/2225341 needs an update

Updated the existing change record: https://www.drupal.org/node/2225341/revisions/view/7074371/7520473