-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add resolutionHost
option
#3
base: master
Are you sure you want to change the base?
Conversation
With the resolutionHost option you can replace the complete hostname based on your content dimensions.
Hi @j6s ! Thanks for stumbling across my fork. to be honest it is not quite finished yet. You can not use the resolutionHost option in the |
update to current commit
Just a small hint that this PR does not work for Neos 5.x yet - Need to wait till neos/neos-development-collection#2803 get accepted. |
Did some testing and the resolutionHost value now works with all modes but still need to wait for the other PR in the Neos Core. |
Works with Neos 7 🎉 MarcoPNS@e5b05a7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes to the documentation need to be merged into the README.md
that exists since #12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and please undo the permission changes on the PHP files, those do not need to be executable.
"neos/neos": "~4.0||~5.0||~7.0||~8.0 || dev-master", | ||
"neos/flow": "~5.0||~6.0||~7.0||~8.0 || dev-master" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"neos/neos": "~4.0||~5.0||~7.0||~8.0 || dev-master", | |
"neos/flow": "~5.0||~6.0||~7.0||~8.0 || dev-master" | |
"neos/neos": "^7.0 || ^8.0 || dev-master", | |
"neos/flow": "^7.0 || ^8.0 || dev-master" |
@@ -482,6 +513,6 @@ protected function getRequestPathByNode(NodeInterface $node) | |||
$currentNode = $currentNode->getParent(); | |||
} | |||
|
|||
return implode('/', array_reverse($requestPathSegments)); | |||
return '/' . implode('/', array_reverse($requestPathSegments)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
@@ -422,6 +421,7 @@ protected function resolveRoutePathForNode(NodeInterface $node) | |||
* | |||
* @param NodeInterface $siteNode The site node, used as a starting point while traversing the tree | |||
* @param string $relativeRequestPath The request path, relative to the site's root path | |||
* @deprecated Use getNodeFromRequestPath() - return only the node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
@@ -409,8 +409,7 @@ protected function resolveRoutePathForNode(NodeInterface $node) | |||
$nodeContextPathSuffix = ($workspaceName !== 'live') ? substr($nodeContextPath, strpos($nodeContextPath, '@')) : ''; | |||
|
|||
$requestPath = $this->getRequestPathByNode($node); | |||
|
|||
return trim($requestPath, '/') . $nodeContextPathSuffix; | |||
return $requestPath . $nodeContextPathSuffix; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
$relativeNodePath = $this->getRelativeNodePathByUriPathSegmentProperties($siteNode, $requestPathWithoutContext); | ||
$node = ($relativeNodePath !== false) ? $siteNode->getNode($relativeNodePath) : null; | ||
$node = $this->getNodeFromRequestPath($siteNode, $requestPathWithoutContext) ?? null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will conflict with #11 I think. Just pointing this out already…
@kdambekalns It's been sooo long. I can no longer answer exactly why. I would first have to work my way back into it. |
I just looked at this again… @MarcoPNS if you still use this and have interest in it, I'd suggest we do this:
If you can't help, I suggest we close this. 🤷♂️ |
Hi! There are cases where the options are not strict enough. I had projects where the domains looked like this:
Each domain leads to a separate dimension. So the resolutionHost value was born to handle this type of case. The handling gets more complicated in case of multisites or if you want to mix the modes where you may have cases like this:
As the feedback here was a bit limited, we started to develop an in-house solution for this and wrote a closed source package. If this is something that is also interesting here, I could implement it. This would get rid of the resolutionMode because it's not needed anymore. Config could look like this:
|
This is a PR for a fork that is not mine! (I didn't know that was possible either)
I just happened to stumble upon this fork containing 2 of the things that I wanted:
resolutionHost
option that allows resolution based on the hostnamereadme.md
describing how the plugin works