Problem/Motivation
It's become clear over the past 24 hours that FieldPropExpression and friends (everything in src/PropExpressions) is hard to grok for people.
Eventually we'll need docs, initially unit tests can serve as docs.
In this first pass, there is one key goal: unit tests to go from PHP object representation to string representation and vice versa should pass — because once that's working, contributors won't lose time anymore due to bugs in that low-level foundation. (Which currently still is 99% identical to the PoC from ~1.5 month ago.)
Out of scope: making all code in src/PropExpressions beautiful/elegant/DRY/… — the purpose is only the bold bit above, because that A) unblocks other issues/MRs, B) keeps scope tight.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
- API addition: single
StructuredDataPropExpression::from(…)method to avoid if/else logic to invoke the appropriate class-specific method.
Data model changes
- 🥳 No more mysterious invisible whitespace in the string representation. Result: less frustration, and the sort order in test expectations now actually makes sense.
- The string representation of
Field(Type)ObjectPropsExpressionno longer lists the mapped object props separated by,(comma + space), but only by,(comma).
Issue fork experience_builder-3451626
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
wim leersTo be continued on Monday.
Unless somebody wants to continue where I ended 🤓
Comment #4
wim leers#3452131: PHP 8.4 support landed upstream in #3427999, causing PHPCS CI job to fail fixed the
phpcsfailure that upstream caused.Comment #5
wim leersComment #6
wim leersComment #7
wim leersComment #8
wim leersComment #9
wim leersComment #11
wim leersMassive review from @larowlan on the MR — all addressed!
Comment #13
wim leers@f.mazeikis@gmail.com approved this after I addressed @larowlan's review 👍
Comment #15
wim leers