Sorry, another moving things issue.

This is only used in Drupal\field\Plugin\views\field\Field. So we can move this to a method on that class. I have removed the static caching and replaced it with a property on the class.

Does this method need to be updated/replaced with something else from the field api?

Files: 
CommentFileSizeAuthor
#26 vdc-1912504-26.patch3.95 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,117 pass(es).
[ View ]
#20 1912504-20.patch887 bytesamateescu
PASSED: [[SimpleTest]]: [MySQL] 55,951 pass(es).
[ View ]
#20 interdiff.txt598 bytesamateescu
#18 1912504-18.patch606 bytesdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#15 1912504-15.patch606 bytesdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1912504-15.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#15 1912504-15-no-return.patch567 bytesdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Setup environment: failed to create checkout database.
[ View ]
#10 1912504-10.patch3.36 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,837 pass(es).
[ View ]
#10 interdiff-1912504-10.txt762 bytesdamiankloip
#8 1912504-8.patch3.3 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,891 pass(es).
[ View ]
#4 drupal-1912504-4.patch3.3 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1912504-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
vdc._field_view_formatter_options.patch3.27 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 49,112 pass(es), 501 fail(s), and 253 exception(s).
[ View ]

Comments

dawehner’s picture

+++ b/core/modules/views/views.moduleundefined
@@ -1739,33 +1739,6 @@ function views_handler_field_custom_pre_render_move_text($form) {
-function _field_view_formatter_options($field_type = NULL) {

To be honest I never understood why we moved that into the views.module file ... let's figure that out first (in d7 this has been part of the field handler).

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.phpundefined
@@ -861,4 +868,35 @@ function field_langcode(EntityInterface $entity) {
+   * Borrowed from field_ui.

So if it's borrowed from field_ui we should put a public API in field_ui, as it's helpful for two different places in core already.

damiankloip’s picture

So atm in D8, this function now exists as field_ui_formatter_options in field_ui.admin.inc.

tbh, I'm not sure why we need to copy it? We have enough code to maintain ;) Shall we just switch to including this file and using this function instead for now? hmmm

Status:Needs review» Needs work

The last submitted patch, vdc._field_view_formatter_options.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new3.3 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1912504-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolled.

damiankloip’s picture

Issue tags:-VDC

#4: drupal-1912504-4.patch queued for re-testing.

damiankloip’s picture

#4: drupal-1912504-4.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+VDC

The last submitted patch, drupal-1912504-4.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new3.3 KB
PASSED: [[SimpleTest]]: [MySQL] 55,891 pass(es).
[ View ]

rerolled.

dawehner’s picture

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.phpundefined
@@ -846,4 +853,35 @@ function field_langcode(EntityInterface $entity) {
+   * Borrowed from field_ui.

I'm wondering whether we can replace that by a proper @see

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.phpundefined
@@ -846,4 +853,35 @@ function field_langcode(EntityInterface $entity) {
+   * @param string

Cool a parameter, what is going on.

damiankloip’s picture

StatusFileSize
new762 bytes
new3.36 KB
PASSED: [[SimpleTest]]: [MySQL] 55,837 pass(es).
[ View ]

Let's fix up the docblock.

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

Perfect!

alexpott’s picture

Committed f6b5925 and pushed to 8.x. Thanks!

damiankloip’s picture

Status:Reviewed & tested by the community» Fixed
tstoeckler’s picture

Status:Fixed» Needs work
+    return $options;

Sorry in case I'm just being stupid, but I don't see $options being defined in that function at all. That should be

<?php
return $this->formatterOptions;
?>

I guess.

damiankloip’s picture

Assigned:Unassigned» alexpott
Status:Needs work» Needs review
StatusFileSize
new567 bytes
FAILED: [[SimpleTest]]: [MySQL] Setup environment: failed to create checkout database.
[ View ]
new606 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1912504-15.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Yeah, you are totally right. Although in views we will never need that return value, as we won't get all field options. So Shall we just fix it or remove it? @alexpott, I leave it up to you.

Status:Needs review» Needs work

The last submitted patch, 1912504-15-no-return.patch, failed testing.

tstoeckler’s picture

I don't really have an opinion which patch should go in, but the "no-return" one, if we decide to go with it, should also make $field_type a required argument and remove the condition for it.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new606 bytes
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

I think we might as well just return formatterOptions, then this works the same as the field api, even though we don't actually use all field info.

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

I would always go with the return one, because people might expect that behavior.

amateescu’s picture

StatusFileSize
new598 bytes
new887 bytes
PASSED: [[SimpleTest]]: [MySQL] 55,951 pass(es).
[ View ]

There's a patch at #1982138: Clean out field_ui.admin.inc that provides an identical helper in the formatter plugin manager, where it belongs. Can we please add a @todo to remove this duplicated one?

dawehner’s picture

Sure.

Status:Reviewed & tested by the community» Needs work
Issue tags:-VDC

The last submitted patch, 1912504-20.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
Issue tags:+VDC

#20: 1912504-20.patch queued for re-testing.

Doesn't look like it actually failed.

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

ReRTBC

alexpott’s picture

Status:Reviewed & tested by the community» Needs work

#1982138: Clean out field_ui.admin.inc has landed so lets do the removal here...

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new3.95 KB
PASSED: [[SimpleTest]]: [MySQL] 55,117 pass(es).
[ View ]

Manual testing still worked.

damiankloip’s picture

Status:Needs review» Reviewed & tested by the community

Nice, looks good. Plus there is already test coverage for this in the UI. I like removing code we don't really need. This was originally here just so we didn't have to load the field_ui.admin.inc file.

amateescu’s picture

Title:move _field_view_formatter_options to Field.php» Remove _field_view_formatter_options() from Views as it duplicates FormatterPluginManager::getOptions()

This was originally here just so we didn't have to load the field_ui.admin.inc file.

If this is really true, I'm speechless..

Anyway, updated the title to tell what's really going on in the latest patch.

damiankloip’s picture

Or something like that anyway..

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 8cc0c63 and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

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

xjm’s picture

Title:Remove _field_view_formatter_options() from Views as it duplicates FormatterPluginManager::getOptions()» [Change notice] Remove _field_view_formatter_options() from Views as it duplicates FormatterPluginManager::getOptions()
Project:Drupal core» Views
Version:8.x-dev» 8.x-3.x-dev
Component:views.module» Code
Assigned:alexpott» Unassigned
Status:Closed (fixed)» Active
Issue tags:+Needs change record