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
CommentFileSizeAuthor
#6 1674356-plugin-classes.patch1.16 MBdamiankloip
#5 1674356-plugin-classes.patch788.49 KBdamiankloip
#4 1674356-4.patch25.5 KBdamiankloip
#2 1674356-plugins.patch25.47 KBdawehner

Comments

merlinofchaos’s picture

merlinofchaos commented

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?

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new 1674356-plugins.patch25.47 KB

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

damiankloip’s picture

damiankloip commented

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....

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1674356-4.patch25.5 KB

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

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1674356-plugin-classes.patch788.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!

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1674356-plugin-classes.patch1.16 MB

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

damiankloip’s picture

damiankloip commented

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

dawehner’s picture

dawehner
Primary language German
commented
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.

Status: Needs review » Needs work

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

aspilicious’s picture

aspilicious commented
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?

dawehner’s picture

dawehner
Primary language German
commented 
+    $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!

eclipsegc’s picture

eclipsegc commented

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

dawehner’s picture

dawehner
Primary language German
commented

Linking the issue #1705702: Provide a way to allow modules alter plugin definitions

damiankloip’s picture

damiankloip commented
Status: Needs review » Fixed

Now that we have got plugins into views, closing for now. If anyone disagrees, feel free to reopen.

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