Spin-off of #2329937: Allow definition objects to provide options:
The current AllowedValuesInterface has some terminology problems:
- It's not primarily about allowed values, but about both options and their associated values. As options include values, refering to options in the name makes sense.
- The interface is called AllowedValuesInterface, but an $object implementing the AllowedValuesInterfaces *is not* the allowed values - it can provide them to you.
- The term AllowedValues is only in the name, but not in any of its methods.
Rename AllowedValuesInterface to OptionsProviderInterface.
That plays well with the bigger plan over at #2329937: Allow definition objects to provide options as well.
Do it.
-
Rename of AllowedValuesInterface to OptionsProviderInterface ;)
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | rename-2332885-12-interdiff.txt | 295 bytes | berdir |
| #12 | rename-2332885-12.patch | 11.39 KB | berdir |
| #7 | rename-2332885-7.patch | 11.23 KB | cs_shadow |
| #5 | rename-2332885-5.patch | 12.49 KB | cs_shadow |
| #1 | d8_allowed_values_rename.patch | 12.86 KB | fago |
Comments
Comment #1
fagoComment #2
fagoComment #5
cs_shadow commentedComment #7
cs_shadow commentedComment #8
cs_shadow commentedLet's try again. Forgot to `git pull` last time.
Comment #12
berdirLooks like the file wasn't renamed...
Comment #13
catchI'm not 100% on this for a couple of reasons:
1. The field settings key is still allowed_values
2. In the case of entity reference fields, lots of widgets don't actually use options, and the method just gets used for validation (and for something that taxonomy terms where we only want to check which vocabulary a term is in most of the time, it's really inefficient to check it that way).
3. There's a bit of a weird situation where you have to implement this interface to be able to use certain widgets/formatters, but that doesn't necessarily have anything to do with the field type itself. So a list field that is text or number has exactly the same schema as a non-list field that is text or number - the only difference is this interface and the allowed values setting. The fact the allowed_values setting does double duty for both options and validation feels like a limitation more than a feature.
Comment #14
yched commentedRe #13: note that "allowed_values" is only how List* field types chose to formulate their notion of "AllowedValuesInterface / OptionsProvider", the is no generic meaning for "allowed_values" (taxo does too, for D7 reasons).
The one generic feature here is AllowedValuesInterface / OptionsProviderInterface", which is intended to be the interface that makes the generic widgets and formatters work independantly of the field type - well, and for validation too.
I think that the move from "XxxItem implements AllowedValuesInterface" to "The FieldDefinition specifies an OptionsProvider" (as #2329937: Allow definition objects to provide options is doing IIRC) is a good move. So if we think "options" is too UI tainted and does not reflect the use for validation, maybe AllowedValuesProvider ?
Comment #15
catchAllowedValuesProvider would work for me.
Comment #16
fagoyeah, there is allowed_values also, but I'd argue that it's not OptionsProvider what is mis-named as the setting actually contains not only the allowed values, but the allowed values + their labels, i.e. the options. Thus renaming the setting might make sense, but as this is something internal to the field-type implementation it seems like being a separate issue to me (and I do not care much as its internal).
yes, but it's actually options who do the double-duty. They provide options + allowed values. Allowed-values do not necessarily include labels, so for having both features you need options imo.
Comment #18
catchDiscussed with fago in irc and he's going to open an issue to look at list fields (and we probably need one for taxonomy too to stop using options for validation regardless of the formatter - that's an easy OOM).
Back to RTBC.
Comment #19
alexpottCommitted f61ad20 and pushed to 8.0.x. Thanks!
Comment #21
fagoWrote down issues for the points discussed with catch, feedback wanted: