Closed (fixed)
Project:
Drupal core
Version:
11.x-dev
Component:
forms system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
6 Jul 2025 at 08:35 UTC
Updated:
1 Aug 2025 at 08:14 UTC
Jump to comment: Most recent, Most recent file
Show commands
changes
Comments
Comment #3
berdirI was tempted to also move form_select_options() as it's a function that in core is only used from template_preprocess_select(), but there are direct calls in contrib, so we might want to move it to FormHelper or so and that is then definitely out of scope.
Similar with form_get_options(), which is actually not called at all in core or contrib and I think we should deprecate that without replacement.
Beside those two, the only thing left in form.inc is then batch functions.
Comment #4
nicxvan commentedI have done a partial review:
I've confirmed all deprecations are 11.3 -> 12
All are __function__
All functions call the right method
All initial preprocess call the right method
All comments point to the new method
Need to run the git command to confirm the body of the methods align.
Comment #5
nicxvan commentedOk I had a chance to finish my review with:
git diff 11.x --color-moved=dimmed_zebra --color-moved-ws=ignore-all-spaceSee #3518822-14: Convert template preprocess hooks in core/includes/theme.inc for an explanation.
With that and my review above this is RTBC.
The only line length changes for Codesniff are the comment in the media module.
Comment #8
catchCommitted/pushed to 11.x, thanks!
Comment #9
mstrelan commentedSorry if I've missed something but why are we deprecating functions that never existed before? Any how come the originals have been renamed? I've definitely seen a template_preprocess function called directly before, even if that seems like a bad idea.
Comment #11
catchNo it's me that missed something - completely missed the double underscore being added here. Reverted for now, and yeah same question as #9 - I can understand deprecating the functions given people do call them from contrib, but for that to work they can't be renamed.
Comment #12
nicxvan commentedYeah I missed that too, I'll add that to my checklist for these conversions.
Comment #15
nicxvan commentedOk I reopened the mr and provided suggestions for all of the functions.
Comment #16
libbna commentedI will work on the suggestions.
Comment #17
libbna commentedComment #18
mstrelan commentedChanges look good to me.
FYI @libbna when there are suggestions on an MR you can just click "apply suggestion" and it will create a commit for you. You can also batch them up and apply them in a single commit too.
Comment #19
berdirSorry about this. I used some multi-line search & replace trickery to add the @deprecated to all template function in form.inc and that resulted in the extra _. Thanks for noticing and fixing it up.
Comment #20
libbna commentedGot it, @mstrelan — I’ll be mindful of this going ahead. Thanks!
Comment #21
needs-review-queue-bot commentedThe 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.
Comment #22
berdirRebased, the media preprocess hook moved to MediaThemeHooks.
Comment #25
catchCommitted/pushed to 11.x, thanks!