Instead of LANGUAGE_TYPE_CONTENT, LANGUAGE_TYPE_URL, etc, we should have Language::CONTENT, Language::URL, etc. This will give us benefit of the autoloader, and reduce the amount of code in bootstrap.inc.

Postponed for:

Proposed change

<

Thanks! Should we prefix them with LANGUAGE_ even though they are under the Language class? Seems superfluous to me.

I totally agree with you.

Here is a list of constants the current patch moves, with a suggestion of a new naming. We should then probably move this to the issue summary.

Before After
LANGUAGE_SYSTEM Language::LANGCODE_SYSTEM
LANGUAGE_NOT_SPECIFIED Language::LANGCODE_NOT_SPECIFIED
LANGUAGE_NOT_APPLICABLE Language::LANGCODE_NOT_APPLICABLE
LANGUAGE_DEFAULT Language::LANGCODE_DEFAULT
LANGUAGE_CONFIGURABLE Language::STATE_CONFIGURABLE
LANGUAGE_LOCKED Language::STATE_LOCKED
LANGUAGE_ALL Language::STATE_ALL
LANGUAGE_SITE_DEFAULT Language::STATE_SITE_DEFAULT
LANGUAGE_TYPE_CONTENT Language::TYPE_CONTENT
LANGUAGE_TYPE_INTERFACE Language::TYPE_INTERFACE
LANGUAGE_TYPE_URL Language::TYPE_URL
LANGUAGE_LTR Language::DIRECTION_LTR
LANGUAGE_RTL Language::DIRECTION_RTL
Files: 
CommentFileSizeAuthor
#86 drupal-1620010-86.patch419.01 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,765 pass(es).
[ View ]
#86 interdiff.txt510 bytesdawehner
#84 drupal-1620010-84.patch418.87 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 55,875 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#84 interdiff.txt1.97 KBParisLiakos
#77 drupal-1620010-76.patch418.46 KBjibran
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]
#77 interdiff.txt658 bytesjibran
#76 drupal-1620010-76.patch418.46 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 55,539 pass(es), 96 fail(s), and 51 exception(s).
[ View ]
#76 interdiff.txt658 bytesParisLiakos
#72 drupal-1620010-72.patch418.15 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 46,111 pass(es), 229 fail(s), and 11,211 exception(s).
[ View ]
#72 interdiff.txt2.61 KBdawehner
#69 drupal-1620010-66.patch420.75 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Repository checkout: failed to checkout from [git://git.drupal.org/project/drupal.git].
[ View ]
#58 drupal-1620010-58.patch431.48 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1620010-58.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#58 interdiff.txt1.52 KBdawehner
#55 drupal-1620010-55.patch431.47 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 53,884 pass(es), 274 fail(s), and 157 exception(s).
[ View ]
#49 drupal-1620010-49.patch430.58 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 53,941 pass(es), 4 fail(s), and 4 exception(s).
[ View ]
#47 drupal-1620010-47.patch430.49 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1620010-47.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#42 drupal-1620010-42.patch429.72 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 53,869 pass(es).
[ View ]
#42 interdiff.txt4.45 KBdawehner
#40 drupal-1620010-40.patch427.24 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 53,619 pass(es), 26 fail(s), and 0 exception(s).
[ View ]
#40 interdiff.txt3.02 KBdawehner
#37 drupal-1620010-37.patch429.2 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 53,683 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#35 drupal-1620010-35.patch427.91 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 52,315 pass(es), 15 fail(s), and 3 exception(s).
[ View ]
#32 drupal-1620010-32.patch425.17 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]
#32 interdiff.txt652 bytesdawehner
#30 drupal-1620010-30.patch425.2 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/translation_entity/lib/Drupal/translation_entity/Tests/EntityTranslationWorkflowsTest.php.
[ View ]
#22 drupal-1620010-22.patch380.86 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 47,943 pass(es), 246 fail(s), and 7,105 exception(s).
[ View ]
#22 interdiff.txt7.34 KBdawehner
#20 language-move_constants-1620010-20.interdiff.do_not_test.patch17.61 KBplach
#20 language-move_constants-1620010-20.patch379.27 KBplach
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/lib/Drupal/Core/Language/Language.php.
[ View ]
#19 drupal-1620010-19.patch379.4 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/lib/Drupal/Core/Language/Language.php.
[ View ]
#13 drupal-1620010-13.patch380.01 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1620010-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#9 drupal-1620010-9.patch152.46 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/lib/Drupal/Core/Language/Language.php.
[ View ]

Comments

plach’s picture

Issue tags:-D8MI, -sprint, -language-base

Perhaps the variables should be better "scoped": what about Language::TYPE_(INTERFACE|CONTENT|URL)?

plach’s picture

I mean this would leave room for migrating other constants that now live in boostrap.inc. I'm thinking about Language::NOT_SPECIFIED, Language::NOT_APPLICABLE and so on. We probably want to that here as well.

RobLoach’s picture

Tagging.

Gábor Hojtsy’s picture

I think the main question still is what do we want to do with #1512424: Add a LanguageInterface, and property setters/getters to Language class? If you language_load('de'), you *don't* get an instance of the language class. See http://drupal.org/node/1512424#comment-6101938 - I think this would make sense if we use the Language class more globally. Currently, its limited to the DI / negotiation layer only.

RobLoach’s picture

Hmm, seems like there are more places where we could take advantage of this :-) . Do you think having language_list() and language_load() return Language objects should be in this done in this issue, or a separate one?

Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

Status:Postponed» Active

All of those landed, so looks like this can now be started, right?

Gábor Hojtsy’s picture

Issue tags:-sprint

Moving off the D8MI focus board.

dawehner’s picture

StatusFileSize
new152.46 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/lib/Drupal/Core/Language/Language.php.
[ View ]

Just started to move the constants to the language class, but left out most of LANGUAGE_NOT_SPECIFIED and LANGUAGE_DEFAULT.

Renaming is easy if you have all use statements in place.

Gábor Hojtsy’s picture

Thanks! Should we prefix them with LANGUAGE_ even though they are under the Language class? Seems superfluous to me. Also, different language constants have very different meanings. Eg. LANGUAGE_TYPE_* are language negotiation types, not types of languages :)

plach’s picture

For those I'd go with #1.

dawehner’s picture

<

Thanks! Should we prefix them with LANGUAGE_ even though they are under the Language class? Seems superfluous to me.

I totally agree with you.

Here is a list of constants the current patch moves, with a suggestion of a new naming. We should then probably move this to the issue summary.

Before After
LANGUAGE_SYSTEM Language::LANGCODE_SYSTEM
LANGUAGE_NOT_SPECIFIED Language::LANGCODE_NOT_SPECIFIED
LANGUAGE_NOT_APPLICABLE Language::LANGCODE_NOT_APPLICABLE
LANGUAGE_DEFAULT Language::LANGCODE_DEFAULT
LANGUAGE_CONFIGURABLE Language::STATE_CONFIGURABLE
LANGUAGE_LOCKED Language::STATE_LOCKED
LANGUAGE_ALL Language::STATE_ALL
LANGUAGE_SITE_DEFAULT Language::STATE_SITE_DEFAULT
LANGUAGE_TYPE_CONTENT Language::NEGOTIATION_CONTENT
LANGUAGE_TYPE_INTERFACE Language::NEGOTIATION_INTERFACE
LANGUAGE_TYPE_URL Language::NEGOTIATION_URL
LANGUAGE_LTR Language::DIRECTION_LTR
LANGUAGE_RTL Language::DIRECTION_RTL
dawehner’s picture

StatusFileSize
new380.01 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1620010-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Really not sure whether it's worth to convert that now, as it will certainly conflict with all the EntityNG changes.

alexpott’s picture

Status:Active» Needs review

Let's see what the testbot thinks

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-13.patch, failed testing.

plach’s picture

@dawehner:

Overall the proposal looks good to me, but:

LANGUAGE_SYSTEM Language::LANGCODE_SYSTEM

These could become simpler: Language::CODE_SYSTEM.

LANGUAGE_TYPE_CONTENT Language::NEGOTIATION_CONTENT

These sould really be prefixed with TYPE: we spent a lot of time discussing and standardizing on a terminology: Language::TYPE_CONTENT.

Gábor Hojtsy’s picture

Agreed with @plach on TYPE, not on CODE :) We also spent a lot of time standardising on "langcode" vs. anything else :D

plach’s picture

Right :)

plach’s picture

Issue summary:View changes

Updated issue summary.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new379.4 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/lib/Drupal/Core/Language/Language.php.
[ View ]

Thank you for the feedback!

Added the feedback from #16 and #17 to the issue summary and updated the patch.

Seriously, I need a better internet connection for these kind of patches :)

