Needs work
Project:
Drupal core
Version:
main
Component:
views.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
12 Oct 2012 at 00:13 UTC
Updated:
18 Sep 2025 at 18:06 UTC
Jump to comment: Most recent
Originally this issue was going to be "Re-name hook_views_pre_view()" based on a discussion with @dawehner, but the more I looked into it, the more I realized that a lot of the hook and method names could probably be changed:
ViewExecutable::preview()ViewExecutable::preExecute()
hook_views_pre_view()preExecute()preview() method. In DisplayPluginBase, this runs ViewExecutable::render():
ViewExecutable::build() called:
hook_views_pre_build()hook_views_post_build()hook_views_pre_execute()hook_views_post_execute()hook_views_pre_render() executed for modules and then themes.hook_views_post_render() executed for modules and then themes.ViewExecutable::postExecute()ViewExecutable::executeDisplay()ViewExecutable::preExecute()
hook_views_pre_view()preExecute()execute() method. In DefaultDisplay this runs ViewExecutable::render():
ViewExecutable::build() called:
hook_views_pre_build()hook_views_post_build()hook_views_pre_execute()hook_views_post_execute()hook_views_pre_render() executed for modules and then themes.hook_views_post_render() executed for modules and then themes.ViewExecutable::postExecute()
Comments
Comment #1
xjmComment #2
xjmOne thing that would probably help a lot would be for the pre- and post- "build" and "execute" hooks to have the word "query" in their names.
hook_views_pre_execute()happens duringViewExecutable::execute()which is way after and somewhat decoupled fromViewExecutable::preExecute(), which really had me scratching my head for awhile. So maybe the first step is:hook_views_pre_execute()renamed tohook_views_pre_query()hook_views_post_execute()renamed tohook_views_post_query()Comment #3
xjmAnother note, actually changing hook names should be postponed until after the merge, but I'm leaving the issue open for now to get more feedback.
Comment #4
xjmAnother confusing thing is the way that
ViewExecutable::executeDisplay()calls the display handler'sexecute()which goes back and callsViewExecutable::render()which callsViewExecutable::execute().Comment #5
xjmAnd I find myself wondering what methods we could make protected to make everything a bit less overwhelming.
Comment #5.0
xjmUpdated issue summary.
Comment #6
xjmOkay, actually marking this postponed lest someone come along and think that I'm saying to make any of these changes now. :)
Comment #7
dawehner@xjm
I agree #4 is hard to understand. Also preview() is calling display::preview().
Maybe we could get rid of some of these abstractions, what about removing custom execute/preview/render methods on the display and just keep executeDisplay? This probably needs research and better test coverage first. It's though still cool to have control from your display handler, as some (like ctools context) are pretty awesome flexible based on that.
I'm wondering whether people could mix this up with building the query, i guess no. In general i really like this renaming.
Then we could also rename hook_views_pre_view to hook_views_pre_execute as that's what its doing.
Comment #8
xjmWell, we could also do:
If this is possible, it sounds like a great idea. It could make the DX better (think of how overwhelming the view object is when you dpm() it, and how much redundancy there is) and possibly also help a bit with our performance and memory footprint. I'd agree though that we'd need more test coverage first.
Comment #9
dawehnerThis hooks are probably used for much more then change the query, because you have the full $view object available so you can alter around,
prefix with query maybe let people think that these hooks shouldn't be used for other things.
Comment #10
xjmYeah, that's a fair point. The earlier suggestion is probably better then.
Comment #11
xjmComment #12
xjmSince I've had conversations with three different people over the past week trying to explain Views' internal terminology, I think it's probably time to tackle this.
Comment #12.0
xjmUpdated issue summary.
Comment #13
xjmSo renaming the hooks is no longer in scope during the beta. However, adding this documentation to the views documentation group still is something we should do. I guess this needs to get split into two issues now -- one for the docs, and a postponed one for renaming the hooks. (I wonder if it is possible to "rename" the hooks in 8.1.x, and provide BC by still invoking the old hooks, but marking them deprecated? hm.)
Comment #27
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #28
smustgrave commentedThis seems to still be valid.