Updated: Comment #N

Problem/Motivation

Currently, we have getPluginTypes and viewsHandlerTypes methods on ViewExecutable. These methods are used all over the place so we end up calling ViewExecutable::method. Which in some contexts doesn't look like it makes much sense.

Also, we need getPluginTypes to be more flexible; so we can e.g. just return a list of regular plugin types or just handler types (or all, as we do now).

Proposed resolution

Create a ViewPluginInfo class (bikeshed/find a better name). Move these methods into there. Make them more flexible.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#17 2219689-17.patch1.9 KBdamiankloip
#12 interdiff-2219689-12.txt714 bytesdamiankloip
#12 2219689-12.patch9.22 KBdamiankloip
#8 2219689-8.patch9.18 KBdamiankloip
#3 interdiff-2219689-3.txt6.57 KBdamiankloip
#3 2219689-3.patch9.18 KBdamiankloip
#1 vdc.ViewPluginInfo.patch9.17 KBdamiankloip
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
FileSize
vdc.ViewPluginInfo.patch9.17 KB

Here is a start.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

I wonder why we don't put it into the views class. It is a bit similar to \Drupal::version

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
FileSize
2219689-3.patch9.18 KB
interdiff-2219689-3.txt6.57 KB

That works for me.

Same shit, different class :)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Nice shit!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

3: 2219689-3.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 2219689-3.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Issue tags: +Needs reroll

2219689-3.patch no longer applies.

error: patch failed: core/modules/views/lib/Drupal/views/ViewExecutable.php:1839
error: core/modules/views/lib/Drupal/views/ViewExecutable.php: patch does not apply
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Issue tags: -Needs reroll
FileSize
2219689-8.patch9.18 KB
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs work » Needs review
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/views/lib/Drupal/views/Views.php
@@ -368,4 +382,155 @@ public static function pluginList() {
+    // Statically cache this so t() doesn't run a bajillion times.

t('bajillion')?

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs work » Needs review
FileSize
2219689-12.patch9.22 KB
interdiff-2219689-12.txt714 bytes

OK FINE :)

How about that?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented

Looks good to me.

damiankloip’s picture

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

Back to RTBC in that case :)

Do your worst.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Worst done.

Committed 55b0a05 and pushed to 8.x. Thanks!

  • Commit 55b0a05 on 8.x by alexpott: 
    Issue #2219689 by damiankloip: Separate out logic for getPluginTypes/...
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Fixed » Needs review
FileSize
2219689-17.patch1.9 KB

How did the $plugins property get all the way down there...? Let's move it quickly to be with the other properties.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

ha

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 3c89fb9 and pushed to 8.x. Thanks!

  • Commit 3c89fb9 on 8.x by alexpott: 
    Issue #2219689 followup by damiankloip: Separate out logic for...
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Thanks and apologies, all at the same time :)

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented 
+++ b/core/modules/views/lib/Drupal/views/Views.php
@@ -368,4 +382,156 @@ public static function pluginList() {
+          'title' => static::t('Relationships'),
+          'ltitle' => static::t('relationships'),
+          'stitle' => static::t('Relationship'),
+          'lstitle' => static::t('Relationship'),
...
+          'title' => static::t('Header'),
+          'ltitle' => static::t('header'),
+          'stitle' => static::t('Header'),
+          'lstitle' => static::t('Header'),
...
+          'title' => static::t('Footer'),
+          'ltitle' => static::t('footer'),
+          'stitle' => static::t('Footer'),
+          'lstitle' => static::t('Footer'),
...
+          'title' => static::t('No results behavior'),
+          'ltitle' => static::t('no results behavior'),
+          'stitle' => static::t('No results behavior'),
+          'lstitle' => static::t('No results behavior'),

i dont think static::t() works for the string extractor. We should either open an issue for that or revert them back to t() ? those strings are untranslatable now

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Booo :( open a follow up?

Although, IMO we need to try and make this sort of static t() implementation work?

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented

i agree we need a static version of t(), this usecase never occurred to me..but i dont see the point of having static::translationManager property and fill it from \Drupal::translation()..a \Drupal::t() would do the exact same thing, and would be far less to type for you..

so i guess we need an issue: deprecate t() for \Drupal::t() (To make static things unit testable)?

But i still think we should have another one quick followup here, to remove the static::t() and translationManager bits. do you agree?

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Yes, sounds like a good settlement to me!

ParisLiakos++

Status: Fixed » Closed (fixed)

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

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Status: Closed (fixed) » Needs work

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • alexpott committed 3c89fb9 on 8.3.x 
    Issue #2219689 followup by damiankloip: Separate out logic for...
  • alexpott committed 55b0a05 on 8.3.x
    Issue #2219689 by damiankloip: Separate out logic for getPluginTypes/...

  • alexpott committed 3c89fb9 on 8.3.x 
    Issue #2219689 followup by damiankloip: Separate out logic for...
  • alexpott committed 55b0a05 on 8.3.x
    Issue #2219689 by damiankloip: Separate out logic for getPluginTypes/...

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • alexpott committed 3c89fb9 on 8.4.x 
    Issue #2219689 followup by damiankloip: Separate out logic for...
  • alexpott committed 55b0a05 on 8.4.x
    Issue #2219689 by damiankloip: Separate out logic for getPluginTypes/...

  • alexpott committed 3c89fb9 on 8.4.x 
    Issue #2219689 followup by damiankloip: Separate out logic for...
  • alexpott committed 55b0a05 on 8.4.x
    Issue #2219689 by damiankloip: Separate out logic for getPluginTypes/...

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs work » Fixed

Follow-up for #22 #27 filed #3150721: Review static translation for \Drupal\views\Views::getHandlerTypes()

Status: Fixed » Closed (fixed)

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