The current schema organization lets each widget/formatter plugin completely override the schema for $display->content[$field_name] (which is a keyed array with weight, plugin id, label and settings).
This is wrong and not in-line with the actual data model : the widget/formatter only gets to say what's inside 'settings', what's in the level above is not its decision.
This is also not consistent with the way field type plugins express the schema for their settings - field.[field_type].settings is *only* the mapping of settings.
API changes
This will force widget / formatter plugins to update the config schema for their settings.
Comment | File | Size | Author |
---|---|---|---|
#19 | interdiff.txt | 17.62 KB | Gábor Hojtsy |
#19 | 2378055-refactor-entity-view-form-field-schemas-19.patch | 32 KB | Gábor Hojtsy |
#16 | interdiff.txt | 17.13 KB | Gábor Hojtsy |
#16 | 2378055-refactor-entity-view-form-field-schemas-16.patch | 31.5 KB | Gábor Hojtsy |
#15 | interdiff.txt | 1.35 KB | Gábor Hojtsy |
Comments
Comment #1
catchThis looks like a bug rather than a task to me. I think it's OK to make the change at least until schemas settle down more generally on impact > disruption.
Comment #2
yched CreditAttribution: yched commentedadding tags
Comment #3
Gábor HojtsyThere you go. @yched this is what you had in mind? We do get rid of loooooots of cruft in the schema also.
Comment #5
Gábor HojtsyThis fails on
I don't see how this is related and cannot reproduce locally. Will work on fixing the other fails by attempting to reproduce them locally but does not sound very effective. #2370305: Refactor field type configuration schemas for DX, easier to find errors also fails on the same thing but is a totally different patch...
Comment #6
yched CreditAttribution: yched commentedWay cool, yes, exactly !!
Well, if those fails can be solved :-)
Comment #8
alexpottFor the unrelated test fails... #2376039: Undefined property ContainerAwareEventDispatcherTest::results in run-tests.sh
Comment #10
yched CreditAttribution: yched commentedSo, the overall structure and mounting points are perfect.
My only nitpick is about schema entry names :
What #2370305: Refactor field type configuration schemas for DX, easier to find errors does is :
"field type plugins provide the config schema for their settings at field.(field|storage)_settings.[%field_type]"
So for consistency, we should do something like
"widget/formatter plugins provide the config schema for their settings at (widget|formatter).settings.[%plugin_id]"
(instead of entity_view_display.field.[%plugin_id] currently)
Rationale : When I'm writing a widget/formatter, I shouldn't have to know/care about about the concept of "entity view/form display", I'm just writing a widget, and i want to describe its settings.
It's only incidental that EVD/EFD are the primary ConfigEntities enclosing widget/formatter configurations, but they're not the only ones, Views contain configured formatters too...
Comment #15
Gábor HojtsyIf I use relative dynamic type referencing right, that should fix the fails :) Also noticed two labels missing.
Comment #16
Gábor HojtsyAlso I am fine with the widget.settings.* and formatter.settings.* naming, made those changes too. Any other concerns?
Comment #17
Gábor HojtsyNote that for naming things internal to schemas, we should keep in mind that this is the same namespace that is for modules. So if there is a module named formatter or widget, those will share the namespace with these settings. I don't think we have any guidelines for namespacing component schemas where they don't belong to a config key though, so no good guideline to suggest.
Comment #18
yched CreditAttribution: yched commenteds/formatter/widget :-) (twice, actually)
re #17 namespacing: true - event though the current sub-schemas for field.(settings|value).[field_type] are in fact not strictly related to field.module (because of Core's BaseFieldOverrides) - they are not defined in field.schema.yml, for that matter.
Would
field.widget_settings.[widget_type]
field.formatter_settings.[formatter_type]
be better ?
Namespacing is better (with the same "not really field.module actually" restriction), drawback is that it's not really the same "family" than the ones for the field type:
field.field_settings.[field_type]
field.storage_settings.[field_type]
field.value.[field_type]
Or maybe we can use dots to more clearly separate from the above ?
field.widget.settings.[widget_type]
field.formatter.settings.[formatter_type]
Comment #19
Gábor HojtsyRenamed to field.widget.settings and field.formatter.settings so we are good with the namespacing. Also made the textual fix you suggested (and fixed other occurrences of formatter to widget around the same region as appropriate). As far as I see, we are done here and should get this in? :)
Comment #20
yched CreditAttribution: yched commentedAwesome. Thanks @Gábor !
Comment #21
alexpottThis issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed dd1350f and pushed to 8.0.x. Thanks!