system_sort_modules_by_info_name() is misnamed

Patch.

This looks good but can't be backported to D7. Thus, I'm going to wait a bit with committing it to D8.

We could consider adding wrapper functions to D7 when there is a straight switch like this, so that people can make modules 'forwards compatible' to the extent possible. So I'm tagging for backport but that likely needs wider discussion.

Attached variant would add a backwards-compatible wrapper to promote forwards compatibility. (To the DeLorean!)

This is patterned on the solution in #1174444: Make the _element_validate_* functions in field.module available for all contrib modules to use.

Dropping to 7.x temporarily for testbot.

Patch passed; back to 8.x

Added issue summary.

#1: rename-system_sort_modules_by_info_name-778346-1.patch queued for re-testing.

Updated issue summary.

Adding link to meta issue.

#5: system_sort_by_info_name-wrapper-D7-778346-5.patch queued for re-testing.

Reroll of the D8 patch for core/.

#9: system_sort_by_info_name-778346-9.patch queued for re-testing.

Still valid, but this is a task. Should also consider whether this should move to the extension system.

Proposed changes have been addressed. Please review it.

+++ b/core/modules/system/system.module
@@ -1056,7 +1056,7 @@ function system_region_list($theme, $show = REGIONS_ALL) {
-function system_sort_modules_by_info_name($a, $b) {
+function system_sort_by_info_name($a, $b) {

The original function needs to be left as a wrapper around the new one, with deprecation warnings.

Added deprecation warnings with the wrapper function for 9.3.x. As mentioned in #22, Please have a look.

Fixed the above(#23) issue. Please have a look.

This needs a test of the deprecation, see item #3 in How we deprecate.

Added test as mentioned in #25.

@vsujeetkumar thanks for the patch
patch apply successfully

Re #27 @vikashsoni adding a screenshot that shows a patch applies provides zero benefit. In the previous comment, the Drupal test runner automatically confirms a patch applies:

I notice this has been pointed out to you before:

• #3220566: Olivero: wide image can overlap sidebar when toolbar open and in vertical mode
• #3217924: Olivero: Convert all colors (blues and grays) to HSL and normalize hues and saturations.

I encourage you to use Slack to work with some of the contributors involved in these issues. We'd be happy to help you find ways to contribute to issues that benefits them and would actually get you credited (providing screenshots of a patch applying does not qualify for issue credit)

The patch looks good. Just a little bit more work:

1. +++ b/core/modules/system/system.module
   @@ -890,9 +890,17 @@ function system_region_list($theme, $show = REGIONS_ALL) {
   - * Array sorting callback; sorts modules by their name.
   + * {@inheritdoc}
   

   I think that this change is wrong. If I am wrong, then where does this method inherit its docblock from?

2. +++ b/core/modules/system/tests/src/Kernel/SystemDeprecationTest.php
   @@ -0,0 +1,30 @@
   +    foreach (\Drupal::service('extension.list.module')->getAllInstalledInfo() as $module => $info) {
   +      $module_info[$module] = new \stdClass();
   +      $module_info[$module]->info = $info;
   +    }
   

   Could we initialize the array before we start using it by adding: $module_info = [];

3. +++ b/core/modules/system/system.module
   @@ -890,9 +890,17 @@ function system_region_list($theme, $show = REGIONS_ALL) {
   +  @trigger_error('system_sort_modules_by_info_name() is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. Implement system_sort_by_info_name() instead. See https://www.drupal.org/project/drupal/issues/778346', E_USER_DEPRECATED);
   
   +++ b/core/modules/system/tests/src/Kernel/SystemDeprecationTest.php
   @@ -0,0 +1,30 @@
   +    $this->expectDeprecation('system_sort_modules_by_info_name() is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. Implement system_sort_by_info_name() instead. See https://www.drupal.org/project/drupal/issues/778346');
   

   The deprecation message must link to the change record (https://www.drupal.org/node/3225624) not the original issue.

let me work on above feedback

Kindly review the patch.

LGTM.

There are some possible follow-ups to this:

1. Move it to a utility class or trait out of system.module

2. Inline the logic in the closure in the three places it's used and remove the public function altogether.

At three usages it's a bit borderline which of these is the better option, so going ahead and committing this one.

This is always being used to sort lists of Extension objects. We should add a public static function sort to \Drupal\Core\Extension\Extension and use that instead.

+++ b/core/modules/system/src/Controller/AdminController.php
@@ -50,7 +50,7 @@ public function index() {
       $module_info[$module]->info = $info;
     }
 
-    uasort($module_info, 'system_sort_modules_by_info_name');
+    uasort($module_info, 'system_sort_by_info_name');
     $menu_items = [];

This method should get the installed module list as extensions form \Drupal\Core\Extension\ModuleHandlerInterface::getModuleList() instead of

    $module_info = $this->moduleExtensionList->getAllInstalledInfo();
    foreach ($module_info as $module => $info) {
      $module_info[$module] = new \stdClass();
      $module_info[$module]->info = $info;
    }

Which is really odd code.

lol funny xpost - I'll open a follow-up when I have a moment.

+++ b/core/modules/system/system.module
@@ -893,6 +893,14 @@ function system_region_list($theme, $show = REGIONS_ALL) {
+/**
+ * Array sorting callback; sorts modules by their name.
+ */
+function system_sort_by_info_name($a, $b) {

If we don't move this to an object then documentation should be improved... the whole point of this change is that this is not only about modules.

Created a follow-up to deal with #35 / #36 / #37...
See #3225779: Provide sort callback on ExtensionList class