We don't need this property really, plus it's not very descriptive.

Files: 
CommentFileSizeAuthor
#25 drupal-1931860-25.patch3.74 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,669 pass(es).
[ View ]
#25 interdiff.txt587 bytesdawehner
#22 drupal-1931860-22.patch4.04 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/views/lib/Drupal/views/ViewExecutable.php.
[ View ]
#22 interdiff.txt881 bytesdawehner
#17 drupal-1931860-17.patch3.62 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 56,544 pass(es).
[ View ]
#17 interdiff.txt650 bytesdawehner
#15 drupal-1931860-15.patch4.26 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 53,325 pass(es).
[ View ]
#15 interdiff.txt1.42 KBdawehner
#13 drupal-1931860-13.patch3.81 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 53,183 pass(es), 7 fail(s), and 4 exception(s).
[ View ]
#13 interdiff.txt1.8 KBdawehner
#11 1931860-11.patch2.94 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 53,174 pass(es), 9 fail(s), and 19 exception(s).
[ View ]
#6 drupal-1931860-6.patch2.8 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 51,544 pass(es), 631 fail(s), and 405 exception(s).
[ View ]
d8.remove-plugin-name.patch1.94 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 52,187 pass(es), 28 fail(s), and 0 exception(s).
[ View ]

Comments

damiankloip’s picture

Status:Active» Needs review

Status:Needs review» Needs work

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

damiankloip’s picture

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

<?php
$this
->view->plugin_name = 'new_plugin';
?>

We could

<?php
$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?

damiankloip’s picture

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

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

dawehner’s picture

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.

dawehner’s picture

StatusFileSize
new2.8 KB
FAILED: [[SimpleTest]]: [MySQL] 51,544 pass(es), 631 fail(s), and 405 exception(s).
[ View ]

What about something like this:

jibran’s picture

Status:Needs work» Needs review
damiankloip’s picture

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

Status:Needs review» Needs work

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

damiankloip’s picture

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

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new2.94 KB
FAILED: [[SimpleTest]]: [MySQL] 53,174 pass(es), 9 fail(s), and 19 exception(s).
[ View ]

Is that what you meant?

Status:Needs review» Needs work

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

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new1.8 KB
new3.81 KB
FAILED: [[SimpleTest]]: [MySQL] 53,183 pass(es), 7 fail(s), and 4 exception(s).
[ View ]

Let's see how much this fixes.

Status:Needs review» Needs work

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

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new1.42 KB
new4.26 KB
PASSED: [[SimpleTest]]: [MySQL] 53,325 pass(es).
[ View ]

What about using even getPlugin on the display directly?

damiankloip’s picture

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

oops.

dawehner’s picture

StatusFileSize
new650 bytes
new3.62 KB
PASSED: [[SimpleTest]]: [MySQL] 56,544 pass(es).
[ View ]

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

damiankloip’s picture

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

damiankloip’s picture

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

dawehner’s picture

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

damiankloip’s picture

Is that being paranoid?

dawehner’s picture

StatusFileSize
new881 bytes
new4.04 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/views/lib/Drupal/views/ViewExecutable.php.
[ View ]

I don't like to argue that :)

Status:Needs review» Needs work

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

jibran’s picture

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

Typo

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new587 bytes
new3.74 KB
PASSED: [[SimpleTest]]: [MySQL] 55,669 pass(es).
[ View ]

Ups.

damiankloip’s picture

Status:Needs review» Reviewed & tested by the community

Looks good.

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 228a2aa and pushed to 8.x. Thanks!

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