Make a battleplan for the plugins [#1674356]

For #1497366: Introduce Plugin System to Core it would help to make a proper plan how to handle the converting.

Here is some suggestion:

Copy the existing ViewsObject to a basic plugin which extends PluginAbstract to provide a common base for the handlers and plugins
Start to convert a single existing plugin-Type and figure out whats actually needed on the basic plugin

Proper filesystem structure

lib/Drupal/views/Plugin/PluginBase.php
lib/Drupal/views/QueryPlugin/Plugin.php
lib/Drupal/views/QueryPlugin/Type/QueryPluginType.php

Comment	File	Size	Author
#6	1674356-plugin-classes.patch	1.16 MB	damiankloip
#6
#5	1674356-plugin-classes.patch	788.49 KB	damiankloip
#5
#4	1674356-4.patch	25.5 KB	damiankloip
#4
#2	1674356-plugins.patch	25.47 KB	dawehner
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

merlinofchaos CreditAttribution: merlinofchaos commented 7 July 2012 at 15:58

Here are some additions:

Create a basic interface that we want to use for plugins, and then our PluginBase mostly just implements that interface.

As BojanZ and I discussed,for Query we also want an interface based upon query type. So we'd have a SQLQueryInterface and an EFQQueryInterface.

Also I think the namspace goes lib/Drupal/views/Plugin/Query doesn't it?

Comment #2

dawehner

German

CreditAttribution: dawehner commented 9 July 2012 at 18:42

File	Size
1674356-plugins.patch	25.47 KB

Please don't take this to serious, just backuping some work and hey at least the query is runned.

Comment #3

damiankloip CreditAttribution: damiankloip commented 11 July 2012 at 15:51

I know you have said don't look into this too much, and I'm not :). Just one thing though, incase it's been missed; Don't we need to namespace the Plugins further? So inside the Plugins directory, we need the views plugins to be in the views namespace/subdir (lib/Drupal/views/Plugins/views/..)?

I.e. Drupal\views\Plugin\Query\QueryInterface will need to become Drupal\views\Plugin\views\Query\QueryInterface, or something like....

Comment #4

damiankloip CreditAttribution: damiankloip commented 13 July 2012 at 10:30

File	Size
1674356-4.patch	25.5 KB

I have updated dawehners patch from #2 to reflect the new plugin naming.

Comment #5

damiankloip CreditAttribution: damiankloip commented 14 July 2012 at 17:57

File	Size
1674356-plugin-classes.patch	788.49 KB

Here is another updated patch, This will hopefully get a bulk of the boring work out of this conversion. I have converted all of the handlers and plugins into the new Plugin folder format. I haven't thought a great deal about alot of the class names right now. So please chime in with stuff like this, as the point of this patch is as a bit of a catalyst.

There are currently argument plugins and argument handlers that have been converted. Currently the argument handlers are still in a /Plugins/views/handlers directory. So ideas on the two different argument types welcome.

I have not tested this yet, so please don't expect this to work properly yet!

Comment #6

damiankloip CreditAttribution: damiankloip commented 14 July 2012 at 18:01

File	Size
1674356-plugin-classes.patch	1.16 MB

With the existing plugins removed. I have not had a chance to look at views_ui and views_wizard plugins yet.

Comment #7

damiankloip CreditAttribution: damiankloip commented 14 July 2012 at 19:03

Looks like this patch might start getting a bit out of hand here. I have created a sandbox for this conversion instead: http://drupal.org/sandbox/damiankloip/1685040

Comment #8

dawehner

German

CreditAttribution: dawehner commented 30 July 2012 at 03:36

Status:

Active

» Needs review

We finished the work today, so this issue somehow needs review but i guess we will simply wait for the AnotationsDiscovery patch in core.

Comment #9

30 July 2012 at 04:18

Status:

Needs review

» Needs work

The last submitted patch, 1674356-plugin-classes.patch, failed testing.

Comment #10

aspilicious CreditAttribution: aspilicious commented 30 July 2012 at 07:59

Status:

Needs work

» Needs review

Well while working on some patches: http://drupalcode.org/sandbox/damiankloip/1685040.git/shortlog/refs/head... I started to understand the system (and views :p).

I'm still very new to this but as I understand we removed hook_views_plugins.
So I guess the plugins.inc file is incorrect? or obsolete?

And what about hook_views_plugin_alter than?

Comment #11

dawehner

German

CreditAttribution: dawehner commented 30 July 2012 at 11:22

+    $localization_instance = views_get_plugin('localization', $view->localization_plugin);
+    $this->assertEqual('LocalizationTest', get_class($localization_instance), 'Make sure that init_localization initializes the right translation plugin');

You really want to be sure that the plugin instance got created on the actual $view object.

Some answers to the other questions:

You are absolute right, hook_views_plugins is not needed anymore
Yes plugins.inc is not needed anymore as it only contains old code
Regarding the alter hook: this is something we have to figure out with eclipsegc, but i would guess we need some kind of alter hook in our discovery plugin

Thanks for your help!

Comment #12

EclipseGc CreditAttribution: EclipseGc commented 30 July 2012 at 13:52

I've been wanting to remove the alter logic from the HookDiscovery class and make an AlterDiscoveryDecorator class instead. This could wrap any discovery/decorator and allow alteration to be nested at the appropriate level within the system. This should be a really simple patch and is probably better off happening sooner than later.

Eclipse