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.
follow up #2009674: Replace theme() with drupal_render() in system module
Need replace theme('status_report') with drupal_render() in system_status().
Pay attention to theme_status_report(). Need good testing, because simply build array:
array(#theme => 'status_report', '#requirements' => $requirements)
cause errors.
Related Issues
#2013258: Simplify render for theme_status_report()
#1938930: Convert theme_system_modules_details() to #type table
Comment | File | Size | Author |
---|---|---|---|
#26 | replace-theme-system-status-2010982-25.patch | 742 bytes | pplantinga |
#15 | 2010982_replace-theme-system-status_15.patch | 1.2 KB | andypost |
#12 | 2010982_replace-theme-system-status_12.patch | 861 bytes | somepal |
#12 | interdiff_2010982_replace-theme-system-status.txt | 861 bytes | somepal |
#12 | system-status.png | 46.81 KB | somepal |
Comments
Comment #1
andypostCan you pot a patch and we'll see how many tests are broken first.
// code wins
Comment #2
somepal CreditAttribution: somepal commentedComment #3
somepal CreditAttribution: somepal commentedI am working on this. screenshot of errors displayed after these modifications attached along with the patch.
Comment #4
somepal CreditAttribution: somepal commentedComment #5
somepal CreditAttribution: somepal commentedSo here's the patch.
Comment #6
andypostshould return render array
any reason to check title?
Please try not touch this
Comment #7
andypostrelated issue #1938930-10: Convert theme_system_modules_details() to #type table
Comment #8
andypostSuppose better of
theme_status_report()
at allComment #9
andypost@Joe9 please re-roll #7
For theme function filed #2013258: Simplify render for theme_status_report()
Comment #11
somepal CreditAttribution: somepal commented@andypost I would need to target
status()
instead ofsystem_status()
now.Comment #12
somepal CreditAttribution: somepal commented@andypost Here's the re-roll for #7.
was to avoid blank rows rendered at the bottom (pls ref. attach image)
Comment #13
somepal CreditAttribution: somepal commentedComment #15
andypostBetter to make Controller to return render array
Comment #16
tstoecklerSuper minor, but would an empty array make more sense here?
Otherwise makes total sense!
Comment #17
andypost@tstoeckler following
hook_theme()
all core defines default as NULL, so not sure it makes sense to introduce new kind of default valueComment #18
tstoecklerOK, fine with me. Looks RTBC to me, but I'll let someone else pull the trigger. :-)
Comment #19
pplantinga CreditAttribution: pplantinga commentedShouldn't we be using drupal_render?
instead of:
use:
Comment #20
pplantinga CreditAttribution: pplantinga commentedShouldn't we be using drupal_render?
instead of:
use:
Comment #21
tstoeckler@pplantinga: We try to render things as late as possible, because as long as they are not rendered one can alter the render array. As soon as they are rendered you only have a string and there is no way to alter it anymore. So whenever simply passing a render array works, we should do it.
Comment #22
pplantinga CreditAttribution: pplantinga commentedFine by me. I understand that it's better, I was just thinking of guidelines on #2006152: [meta] Don't call theme() directly anywhere outside drupal_render() where it says:
Don't try to do anything new/fancy like returning the renderable array instead of rendering it where theme() was, despite this probably being "better" in many cases it will also incur quite a bit of extra testing overhead. We're trying to minimise the scope of this issue. Don't change the name/structure of any existing variables or try to add/remove/merge any existing functions. Touch nothing but the lamp (@see Aladdin). If the patch does change anything else, it will need manual testing and therefore slow/stall what should be relatively simple tasks. @thedavidmeister, who wrote this rule, is aware of the irony/hypocrisy in that the patches he submitted (before writing this rule) all refactored things slightly when they probably shouldn't have :P
Comment #23
tstoecklerAhh, OK. I still think we're fine in this case. Controllers returning render arrays is really standard behavior. Don't feel strongly either way, though.
Comment #24
pplantinga CreditAttribution: pplantinga commentedChanging requirements array to a variable is (hopefully) going to happen here:
#2041283: theme_status_report() is severely broken
Comment #25
pplantinga CreditAttribution: pplantinga commented#2041283: theme_status_report() is severely broken is in
Comment #26
pplantinga CreditAttribution: pplantinga commentedUpdated patch
Comment #27
pplantinga CreditAttribution: pplantinga commentedComment #28
Eric_A CreditAttribution: Eric_A commentedAll issues gone and a perfect one-liner remained.
Comment #29
webchickCommitted and pushed to 8.x. Thanks!
Comment #30.0
(not verified) CreditAttribution: commentedUpdated issue summary.