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

LinkGenerator: accept classname in $dest #296

Draft
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

mabar
Copy link
Contributor

@mabar mabar commented Sep 25, 2021

  • new feature
  • BC break? no

This is minimal change needed in Nette in order to support NOT using mapping and to reference class directly instead.

$this->linkGenerator->link(self::class . ':default');

To fully support that behavior also a class_exists() check is needed in PresenterFactory. Without this is my change untestable, so let me know if I should modify also PresenterFactory and write tests.
https://github.com/orisai/nette-application/blob/73906f0f3009f751c15d8170467246957ca2e239/src/Mapping/DefaultPresenterFactory.php#L147-L163

I am already able to achieve it with $this->presenter->link($this->presenter::class . ':default');.

Benefit of this change is presenters don't have to follow any namespace structure, just like any class.

@dg
Copy link
Member

dg commented Sep 26, 2021

There should be a condition that always makes it clear whether it is a class or a presenter, otherwise it could get awkward. For example, a class must contain at least one character \.

@mabar
Copy link
Contributor Author

mabar commented Sep 26, 2021

I wrote an regex which expects [[Class\Name:]action] [#fragment] and all the tests on regex101.com work (check the unit tests tab). But I cannot get it work in PHP, it does not match my test string App\Admin\User\List\UserListPresenter:default, not sure why :/
https://regex101.com/r/u2DneV/1

@mabar
Copy link
Contributor Author

mabar commented Sep 26, 2021

Okay, got it. Had to add some character classes. Seems like regex101 does not use php, just PCRE2 - regex with [\\] and \\ works with regex101, [\\\] works with php

@mabar mabar marked this pull request as draft September 28, 2021 15:59
@dg dg force-pushed the master branch 3 times, most recently from cb29fc4 to ef31716 Compare October 6, 2021 23:39
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.

2 participants