Updated: Comment #0

Problem/Motivation

#1862202: Objectify the language system obsoleted all the language_* functions in bootstrap.inc. The most commonly used were not replaced/removed to limit the size of the patch.

Proposed resolution

Replace all the deprecated function calls with their OO counterpart.

function language($type) {
  return \Drupal::languageManager()->getCurrentLanguage($type);
}
function language_list($flags = Language::STATE_CONFIGURABLE) {
  return \Drupal::languageManager()->getLanguages($flags);
}
function language_load($langcode) {
  return \Drupal::languageManager()->getLanguage($langcode);
}
function language_default() {
  return \Drupal::languageManager()->getDefaultLanguage();
}

This issue only applies to tests and procedural code, so there is no need to worry about dependecy injection. Object oriented code is being handled by #2225427: Remove remaining uses of deprecated language functions (mostly in object oriented code).

Remaining tasks

  1. Write a patch
  2. Fix failing tests
  3. Reviews

User interface changes

None

API changes

None

Comments

plach’s picture

Issue summary: View changes
sun’s picture

Issue tags: +@deprecated
plach’s picture

Status: Postponed » Active

The parent issue has been committed.

sun’s picture

function language_list($flags = Language::STATE_CONFIGURABLE) {
  return \Drupal::languageManager()->getLanguages($flags);
}
function language_load($langcode) {
  return \Drupal::languageManager()->getLanguage($langcode);
}
function language_default() {
  return \Drupal::languageManager()->getDefaultLanguage();
}
sun’s picture

Status: Active » Needs review
Issue tags: +API clean-up
FileSize
48.46 KB
PASSED: [[SimpleTest]]: [MySQL] 59,755 pass(es). View

Wowza, this is a much larger effort than I imagined.

Attached patch is just language_list(), and it does not properly injected the language_manager service dependency yet.

Perhaps we should split this issue into three discrete issue for the 3 functions?

plach’s picture

If we have 50K+ patches for each function, I guess it would definitely make sense to have separate issues. Or maybe we could commit a first pass without DI and then a second one where we'd inject the language manager.

cosmicdreams’s picture

OK, I started this issue to remove the language_load function before I found this issue:

#2182133: language_load() is deprecated, replace with provided suggestion.

I will focus on removing the language_load function in favor of it's corresponding method from the language manager.

cosmicdreams’s picture

sun’s picture

Title: Remove deprecated language_* functions from bootstrap.inc » Remove deprecated language_list() function from bootstrap.inc

Good to hear. So let's create the last missing separate issue for language_default().

To be honest, I did not actually expect this patch here to pass tests... While that is great, we still need to properly inject the language manager where possible.

cosmicdreams’s picture

Yea, it does feel a bit excessive to always be calling the language manager from the global drupal object. I ended up starting an issue for language() as well: #2182257: language() is deprecated, replace with suggested function

Both of the issues I started for this are copy-paste jobs (that I went through change by change to make sure it was done with oversight.

ianthomas_uk’s picture

FileSize
47.04 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2166915-remove-uses-language-list-11.patch. Unable to apply patch. See the log in the details link for more information. View

Re-roll, also keeps the language_list function itself to reduce the chance of the patch breaking HEAD.

While injecting the language manager is the preferred option, do we need to do it in this patch, which is already 50k? I think we're better replacing all the procedural functions first on a per-function basis, then going through the OO code adding DI on a per-module basis (i.e. replacing all calls to \Drupal::languageManager(), whatever function was eventually being called).

plach’s picture

+1 on #11

sun’s picture

Status: Needs review » Reviewed & tested by the community
sun’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 2166915-remove-uses-language-list-11.patch, failed testing.

ianthomas_uk’s picture

ianthomas_uk’s picture

On second thoughts, this isn't a great candidate for the script

ianthomas_uk’s picture

Title: Remove deprecated language_list() function from bootstrap.inc » Remove uses of deprecated language functions
Assigned: Unassigned » ianthomas_uk
Issue summary: View changes
FileSize
64.25 KB

This patch is just a combination / reroll of #11, plus #2182133-1: language_load() is deprecated, replace with provided suggestion. by cosmicdreams.

If we're going to split this up, it would make more sense to do so by file rather than function, as otherwise the sub-issues will conflict with each other, especially when we start doing dependency injection. I'm working on a combined patch, but might split it that way if it gets too large.

ianthomas_uk’s picture

Title: Remove uses of deprecated language functions » Remove uses of deprecated language functions in tests and procedural code
Status: Needs work » Needs review
FileSize
67.18 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,731 pass(es). View

OK, I've reduced the scope of this issue to cover just tests and procedural code - i.e. places where we don't need to worry about dependency injection. I'll open a separate issue for the OO code.

No interdiff I'm afraid, but I don't think it would have helped much if I had been able to provide one.

ianthomas_uk’s picture

Issue summary: View changes
ianthomas_uk’s picture

plach’s picture

Looks good, thanks. I found only the following minor issue:

+++ b/core/modules/language/language.module
@@ -484,7 +484,7 @@ function language_save($language) {
+  // propagated and the \Drupal::languageManager()->getLanguages() cache is flushed.

80 chars

Not sure whether we actually need a new issue or whether we can just commit the latest patch and then post a new one.

ianthomas_uk’s picture

FileSize
67.19 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,783 pass(es). View

I've changed that comment to:

  // Update URL Prefixes for all languages after the new default language is
  // propagated and the LanguageManagerInterface::getLanguages() cache is
  // flushed.

No other changes.

plach’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

ianthomas_uk’s picture

Status: Reviewed & tested by the community » Postponed
Related issues: +#2208607: Write script to automate replacement of deprecated functions where possible

Thanks for the review. We don't want to cause unnecessary rerolls of more important patches during drupaldevdays, so postponing for now and I'll reroll this over the weekend if it needs it.

ianthomas_uk’s picture

Status: Postponed » Reviewed & tested by the community
FileSize
67.19 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,973 pass(es). View

Actually, this didn't conflict much at all - just two context changes in one test.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • Commit be83ed2 on 8.x by webchick:
    Issue #2166915 by ianthomas_uk, sun | plach: Remove uses of deprecated...

Status: Fixed » Closed (fixed)

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