plach’s picture

StatusFileSize
new379.27 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/lib/Drupal/Core/Language/Language.php.
[ View ]
new17.61 KB

This fixes some wrong constant names and rewraps comments to column 80. The interdiff covers only the latter.

Status:Needs review» Needs work

The last submitted patch, language-move_constants-1620010-20.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new7.34 KB
new380.86 KB
FAILED: [[SimpleTest]]: [MySQL] 47,943 pass(es), 246 fail(s), and 7,105 exception(s).
[ View ]

Fixes hopefully all problems.

fago’s picture

LANGUAGE_DEFAULT is right now not used as constant for language_default() but for the default language of an entity. Still, we could keep the name and use it to refer to the default language in whatever context? Or is that confusing and we should rename it?

Gábor Hojtsy’s picture

@fago: not sure how can LANGUAGE_DEFAULT ('und') accurate to signify defaultness at places where the default might be different. Eg. content types, vocabularies, etc. have a setting of default language for new entities created, and one possible option is 'und', while there are other options. So the default language of those entities when created is not necessarily 'und', however that is one of the possible options as well, so I'm not seeing how it can be used generally as-is. Not sure what it was meant to do, I believe it was introduced with the "property api" patches.

plach’s picture

@fago:

Do you mean using a LANGUAGE_DEFAULT constant instead of language_default()?

fago’s picture

It's used to refer to the entity default language - what ever it is. It's value is 'und' right now because of the BC-mode, however a new, better BC-mode just got committed. I think with the new BC mode changing the constant should work now - thus we could move it over to a more reasonable value.

Do you mean using a LANGUAGE_DEFAULT constant instead of language_default()?

No, language_default() is getting the site's default language. I'd use the constant to refer to the language default depending on the context, e.g. in site context it's language_default() while in entity context it's $entity->langcode->value.

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-22.patch, failed testing.

fago’s picture

Status:Needs work» Needs review

Gave it a short test - no unfortunately the new BC-mode does not allow to change the constant right now either. We'll need to wait for the BC mode to be removed for changing it.

I also noted we already have a site default language constant. Should we just make it an entity default language constant?

plach’s picture

No, language_default() is getting the site's default language. I'd use the constant to refer to the language default depending on the context, e.g. in site context it's language_default() while in entity context it's $entity->langcode->value.

Ok, now I get it :)

Not sure about this, I think we should try and see how it looks while addressing the active language/active translation issue. See for instance #1807692-13: Introduce a column synchronization capability and use it to translate alt and titles through the image field widget (EntityTranslationController.php and related hunks): the current solution is a hack, we should try to find a better one. For instance passing along an EntityTranslation object instead of an Entity object. Anyway we'll need an issue to dicuss this.

dawehner’s picture

StatusFileSize
new425.2 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/translation_entity/lib/Drupal/translation_entity/Tests/EntityTranslationWorkflowsTest.php.
[ View ]

Just another rerole.

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-30.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new652 bytes
new425.17 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-32.patch, failed testing.

dawehner’s picture

Issue tags:+PHPUnit Blocker

.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new427.91 KB
FAILED: [[SimpleTest]]: [MySQL] 52,315 pass(es), 15 fail(s), and 3 exception(s).
[ View ]

Another rerole, hopefully I catched all of them.

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-35.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new429.2 KB
FAILED: [[SimpleTest]]: [MySQL] 53,683 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Less fails, yeah!

dawehner’s picture

