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
@dg dg force-pushed the master branch 2 times, most recently from a1e8c9b to df7c4c9 Compare January 21, 2024 23:50
@dg dg force-pushed the master branch 2 times, most recently from 891b6dd to e3d05b3 Compare February 2, 2024 17:51
@dg dg force-pushed the master branch 3 times, most recently from 426e735 to c19ebdc Compare March 11, 2024 20:02
@dg dg force-pushed the master branch 5 times, most recently from 2b9da37 to 30d90f4 Compare April 7, 2024 02:51
@dg dg force-pushed the master branch 6 times, most recently from bf86204 to c91f90a Compare April 20, 2024 00:46
@dg dg force-pushed the master branch 3 times, most recently from 57bd587 to e908315 Compare May 2, 2024 10:37
@dg dg force-pushed the master branch 8 times, most recently from c5ecbda to ecb200c Compare May 13, 2024 09:25
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.

None yet

2 participants