At the moment we have a few different init methods; One in HandlerBase, and Plugins (not handlers). There is no init() method in the PluginBase class for these to extend, so each one (like AreaPluginBase, CachePluginBase) are just using the same one. We can probably move this into the base class and remove all of these, just overriding where we need to.
So we could try making all plugins follow the handler type constructor, i.e:
public function init(ViewExecutable $view, &$display, $options = NULL) {}
At the moment this doesn't really work with argument_default and validator plugins as they currently pass in the argument plugin here.
Here is an initial patch. Maybe we could change $display in the signature for $plugin or something instead?
I am fully expecting this to fail in its current state.
| Comment | File | Size | Author |
|---|---|---|---|
| #30 | vdc-1850792-30.patch | 75.21 KB | damiankloip |
| #26 | vdc-1850792-26.patch | 74.43 KB | damiankloip |
| #20 | vdc-1850792-20.patch | 74.13 KB | damiankloip |
| #19 | vdc-1850792-19.patch | 0 bytes | damiankloip |
| #17 | vdc-1850792-17.patch | 74.1 KB | tim.plunkett |
Comments
Comment #1
damiankloip commented@dawehner suggested we could have a setArgument() method on argument_* plugins, so maybe something like this?
Comment #2
damiankloip commentedComment #4
dawehnerWorking on it.
Comment #5
dawehnerIt seems to be that, as the display plugin is a special kind of plugin, we should use and extra method for displays.
Comment #7
dawehnerFinally fixed the errors
Comment #9
dawehnerLet's see.
Comment #11
dawehnerForgot to pull recently.
Comment #13
dawehnerThere we are ...
Comment #15
tim.plunkettI love when there is a real reason for a bug!
The options for argument validators are hidden with #states, and there is a FAPI hack in there to force them to be added to the $form_state regardless. However, that hack doesn't respect Views option defaults, so we have to explicitly specify the default value.
Assuming this passes, I think it's RTBC, but assigning to @damiankloip for a final sign-off.
Comment #16
dawehnerIf we do that, shouldn't we do the same for argument default plugins in ArgumentPluginBase::default_argument_form()?
Comment #17
tim.plunkettYes we should.
Comment #18
damiankloip commentedLooking good to me now. This is a nice clean up!
Comment #19
damiankloip commentedRerolled to current HEAD.
Can we get this one is soon if possible? It touches lots of files, as well as other patches needing to use this new format.
Comment #20
damiankloip commentedOops, sorry, a patch with some bytes is good.
Comment #21
damiankloip commented#20: vdc-1850792-20.patch queued for re-testing.
Comment #23
damiankloip commented#20: vdc-1850792-20.patch queued for re-testing.
Comment #24
dawehner#20: vdc-1850792-20.patch queued for re-testing.
Comment #26
damiankloip commentedRe rolled, I think it was the display array patch?
Comment #28
damiankloip commentedYeah ok, that broke slightly.
Comment #29
tim.plunkettDisplayArray::initializeDisplay() is still calling DisplayPluginBase::init(), I think it now needs to call DisplayPluginBase::initDisplay()
Comment #30
damiankloip commentedYeah, nice catch! I think that is right on. Displays don't have an init() method anymore.
Comment #31
tim.plunkettCool, glad that was an easy fix.
This is a huge plus for DX. Let's get it in!
Comment #32
webchickThis looks like sensible clean-up.
Committed and pushed to 8.x. Thanks!
Comment #33.0
(not verified) commentedUpdated issue summary.
Comment #34
mradcliffeFollow-up created #2223251: InputRequired plugin passes incorrect dependency to init method.