There are currently four patterns used by init() methods:

QueryPluginBase.php:           public function init($base_table, $base_field, $options) {


HandlerBase (and children):    public function init(&$view, &$options) {
  
PluginBase (and children):     public function init(&$view, &$display, $options = NULL) {
                               public function init(&$view, &$display, $options = array()) {
                               public function init(&$view, &$display) {
(LocalizationPluginBase)       public function init(&$view) {
(argument plugins)             public function init(&$view, &$argument, $options) {

The last 3 or 4 could all be easily unified, just pick a standard default argument for $options.
I'm not 100% sure about the argument plugin ones.

QueryPluginBase::init() should probably be renamed outright.

In addition, we need to clean up calls to $this->setOptionDefaults($this->options, $this->defineOptions());

CommentFileSizeAuthor
#8 interdiff.txt816 bytesdawehner
#8 drupal-1778356-8.patch18.24 KBdawehner
#6 interdiff.txt823 bytesdawehner
#6 drupal-1778356-6.patch18.19 KBdawehner
#4 drupal-1778356-4.patch17.74 KBdawehner
#2 core-1778356-2.patch17.01 KBdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented

Good idea!

QueryPluginBase::init() should probably be renamed outright.

What about storing the view object on the query plugin? Then we could fetch the base_table and base_field directly form there.
This would be automatically be there if we have unified the init methods.

Regarding the argument plugins, we could introduce a setArgument/getArgument and just call this in get_plugin of the argument plugin base.

dawehner’s picture

dawehner
Primary language German
commented
Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.x-dev
Component: Code » views.module
Status: Active » Needs review
StatusFileSize
new core-1778356-2.patch17.01 KB

Let's start with the query plugins.

Status: Needs review » Needs work

The last submitted patch, core-1778356-2.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new drupal-1778356-4.patch17.74 KB

Rerolled for the getters method, and fixed some of bugs.

Status: Needs review » Needs work

The last submitted patch, drupal-1778356-4.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new drupal-1778356-6.patch18.19 KB
new interdiff.txt823 bytes

Ah getPlugin has to be adapted as well.

Status: Needs review » Needs work

The last submitted patch, drupal-1778356-6.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new drupal-1778356-8.patch18.24 KB
new interdiff.txt816 bytes

Well, it would have been too nice.

aspilicious’s picture

aspilicious commented
Status: Needs review » Needs work

Not yet done. I still see a mixture of

public function init(ViewExecutable $view, &$display) {
public function init(ViewExecutable $view, &$display, $options = NULL) {
public function init(ViewExecutable $view, &$display, $options = array()) {

I would go with the 3th option everywhere

dawehner’s picture

dawehner
Primary language German
commented
Title: Unify and standardize the init() method » Unify and standardize the init() method for the query plugin.
Status: Needs work » Needs review

This patch just tries to fix init methods of query plugins.

aspilicious’s picture

aspilicious commented
Status: Needs review » Reviewed & tested by the community

Than this is done but we need an issue for the other part of the problem written in the original report.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

#8: drupal-1778356-8.patch queued for re-testing.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Thanks! Committed/pushed to 8.x.

Automatically closed -- issue fixed for 2 weeks with no activity.