At the moment we are using a views_theme_functions() which is in views.module, but that includes views.theme.inc and them calls _views_theme_functions() there. You always need a view to use the function anyway, so maybe it makes sense to move this into the ViewExecutable class.

With a quick unit test too.

CommentFileSizeAuthor
#2 2027031-2.patch10.78 KBdamiankloip
vdc.remove-views_theme_functions.patch10.39 KBdamiankloip

Comments

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/modules/views/lib/Drupal/views/ViewExecutable.phpundefined
@@ -2111,4 +2112,36 @@ public function mergeDefaults() {
+   * Provide a full array of possible themes to try for a given hook.
...
+  public function buildThemeFunctions($hook) {

Do we also return something ^^

+++ b/core/modules/views/lib/Drupal/views/ViewExecutable.phpundefined
@@ -2111,4 +2112,36 @@ public function mergeDefaults() {
+   * @param string|null $display
+   *   The display being rendered, if applicable.

This parameter seems to removed.

+++ b/core/modules/views/tests/Drupal/views/Tests/BuildThemeFunctionsTest.phpundefined
@@ -0,0 +1,90 @@
+class BuildThemeFunctionsTest extends UnitTestCase {

Unit Test all the things, but can we just have a ViewExecutable test?

damiankloip’s picture

damiankloip commented
StatusFileSize
new 2027031-2.patch10.78 KB

Nice, thank you!

I have changed the test to be called ViewExecutableUnitTest, as unfortunately we already have ViewExecutableTest, and simpletest module loads all namespaces, so we get a conflict!

damiankloip’s picture

damiankloip commented

Sorry, forgot the interdiff....oh well! :) It's a smallish patch.

dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +PHPUnit

Wow, did we really managed to implement two patterns for the test files?

core/tests looks like this:

core/tests/Drupal/Tests/Core/FooTest

core/modules/{module} looks like this:

core/modules/{module}/Drupal/{module}/Tests/FooTest

I guess we should switch to the second one everywhere?

damiankloip’s picture

damiankloip commented
Issue tags: -PHPUnit

Yeah, that's what I have been assuming. That we use the same dir structure in /tests/ as we do in /lib/. This is not an issue with this patch though?

dawehner’s picture

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

Let's get it in now.

alexpott’s picture

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

Committed fc9584d and pushed to 8.x. Thanks!

damiankloip’s picture

damiankloip commented
Status: Needs work » Fixed

Fixed? :)

Status: Fixed » Closed (fixed)

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

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Move views_theme_functions() to ViewExecutable method » [Change notice] Move views_theme_functions() to ViewExecutable method
Project: Drupal core » Views (for Drupal 7)
Version: 8.x-dev » 8.x-3.x-dev
Component: views.module » Code
Status: Closed (fixed) » Active
Issue tags: +Needs change record
chris matthews’s picture

chris matthews commented
Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.7.x-dev
Component: Code » views.module
Issue summary: View changes

For more information as to why this issue was moved to the Drupal core project, please see issue #3030347: Plan to clean process issue queue

chris matthews’s picture

chris matthews commented
Project: Drupal core » Views (for Drupal 7)
Version: 8.7.x-dev » 7.x-3.x-dev
Component: views.module » Code
Status: Active » Closed (outdated)

Moving back to the contributed Views issue queue and closing as outdated per https://www.drupal.org/project/views/issues/3030347#comment-13023447