+++ b/core/includes/bootstrap.incundefined
@@ -2770,13 +2686,13 @@ function language_multilingual() {
+ *   It can be: Language::STATE_CONFIGURABLE, Language::STATE_LOCKED, Language::STATE_ALL.

+++ b/core/includes/language.incundefined
@@ -528,18 +530,18 @@ function language_url_split_prefix($path, $languages) {
+    // Get languages ordered by weight, add Language::LANGCODE_NOT_SPECIFIED at the end.

+++ b/core/lib/Drupal/Core/Entity/DatabaseStorageControllerNG.phpundefined
@@ -245,9 +246,9 @@ protected function attachPropertyData(array &$entities, $load_revision = FALSE)
+        // Field values in default language are stored with Language::LANGCODE_DEFAULT as

+++ b/core/lib/Drupal/Core/Entity/EntityBCDecorator.phpundefined
@@ -118,28 +119,28 @@ public function &__get($name) {
+      // Language::LANGCODE_DEFAULT by mapping the values to Language::LANGCODE_DEFAULT. This is
...
+      // Language::LANGCODE_DEFAULT while field API expects them to be keyed by langcode.

@@ -160,18 +161,18 @@ public function __set($name, $value) {
+        // with Language::LANGCODE_DEFAULT while field API expects them to be keyed by

+++ b/core/lib/Drupal/Core/Entity/EntityNG.phpundefined
@@ -38,7 +38,7 @@ class EntityNG extends Entity {
+   * are keyed by language code, whereas Language::LANGCODE_DEFAULT is used for values in

@@ -140,12 +140,12 @@ public function uuid() {
+    // Values in default language are always stored using the Language::LANGCODE_DEFAULT

@@ -161,9 +161,9 @@ protected function getTranslatedField($property_name, $langcode) {
+      // Non-translatable fields are always stored with Language::LANGCODE_DEFAULT as key.

@@ -287,15 +287,15 @@ public function language() {
+    // If the default language is Language::LANGCODE_NOT_SPECIFIED, the entity is not

@@ -347,15 +347,15 @@ public function getTranslationLanguages($include_default = TRUE) {
+    // We include the default language code instead of the Language::LANGCODE_DEFAULT

+++ b/core/lib/Drupal/Core/Language/Language.phpundefined
@@ -20,13 +20,97 @@ class Language {
+   * @todo: Change value to differ from Language::LANGCODE_NOT_SPECIFIED once field API
+   * leverages the property API.

+++ b/core/lib/Drupal/Core/Language/LanguageManager.phpundefined
@@ -115,7 +115,7 @@ public function getLanguage($type) {
+   *   Language::TYPE_INTERFACE, or NULL to reset all language types. Defaults to

+++ b/core/lib/Drupal/Core/TypedData/TranslatableInterface.phpundefined
@@ -40,7 +40,7 @@ public function getTranslationLanguages($include_default = TRUE);
+   *   The language code of the translation to get or Language::LANGCODE_DEFAULT to get

+++ b/core/modules/field/field.multilingual.incundefined
@@ -19,10 +21,10 @@
+ * - For untranslatable fields this set only contains Language::LANGCODE_NOT_SPECIFIED.
...
+ *   installed languages with the addition of Language::LANGCODE_NOT_SPECIFIED. This

@@ -102,7 +104,7 @@ function field_language_delete() {
+ * language codes will be returned, otherwise only Language::LANGCODE_NOT_SPECIFIED will

@@ -125,7 +127,7 @@ function field_available_languages($entity_type, $field) {
+    // of enabled languages, otherwise we return just Language::LANGCODE_NOT_SPECIFIED.

@@ -258,7 +260,7 @@ function field_valid_language($langcode, $default = TRUE) {
+ * display language code to be used is just Language::LANGCODE_NOT_SPECIFIED, as no other

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.phpundefined
@@ -188,11 +189,11 @@ public function query($use_groupby = FALSE) {
+        // By the same reason as field_language the field might be Language::LANGCODE_NOT_SPECIFIED in reality so allow it as well.

@@ -846,11 +847,11 @@ function field_langcode(EntityInterface $entity) {
+      // (or Language::LANGCODE_NOT_SPECIFIED), in case the field has no data for the selected language.

+++ b/core/modules/locale/lib/Drupal/locale/Tests/LocalePathTest.phpundefined
@@ -129,11 +130,11 @@ function testPathLanguageConfiguration() {
+    // Assign a custom path alias to second node with Language::LANGCODE_NOT_SPECIFIED.

+++ b/core/modules/locale/locale.bulk.incundefined
@@ -420,7 +420,7 @@ function locale_translate_batch_build($files, $options) {
+ *   contain a language parameter (other than Language::LANGCODE_NOT_SPECIFIED). This

+++ b/core/modules/views/views.moduleundefined
@@ -634,14 +635,14 @@ function views_add_contextual_links(&$render_element, $location, ViewExecutable
+ *   It can be: Language::STATE_CONFIGURABLE, Language::STATE_LOCKED, Language::STATE_ALL.

linebreak.

+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentNewIndicatorTest.phpundefined
@@ -53,8 +55,8 @@ public function testCommentNewCommentsIndicator() {
diff --git a/core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php b/core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php

diff --git a/core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php b/core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php
index 7910196..8b9a09a 100644

index 7910196..8b9a09a 100644
--- a/core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php

--- a/core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php
+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.phpundefined

+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.phpundefined
+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.phpundefined
@@ -7,8 +7,6 @@

@@ -7,8 +7,6 @@

namespace Drupal\comment\Tests;

-use Drupal\simpletest\WebTestBase;
-
/**
  * Tests comments with node access.

Let's keep this small fix out of the patch.

+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentTokenReplaceTest.phpundefined
@@ -23,7 +25,7 @@ public static function getInfo() {
diff --git a/core/modules/comment/lib/Drupal/comment/Tests/Views/DefaultViewRecentComments.php b/core/modules/comment/lib/Drupal/comment/Tests/Views/DefaultViewRecentComments.php

diff --git a/core/modules/comment/lib/Drupal/comment/Tests/Views/DefaultViewRecentComments.php b/core/modules/comment/lib/Drupal/comment/Tests/Views/DefaultViewRecentComments.php
index 46d0318..252d06e 100644

index 46d0318..252d06e 100644
--- a/core/modules/comment/lib/Drupal/comment/Tests/Views/DefaultViewRecentComments.php

--- a/core/modules/comment/lib/Drupal/comment/Tests/Views/DefaultViewRecentComments.php
+++ b/core/modules/comment/lib/Drupal/comment/Tests/Views/DefaultViewRecentComments.phpundefined

+++ b/core/modules/comment/lib/Drupal/comment/Tests/Views/DefaultViewRecentComments.phpundefined
+++ b/core/modules/comment/lib/Drupal/comment/Tests/Views/DefaultViewRecentComments.phpundefined
@@ -7,8 +7,7 @@

@@ -7,8 +7,7 @@

namespace Drupal\comment\Tests\Views;

-use Drupal\entity\DatabaseStorageController;
-use  Drupal\views\Tests\ViewTestBase;
+use Drupal\views\Tests\ViewTestBase;

class DefaultViewRecentComments extends ViewTestBase {

dito

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.phpundefined
@@ -60,7 +62,7 @@ function testBreadCrumbs() {
diff --git a/core/modules/system/lib/Drupal/system/Tests/Path/AliasTest.php b/core/modules/system/lib/Drupal/system/Tests/Path/AliasTest.php

diff --git a/core/modules/system/lib/Drupal/system/Tests/Path/AliasTest.php b/core/modules/system/lib/Drupal/system/Tests/Path/AliasTest.php
index cd432a7..56f7ef9 100644

index cd432a7..56f7ef9 100644
--- a/core/modules/system/lib/Drupal/system/Tests/Path/AliasTest.php

--- a/core/modules/system/lib/Drupal/system/Tests/Path/AliasTest.php
+++ b/core/modules/system/lib/Drupal/system/Tests/Path/AliasTest.phpundefined

+++ b/core/modules/system/lib/Drupal/system/Tests/Path/AliasTest.phpundefined
+++ b/core/modules/system/lib/Drupal/system/Tests/Path/AliasTest.phpundefined
@@ -148,7 +148,6 @@ function testLookupPath() {

@@ -148,7 +148,6 @@ function testLookupPath() {
     // Test the situation where the alias and language are the same, but
     // the source differs. The newer alias record should be returned.
     $pathObject->save('user/2', 'bar');
-    $this->assertEqual($aliasManager->getSystemPath('bar'), 'user/2', 'Newer alias record is returned when comparing two LANGUAGE_NOT_SPECIFIED paths with the same alias.');
   }

Damn, this was probably part of the merge conflict.

+++ b/core/modules/system/system.installundefined
@@ -1396,8 +1397,8 @@ function system_update_8004() {
+    // language. Our best guess is to set all of them to Language::LANGCODE_NOT_SPECIFIED.

linebreak.

+++ b/core/modules/system/tests/upgrade/drupal-7.language.database.phpundefined
@@ -422,7 +422,7 @@
+// is a simple node with Language::LANGCODE_NOT_SPECIFIED as language code. The second

linebreak.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined
@@ -1200,7 +1201,7 @@ public function optionsSummary(&$categories, &$options) {
+      Language::LANGCODE_NOT_SPECIFIED => t('Language neutral'),

Indentation.

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-37.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new3.02 KB
new427.24 KB
FAILED: [[SimpleTest]]: [MySQL] 53,619 pass(es), 26 fail(s), and 0 exception(s).
[ View ]

It seems to make sense to skip the line breaks in this issue, as they have the potential to blow up this patch even more. Would do you think?

This patch should be really green now.

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-40.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new4.45 KB
new429.72 KB
PASSED: [[SimpleTest]]: [MySQL] 53,869 pass(es).
[ View ]

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-42.patch, failed testing.

dawehner’s picture

I can reproduce the error locally, it's something quite odd: it tells you that the ConfigStorageController uses the LANGUAGE_NOT_SPECIFIED constant, even this constant is not used anywhere.

My theory is that this const is somewhere stored in APC and then PHP get's confused. This one is based upon the experimental behavior of going away when restarting apache.

msonnabaum’s picture

Status:Needs work» Needs review

#42: drupal-1620010-42.patch queued for re-testing.

ParisLiakos’s picture

dreditor crashed on me:/
anyways, good job, i just found that several comments now exceed 80 chars

dawehner’s picture

StatusFileSize
new430.49 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1620010-47.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Wrapped the 80 chars.

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-47.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new430.58 KB
FAILED: [[SimpleTest]]: [MySQL] 53,941 pass(es), 4 fail(s), and 4 exception(s).
[ View ]

Rerolled against a views change.

ParisLiakos’s picture

Status:Needs review» Reviewed & tested by the community

ok, went through most of it again, looks legit, i grep'ed, couldnt find anything forgotten
good job!
i hope bot agrees and you dont reroll this many times till commit, although i doubt:/

ParisLiakos’s picture

Status:Reviewed & tested by the community» Needs work

sorry, my grep was overengineered:P
found some left

./core/modules/locale/lib/Drupal/locale/Tests/LocaleTranslationTest.php:    $this->assertTrue(t($name, array(), array('langcode' => Language::LANGCODE_SYSTEM)) == $name, t('t() works for Language::LANGUAGE_SYSTEM.'));

./core/modules/locale/lib/Drupal/locale/Tests/LocaleTranslationTest.php:    $this->assertTrue(t($name, array(), array('langcode' => Language::LANGCODE_SYSTEM)) == $name, t('t() works for Language::LANGUAGE_SYSTEM.'));

./core/modules/node/lib/Drupal/node/Tests/PagePreviewTest.php:      'langcode' => LANGUAGE_NOT_SPECIFIED,
./core/modules/node/lib/Drupal/node/Tests/PagePreviewTest.php:      'langcode' => LANGUAGE_NOT_SPECIFIED,

./core/modules/language/lib/Drupal/language/Form/NegotiationSelectedForm.php:      '#languages' => LANGUAGE_CONFIGURABLE | LANGUAGE_SITE_DEFAULT,

./core/modules/language/lib/Drupal/language/Form/NegotiationSelectedForm.php:      '#languages' => LANGUAGE_CONFIGURABLE | LANGUAGE_SITE_DEFAULT,

dawehner’s picture

Aren't all of those not comments, so 80 chars is not an issue?

dawehner’s picture

Status:Needs work» Needs review

Back to needs review.

ParisLiakos’s picture

Status:Needs review» Needs work

testbot failed..also i dont talk about wraps:)

$this->assertTrue(t($name, array(), array('langcode' => Language::LANGCODE_SYSTEM)) == $name, t('t() works for Language::LANGUAGE_SYSTEM.'));
should be
$this->assertTrue(t($name, array(), array('langcode' => Language::LANGCODE_SYSTEM)) == $name, t('t() works for Language::LANGCODE_SYSTEM.'));

or

./core/modules/node/lib/Drupal/node/Tests/PagePreviewTest.php:      'langcode' => LANGUAGE_NOT_SPECIFIED,
should be Language::LANGCODE_NOT_SPECIFIED

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new431.47 KB
FAILED: [[SimpleTest]]: [MySQL] 53,884 pass(es), 274 fail(s), and 157 exception(s).
[ View ]

I seriously hope we will get this in sooner or later.

git add -p isn't fun on that patch :)

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-55.patch, failed testing.

Gábor Hojtsy’s picture

We will definitely needs avoid commit conflicts on this.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new1.52 KB
new431.48 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1620010-58.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I fear that this requires a LOT of other patches to be rerolled, so rerolling just this over time, seems better for the total community.

plach’s picture

Yep, also because we may want to move some constants to the language manager as proposed in #1862202: Objectify the language system.

dawehner’s picture

@plach
Do you have a prefered order of getting it in? I'm fine with rerolling both of it.

plach’s picture

Well, I think it depends on when I'm able to get back to that issue. If this is ready and there are no big RTBC patches around I guess we can simply go on and reroll #1862202: Objectify the language system afterwards. Or we can simply move the proper constants that to the language manager directly here (don't remember exactly which ones).

Gábor Hojtsy’s picture

@dawehner: agreed with this, removing conflict avoidance tag.

I fear that this requires a LOT of other patches to be rerolled, so rerolling just this over time, seems better for the total community.

alexpott’s picture

Weird... tag was not actually removed

ParisLiakos’s picture

Issue tags:+Needs reroll

tagging with needs reroll

dawehner’s picture

@plach, @gabor
If you think it's time for that patch, just comment here, and I will do another rerole.

Berdir’s picture

Status:Needs review» Needs work
Issue tags:+D8MI, +language-base, +Dependency Injection (DI), +Needs reroll, +PHPUnit Blocker

The last submitted patch, drupal-1620010-58.patch, failed testing.

msonnabaum’s picture

Status:Needs work» Needs review

Doesn't apply anymore, but if a reroll passes, I will RTBC this so hard.

dawehner’s picture

StatusFileSize
new420.75 KB
FAILED: [[SimpleTest]]: [MySQL] Repository checkout: failed to checkout from [git://git.drupal.org/project/drupal.git].
[ View ]

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-66.patch, failed testing.

RobLoach’s picture

Status:Needs work» Needs review

As long as this passes, it's RTBC.

dawehner’s picture

StatusFileSize
new2.61 KB
new418.15 KB
FAILED: [[SimpleTest]]: [MySQL] 46,111 pass(es), 229 fail(s), and 11,211 exception(s).
[ View ]

Removed the unused statements in access controllers.

Status:Needs review» Needs work
Issue tags:-D8MI, -language-base, -Dependency Injection (DI), -Needs reroll, -PHPUnit Blocker

The last submitted patch, drupal-1620010-72.patch, failed testing.

ParisLiakos’s picture

Status:Needs work» Needs review

#72: drupal-1620010-72.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+D8MI, +language-base, +Dependency Injection (DI), +Needs reroll, +PHPUnit Blocker

The last submitted patch, drupal-1620010-72.patch, failed testing.

ParisLiakos’s picture

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new658 bytes
new418.46 KB
FAILED: [[SimpleTest]]: [MySQL] 55,539 pass(es), 96 fail(s), and 51 exception(s).
[ View ]

for your rtbc pleasure

jibran’s picture

Issue tags:+Needs reroll
StatusFileSize
new658 bytes
new418.46 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]

#72 missed LANGUAGE_NOT_APPLICABLE in bootstrap.inc

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-76.patch, failed testing.

jibran’s picture

Status:Needs work» Reviewed & tested by the community
Issue tags:-Needs reroll

Crosspost. RTBC as per #68 and #71

aspilicious’s picture

Status:Reviewed & tested by the community» Needs work

The last submitted patch, drupal-1620010-76.patch, failed testing.

plach’s picture

Status:Needs work» Needs review

#77: drupal-1620010-76.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+D8MI, +language-base, +Dependency Injection (DI), +PHPUnit Blocker

The last submitted patch, drupal-1620010-76.patch, failed testing.

ParisLiakos’s picture

Status:Needs work» Needs review
StatusFileSize
new1.97 KB
new418.87 KB
FAILED: [[SimpleTest]]: [MySQL] 55,875 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

a forgotten use statement, and well a class named Language:)

Status:Needs review» Needs work

The last submitted patch, drupal-1620010-84.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new510 bytes
new419.01 KB
PASSED: [[SimpleTest]]: [MySQL] 55,765 pass(es).
[ View ]

Arg

tim.plunkett’s picture

Status:Needs review» Reviewed & tested by the community

RTBC if green.

alexpott’s picture

Title:Move LANGUAGE constants to the Language class» Change notice: Move LANGUAGE constants to the Language class
Priority:Normal» Critical
Status:Reviewed & tested by the community» Active

Committed f2d710c and pushed to 8.x. Thanks!

Gábor Hojtsy’s picture

Title:Change notice: Move LANGUAGE constants to the Language class» Move LANGUAGE constants to the Language class
Priority:Critical» Normal
Status:Active» Fixed

Added change notice at https://drupal.org/node/2011418

plach’s picture

Awesome :)

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

Anonymous’s picture

Issue summary:View changes

added proposed