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.
Problem/Motivation
Conceptually, the discovery of templates is part of the Theme Registry. At the moment, the discovery (drupal_find_theme_templates()
) is still in theme.inc
and portions of them call the Theme Registry service.
The practical implication is that extending/modifying template discovery is much harder with these procedural functions.
Proposed resolution
- Create methods on the theme registry service for finding theme templates and theme functions.
- Have the procedural functions to nothing more than wrap the service.
Remaining tasks
Mark functions as deprecated?
Update interface?
User interface changes
None
API changes
Mark the procedural functions as deprecated?
Data model changes
none
Comment | File | Size | Author |
---|---|---|---|
#10 | theme-discovery-2612800-10.patch | 14.51 KB | stevector |
Issue fork drupal-2612800
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
stevectorHere's a first pass.
Comment #4
stevectorI had an errant debug message in that last patch.
Comment #5
stevectorComment #8
stevectorOdd,
fails in one test but this does not:
Trying again.
Comment #9
joelpittetThanks for this clean-up. These is my favourite clean-ups:)
Likely will need those deprecated notices in the docblocks.
re #8 that is strange.
Since this is a OOP thing now it should be findThemeTemplates() or just findTemplates() to save a bit of redundency but most important is the camel casing for methods.
Comment #10
stevectorThanks for the review! This patch adds the deprecated notice, camel casing and fixes an unnecessary line break.
Comment #11
joelpittetThanks for the cleanup @stevector. This is the related type of move #2591515: Move twig_without() to the TwigExtension where all the other filters are and deprecate and this patch is trying to touch the same bit of code #2339447: Improve theme registry build performance by 85% which may cause a re-roll.
Not sure if this will will make it into a patch release or 8.1.x, so I'll just leave it until 8.0.0 is released.
Comment #19
markhalliwellI like the idea of this issue, getting rid of procedural functions, but I think we can do one better and make this one of the goals of #2869859: [PP-1] Refactor theme hooks/registry into plugin managers.
This way, themes (base themes) would finally have the power to properly hook into this process.
Comment #20
markhalliwellComment #21
markhalliwellThe above issue #2869859: [PP-1] Refactor theme hooks/registry into plugin managers really makes this issue a moot point.
This issue really pertains to #2957440: Refactor ThemeRegistry and Theme\Registry for BC purposes.
Comment #31
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #32
andypostneeds re-roll for 10.1 and fix deprecation to that version
Comment #35
Bhanu951 CreditAttribution: Bhanu951 as a volunteer commentedComment #36
smustgrave CreditAttribution: smustgrave at Mobomo commentedMR had just a few failures.
Comment #37
Bhanu951 CreditAttribution: Bhanu951 as a volunteer commentedComment #39
Bhanu951 CreditAttribution: Bhanu951 as a volunteer commentedGetting this error , which is causing the test failures.
Comment #40
andypostUpdated title as only one left
is deprecated and removed
Comment #42
elberHi I just rebased
Comment #43
andypostIt needs work to remove all mentions of "theme function", left a set of review-comments
Comment #45
bharath-kondeti CreditAttribution: bharath-kondeti as a volunteer and at Specbee commentedComment #46
VladimirAusFrom @andypost review:
findThemeFunctions
is not removedcore/lib/Drupal/Core/Theme/Registry.php
is not refactored