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] Support Vendor.Package as namespace name #329

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

NamelessCoder
Copy link
Member

@NamelessCoder NamelessCoder commented Jul 30, 2017

Using this format (or simply Package if no vendor name is
used for the package) makes ViewHelperResolver look in
that namespace + ViewHelpers, for example:

<TYPO3Fluid.Fluid:render partial="Partial" />

Will internally resolve to:

\TYPO3Fluid\Fluid\ViewHelpers\RenderViewHelper

Without the need to register the namespace. Note that the
example uses the native Fluid namespace to demonstrate,
although this namespace is always present (unless explicitly
removed by a an override).

@NamelessCoder
Copy link
Member Author

Needs #328 before tests will succeed. Will need to be conflict resolved and rebased.

@lolli42
Copy link
Member

lolli42 commented Aug 25, 2017

Like the patch, tried to rebase it, but didn't succeed, got quite some unit test failures after a straight conflict resolving. Could you have a look at this, Claus?

@NamelessCoder NamelessCoder force-pushed the directnamespace branch 2 times, most recently from f4d061b to ea20c2f Compare August 27, 2017 14:03
@NamelessCoder
Copy link
Member Author

Thanks for reviewing / testing, Christian :)

This last force-push should fix the issue. The error messages were slightly different and there was a PHP 5.x problem as well.

Using this format (or simply Package if no vendor name is
used for the package) makes ViewHelperResolver look in
that namespace + ViewHelpers, for example:

```xml
<TYPO3Fluid.Fluid:render partial="Partial" />
```

Will internally resolve to:

```php
\TYPO3Fluid\Fluid\ViewHelpers\RenderViewHelper
```

Without the need to register the namespace. Note that the
example uses the native Fluid namespace to demonstrate,
although this namespace is always present (unless explicitly
removed by a an override).
@NamelessCoder
Copy link
Member Author

Fixed a minor commit message problem (TemplatePaths was mentioned as the place to change ViewHelper namespaces, I guess I was thinking about something else while writing that).

Copy link
Member

@mbrodala mbrodala left a comment

Choose a reason for hiding this comment

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

Looks and works great, thanks a lot for finally bringing this to Fluid. 👍

Copy link
Contributor

@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. One question: Is it intended that the resolving doesn't ucfirst the namespace parts?

@mbrodala
Copy link
Member

I'd say yes since there is no rule that a namespace must use UpperCamelCase, see for example mikey179/vfsStream which uses org\bovigo\vfs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants