Remove plugin_name property from ViewExecutable. [#1931860]

Comment	File	Size	Author
#25	drupal-1931860-25.patch	3.74 KB	dawehner

#25	interdiff.txt	587 bytes	dawehner
#22	drupal-1931860-22.patch	4.04 KB	dawehner

#22	interdiff.txt	881 bytes	dawehner
#17	drupal-1931860-17.patch	3.62 KB	dawehner

#17	interdiff.txt	650 bytes	dawehner
#15	drupal-1931860-15.patch	4.26 KB	dawehner

#15	interdiff.txt	1.42 KB	dawehner
#13	drupal-1931860-13.patch	3.81 KB	dawehner

#13	interdiff.txt	1.8 KB	dawehner
#11	1931860-11.patch	2.94 KB	damiankloip

#6	drupal-1931860-6.patch	2.8 KB	dawehner

	d8.remove-plugin-name.patch	1.94 KB	damiankloip

Comment #1

damiankloip CreditAttribution: damiankloip commented 2 March 2013 at 13:03

Status:

Active

» Needs review

Log in or register to post comments

Comment #2

2 March 2013 at 13:52

Status:

Needs review

» Needs work

The last submitted patch, d8.remove-plugin-name.patch, failed testing.

Log in or register to post comments

Comment #3

damiankloip CreditAttribution: damiankloip commented 2 March 2013 at 14:48

Yeah, so I don't think we can remove this so easily. As argument plugins can alter the style plugin, and this means setting this property, which is then used to load the style plugin in initStyle. We could pass an argument into initStyle? So instead of

$this->view->plugin_name = 'new_plugin';

We could

$this->view->initStyle('new_plugin');

And alter it that way maybe.

EDIT: Meh, we have style_options too that can be overridden. I don't love just setting the property like that, but not sure of a better way at the moment. As a minimum we should rename the plugin_name property on ViewExecutable?

Log in or register to post comments

Comment #4

damiankloip CreditAttribution: damiankloip commented 2 March 2013 at 15:07

Or we could store this info in the build_info array instead?

Meh, that's more confusing to use, and not any cleaner.

Log in or register to post comments

Comment #5

dawehner

German

CreditAttribution: dawehner commented 4 March 2013 at 18:06

One other way could be somehow to set the value on the display BUT then don't interfere with the saving of the view. Currently this would automatically change the style plugin.

One other way could be also to ask the argument plugin explicit whether it wants to provide a different style plugin/style options, but yeah this seems to be really custom as well.

Log in or register to post comments

Comment #6

dawehner

German

CreditAttribution: dawehner commented 18 March 2013 at 09:00

File	Size
drupal-1931860-6.patch	2.8 KB

What about something like this:

Log in or register to post comments

Comment #7

jibran

he/him

Urdu

Sydney, Australia

CreditAttribution: jibran commented 18 March 2013 at 09:35

Status:

Needs work

» Needs review

Log in or register to post comments

Comment #8

damiankloip CreditAttribution: damiankloip commented 18 March 2013 at 09:53

This approach works for me, if it's already set initStyle will honour that.

Log in or register to post comments

Comment #9

18 March 2013 at 10:37

Status:

Needs review

» Needs work

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

Log in or register to post comments

Comment #10

damiankloip CreditAttribution: damiankloip commented 18 March 2013 at 11:00

I think that last patch left some references to plugin_name in initStyle?

Log in or register to post comments

Comment #11

damiankloip CreditAttribution: damiankloip commented 18 March 2013 at 16:43

Status:

Needs work

» Needs review

File	Size
1931860-11.patch	2.94 KB

Is that what you meant?

Log in or register to post comments

Comment #12

18 March 2013 at 17:36

Status:

Needs review

» Needs work

The last submitted patch, 1931860-11.patch, failed testing.

Log in or register to post comments

Comment #13

dawehner

German

CreditAttribution: dawehner commented 18 March 2013 at 19:00

Status:

Needs work

» Needs review

File	Size
interdiff.txt	1.8 KB
drupal-1931860-13.patch	3.81 KB

Let's see how much this fixes.

Log in or register to post comments

Comment #14

18 March 2013 at 19:57

Status:

Needs review

» Needs work

The last submitted patch, drupal-1931860-13.patch, failed testing.

Log in or register to post comments

Comment #15

dawehner

German

CreditAttribution: dawehner commented 18 March 2013 at 21:08

Status:

Needs work

» Needs review

File	Size
interdiff.txt	1.42 KB
drupal-1931860-15.patch	4.26 KB

What about using even getPlugin on the display directly?

Log in or register to post comments

Comment #16

damiankloip CreditAttribution: damiankloip commented 19 March 2013 at 19:46

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/StyleMappingTest.phpundefined
@@ -56,6 +56,7 @@ public function testMappedOutput() {
+    debug($rendered_output);

oops.

Log in or register to post comments

Comment #17

dawehner

German

CreditAttribution: dawehner commented 19 March 2013 at 20:57

File	Size
interdiff.txt	650 bytes
drupal-1931860-17.patch	3.62 KB

Just wondering whether "ups" is also okay in english?

Log in or register to post comments

Comment #18

damiankloip CreditAttribution: damiankloip commented 21 May 2013 at 12:55

#17: drupal-1931860-17.patch queued for re-testing.

Log in or register to post comments

Comment #19

damiankloip CreditAttribution: damiankloip commented 21 May 2013 at 22:52

+++ b/core/modules/views/lib/Drupal/views/ViewExecutable.phpundefined
@@ -705,20 +687,12 @@ public function initStyle() {
       return is_object($this->style_plugin);

I think while we are ripping everything else out of this method we don't need to check is_object() anymore..

The comment above can be moved onto one line too.

Log in or register to post comments

Comment #20

dawehner

German

CreditAttribution: dawehner commented 22 May 2013 at 00:07

Well I still think we want to be sure that not nothing bad is stored on there ...

Log in or register to post comments

Comment #21

damiankloip CreditAttribution: damiankloip commented 22 May 2013 at 17:03

Is that being paranoid?

Log in or register to post comments

Comment #22

dawehner

German

CreditAttribution: dawehner commented 22 May 2013 at 17:55

File	Size
interdiff.txt	881 bytes
drupal-1931860-22.patch	4.04 KB

I don't like to argue that :)

Log in or register to post comments

Comment #23

22 May 2013 at 17:57

Status:

Needs review

» Needs work

The last submitted patch, drupal-1931860-22.patch, failed testing.

Log in or register to post comments

Comment #24

jibran

he/him

Urdu

Sydney, Australia

CreditAttribution: jibran commented 22 May 2013 at 18:33

+++ b/core/modules/views/lib/Drupal/views/ViewExecutable.phpundefined
@@ -732,7 +732,7 @@ public function initPager() {
+      if (isset($this->items_per_page)) {style

Typo