Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center. foreach ($some_list as $name => $filename) {
drupal_classloader_register($name, dirname($filename));
drupal_get_filename('module', $name, $filename);
}
this code occurs twice in system_list, it should go into a helper function.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | drupal8.system-register.5.patch | 2.27 KB | sun |
| #2 | drupal8.system-register.2.patch | 2.3 KB | sun |
Comments
Comment #1
catchNow we have the same code in three places after fixing a critical bug, so bumping this to major. Also tagging novice since it should be a straightforward patch. Coming from #1830170: module_list() doesn't register namespaces with the classloader when given a fixed module list..
Comment #2
sunThere's nothing "major" here.
Attached patch introduces
system_register(), which is similar to the originally proposed_system_list_warm(), but limited to registering an extension indrupal_get_filename()anddrupal_classloader()."register" stems from lessons half-way learned via other issues. That is, because technically, we are missing the counterpart,
system_unregister(), since we're currently abusing classloader namespaces for things they were not designed for. Both of the current registries should be able to cope with that, but introducing a complementing unregister should be left to a separate issue; potentially the Extensions service.Comment #4
chx CreditAttribution: chx commentedIssue priority wise, we do not overrule core committers. Assigned wise, you remove it, you add it back.
This is why it fails:
Comment #5
sunSorry, copy/paste mistake.
Also wasn't happy with the parameter name $filepath. So I checked, and everywhere else, we're calling that a URI now, so I renamed the parameter to $uri.
Comment #6
YesCT CreditAttribution: YesCT commentedthe new function has good docs.
replaces the 3 places @catch mentions in #1 and that are mentioned in the issue summary.
and the bot came back green.
rtbc. :)
Comment #7
sunI'm pretty sure that this patch no longer applies.
Comment #8
YesCT CreditAttribution: YesCT commented#5: drupal8.system-register.5.patch queued for re-testing.
Comment #9
YesCT CreditAttribution: YesCT commentedIt applied for me locally. I'll ask the bot to retest.
Comment #10
YesCT CreditAttribution: YesCT commentedyep. still applies and passes the bot.
Comment #11
catchLooks good to me. This will need to be refactored again with the Extensions class, but at least it'll be from one place instead of three. Committed/pushed to 8.x.