Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

!!! FEATURE: Add function helpers to Eel and remove magic q #1580

Merged
merged 3 commits into from May 16, 2019

Conversation

mficzel
Copy link
Member

@mficzel mficzel commented May 15, 2019

Function helpers are static functions that are available in Eel without a containing helper. This
change removes the default q variable and instead adds a static method q to the flowQuery
class that is used as helper function with the following configuration:

Neos:
  Fusion:
    defaultContext:
      q: 'Neos\Eel\FlowQuery\FlowQuery::q'
  ContentRepository:
    labelGenerator:
      eel:
        defaultContext:
          q: Neos\Eel\FlowQuery\FlowQuery::q

Note: Nested pathes as identifiers for function-helpers are not allowed and will raise an exception.

This is breaking as it makes it necessary to add the configuration above to Neos.Fusion and Neos.ContentRepository. Also custom code that uses FlowQuery and relies on q beeing always present will have to be adjusted.

Upgrade Instructions:

Only if you are using the EelUtility in to evaluate expressions with q AND are using a custom defaultContextConfiguration you have to make sure that the configuration line q: 'Neos\Eel\FlowQuery\FlowQuery::q' is added to this configuration.

Neos.Eel/Classes/FlowQuery/FlowQuery.php Outdated Show resolved Hide resolved
Neos.Eel/Classes/Utility.php Outdated Show resolved Hide resolved
Neos.Eel/Classes/Utility.php Outdated Show resolved Hide resolved
Neos.Eel/Classes/Utility.php Outdated Show resolved Hide resolved
Neos.Eel/Classes/Utility.php Outdated Show resolved Hide resolved
@kdambekalns kdambekalns added this to In progress in Neos 5.0 & Flow 6.0 Release Board via automation May 15, 2019
@kdambekalns
Copy link
Member

I wonder when this would be breaking… If I am just a "regular Neos integrator", this would be transparent to me, no? I mean, q exists like before, just through a different mechanism. No?

@kdambekalns kdambekalns self-requested a review May 15, 2019 10:22
@mficzel
Copy link
Member Author

mficzel commented May 15, 2019

@kdambekalns q will be available as normal in fusion. It just has to be added to the fusion configuration in a separate pr. Currently cleaning this up

Function helpers are static functions that are available in Eel without a containing helper. This
change removes the default q variable and instead adds a static method `q` to the flowQuery
class that can be used as helper function with the following configuration:

```
Neos:
  Fusion:
    defaultContext:
      q: 'Neos\Eel\FlowQuery\FlowQuery::q'
  ContentRepository:
    labelGenerator:
      eel:
        defaultContext:
          q: Neos\Eel\FlowQuery\FlowQuery::q
```
@mficzel mficzel force-pushed the feature/functionHelpersForEel branch from e2ceb30 to a56e1d8 Compare May 15, 2019 12:24
@mficzel mficzel requested a review from bwaidelich May 15, 2019 12:28
@mficzel
Copy link
Member Author

mficzel commented May 15, 2019

The Neos side of this pr is here: neos/neos-development-collection#2505

@kdambekalns kdambekalns moved this from In progress to Needs review in Neos 5.0 & Flow 6.0 Release Board May 15, 2019
Copy link
Member

@albe albe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple comments, but looks good otherwise.

Neos.Eel/Classes/FlowQuery/FlowQuery.php Outdated Show resolved Hide resolved
Neos.Eel/Classes/Utility.php Outdated Show resolved Hide resolved
Neos.Eel/Classes/Utility.php Show resolved Hide resolved
Neos.Eel/Classes/Utility.php Outdated Show resolved Hide resolved
Co-Authored-By: Alexander Berl <a.berl@outlook.com>
Copy link
Member

@albe albe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good by reading.

Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of type and name in @param is reversed…

Neos.Eel/Classes/FlowQuery/FlowQuery.php Outdated Show resolved Hide resolved
Neos.Eel/Classes/Utility.php Outdated Show resolved Hide resolved
Co-Authored-By: Karsten Dambekalns <karsten@dambekalns.de>
@mficzel mficzel requested a review from kdambekalns May 16, 2019 11:08
@mficzel
Copy link
Member Author

mficzel commented May 16, 2019

@kdambekalns Adjusted as requested. Please reapprove so i can finally merge

@kdambekalns kdambekalns merged commit d2d0722 into neos:master May 16, 2019
Neos 5.0 & Flow 6.0 Release Board automation moved this from Needs review to Done May 16, 2019
@kdambekalns
Copy link
Member

Please reapprove so i can finally merge

As I said, if you enabled contributor edits, I'd have fixed it myself right away. ;)

@mficzel
Copy link
Member Author

mficzel commented May 16, 2019

@kdambekalns usually this is always on on my prs, no clue why not here. I probably wurstfingered it. Or github changed the default and i have to look for it explicitly now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants