Convert Convert Template Preprocess hooks in core/includes

Created on 11 April 2025, about 1 month ago

Problem/Motivation

See πŸ“Œ [pp-1] Convert Template Preprocess hooks to OOP equivalent Postponed: needs info

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component

theme system

Created by

πŸ‡¨πŸ‡­Switzerland berdir Switzerland

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @berdir
  • @berdir opened merge request.
  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland
  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    This now moves everything in theme.inc, but it's a lot: "115 files + 1291 βˆ’ 775" (the template references add a lot of files, and the BC layer duplicated documention results in a lot of additional lines being added but that's mostly temporary)

    I kept pager in a separate commit so far (has its own pretty large unit test that requires a lot of changes and it's a lot of code) and field preprocess as well (also a lot of code, references from other preprocess functions and a internal but used sort callback, see MR review).

    My proposal would be to split those two up into separate issues. They will all conflict due to uses in theme.inc and ThemeCommonElements, but not much more I think.

    There's also a question about internal vs final, especially those generic template preprocess functions like table and container are fairly frequently used in contrib. For me, being able to have them final and not worry about constructor BC is much more helpful then the public API of them, about the only thing that could happen is that we deprecate a template I think, then we could just deprecate it and let it be there.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    it is a lot, even though it's shuffling things around would it be worth breaking up?

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    Yes, as mentioned in #4, I've now split off πŸ“Œ Convert template_preprocess_pager() Needs review and πŸ“Œ Convert field template_preprocess functions in theme.inc Needs review now. I did add authorize report, it's trivial with only two uses and no dependencies and it helps πŸ“Œ Remove 'includes' support from hook_theme() Postponed

    That removes the two most complex cases and about a third of the size off this one.

    I did my best to not include any changes except DI and required reflowing of some comments due to the changed length limits due to them being two spaces more nested. Try using git diff 11.x --color-moved=dimmed_zebra --color-moved-ws=ignore-all-space` to review these issues. It should mark all lines that were just moved and not changed otherwise grey. Doesn't work for the docblocks because those are copied and there don't seem to be any options to highlight that.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    This one appears to have pipeline issues.

    If you are another contributor eager to jump in, please allow the previous poster(s) at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    Pipelines have issues, that's not the fault of this issue, unit tests not passing only on 8.4 is _extremely_ unlikely to be a real issue.

  • The Needs Review Queue Bot β†’ tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide β†’ to find step-by-step guides for working with issues.

  • πŸ‡ΊπŸ‡ΈUnited States dww

    I think this needs a rebase and resolving some merge conflicts after πŸ“Œ Finish deprecating 'Update Manager' related code in Update Status Active landed.

Production build 0.71.5 2024