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?

Support from Acquia helps fund testing for Drupal Acquia logo

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
FileSize
3.3 KB

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
FileSize
3.3 KB

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

FileSize
762 bytes
3.36 KB

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

return $this->formatterOptions;

I guess.

damiankloip’s picture

Assigned: Unassigned » alexpott
Status: Needs work » Needs review
FileSize
567 bytes
606 bytes

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
FileSize
606 bytes

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

FileSize
598 bytes
887 bytes

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
FileSize
3.95 KB

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 (for Drupal 7)
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
Chris Matthews’s picture

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

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