Problem/Motivation
Process plugin documentation is being moved from the Handbook to the API documentation of the process plugin class.
The handbook page for process plugin contains more information than the API documentation of the plugin.
Before we delete the handbook page and redirect it to the API page, we need to either enhance the API doc or decide that it is OK to discard the additional information currently found in the handbook.
The difference is this:
--clips--
YAML Booleans
Using an unquoted string which represents a boolean value in YAML (various forms of yes/no/true/false/on/off) as a key in your map will use that boolean value rather than the string itself as your key, which is almost certainly not what you want. Be sure to quote any such strings to use them as strings.
Doesn't work:
process:
bar:
plugin: static_map
source: foo
map:
# This works as if it were '1: to'.
TRUE: to
Does work:
process:
bar:
plugin: static_map
source: foo
map:
'TRUE': to
Periods in key names
Mapping from a string which contains a period does not work at all:
process:
bar:
plugin: static_map
source: foo
map:
'Version 3.0': version_3_0
This will give error:
exception 'Drupal\Core\Config\ConfigValueException' with message 'Version 3.0 key contains a dot which is not supported.' in /var/www/example/docroot/core/lib/Drupal/Core/Config/ConfigBase.php:211
There is an existing issue #2827897: static_map process plugin does not work with periods in keys to address this. In the meantime, a custom process plugin would be needed to map source data containing periods if they could not be removed by a previous process plugin like machine_name (which will run the value over the transliteration service, lowercases it, replaces anything that's not a number or a letter with an underscore and removes duplicate underscores):
process:
bar:
-
plugin: machine_name
source: foo
-
plugin: static_map
map:
'version_3_0': version_3_0
When writing the patch to include the additional documentation listed above, it was identified that there were several other areas needing improvements:
- API documentation standards: how default values are indicated.
- API documentation standards: use single quotes instead of double quotes.
- It is very confusing which descriptive text belongs to which example.
- There are many places where the phrasing can be clarified.
Proposed resolution
- Include the 'YAML booleans' and 'periods in key names' topics in the API documentation
- Fix API documentation standard violations.
- Harmonize the descriptions so that the descriptive text is always above the code example.
- Clarify the phrasing where possible
Remaining tasks
Do it.
User interface changes
N/A.
API changes
N/A.
Data model changes
N/A.
Comment | File | Size | Author |
---|---|---|---|
#8 | interdiff-2933770-5-8.txt | 1.01 KB | masipila |
#8 | 2933770-8.patch | 5.07 KB | masipila |
#5 | interdiff-2933770-3-5.txt | 3.73 KB | masipila |
#5 | 2933770-5.patch | 5.07 KB | masipila |
#3 | 2933770-3.patch | 4.64 KB | masipila |
Comments
Comment #2
masipila CreditAttribution: masipila as a volunteer commentedComment #3
masipila CreditAttribution: masipila as a volunteer commentedInitial patch attached. There were also multiple other issues which I fixed at the same time, see issue summary updates for details.
Comment #4
quietone CreditAttribution: quietone as a volunteer commenteds/exmaple/example/
s/Default value/The default value/
Seems a bit long winded to get to the information. Can we make the first sentence a 'how to', i.e. 'How to map boolean values'.
I don't think an incorrect example is really useful here. Let's get rid of it and just show how to do it correctly.
I like the simplicity of this but it reads like it is a bug and makes me want to know why. I don't have a suggestion on how to improve it though.
Does this need an empty line before it?
Please check the indentation of "map:", it looks off in several of the existing examples.
Comment #5
masipila CreditAttribution: masipila as a volunteer commented1. Fixed.
2. Fixed.
3-4. Fixed.
5. I rephrased this from 'not possible' to 'not supported'. The associated issue is linked with @see. My opinion is that we should include this in the API documentation as this is potentially a total WTF and therefore we should warn the developer about this. If we choose to include this remark in the API doc, then I will drop a note to the linked issue so that we will remember to remove the disclaimer when that issue will be fixed.
6. I'm not 100% sure. The API doc standards say this: Each @see reference is on its own line, with no additional text beyond the item being referenced. To provide more explanation, just use a regular documentation paragraph.
Markus
Comment #6
phenaproximaCouple of minor things, but mostly looks good to me.
Typo in "default". Also, it should be "A default value", not "The default value".
Nit: There should be an extra blank line before the first @see.
Comment #7
masipila CreditAttribution: masipila as a volunteer commentedComment #8
masipila CreditAttribution: masipila as a volunteer commented1-2: Done.
Comment #9
quietone CreditAttribution: quietone as a volunteer commentedThe change to 'not supported' reads a whole lot better, thanks. I installed the patch and read through the whole doc. Everything in #4 and #7 has been addressed.
Comment #10
larowlan8.4 is criticals only now
Comment #11
quietone CreditAttribution: quietone as a volunteer commented@larowlan, why the jump to 8.6.x?
Comment #12
larowlanAdding review credits
Comment #13
larowlanCommitted as 45662bb and pushed to 8.6.x.
Cherry-picked as eb9b03e and pushed to 8.5.x.
@quietone see https://groups.drupal.org/node/518148
Thanks folks