Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[FrameworkBundle][Routing] added Configurators to handle template and redirect controllers#30501

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:masterfromHeahDude:framework-routing
Feb 9, 2020

Conversation

HeahDude
Copy link
Contributor

@HeahDudeHeahDude commentedMar 9, 2019•
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?let's see
Fixed ticketspartially#24640,#25145
LicenseMIT
Doc PRsymfony/symfony-docs#11120

While working onsymfony/symfony-docs#11085, I felt bad about the long notations required for simpleredirects andtemplates rendering template actions, but I love and use those features since always. Then I gave it a try yesterday night and now I realised I missed#24640 and that#25145 has been closed x).

So here we go, here's my WIP. WDYT of this implementation? ping@javiereguiluz?

I'm going to open the PR in the docs so we can discuss the DX changes there too, and keep focus on the code here.

Cheers!

EDIT

This PR now only update PHP-DSL configurators.

TODO:

GaryPEGEOT reacted with hooray emoji
@HeahDude
Copy link
ContributorAuthor

I'd really appreciate some help if anyone had already overridden some xsd schema thanks!

@HeahDude
Copy link
ContributorAuthor

I've updatedsymfony/symfony-docs#11120 with a nice before/after view.

Also I need to rebase on top of#30507.

Xml config is working properly, but the IDE cannot resolve validation/autocompletion yet, this works thanks to the loader using its constant local path.
I guess we need to create a new path onhttps://symfony.com/routing that we need to declare explicitly inconfig/routes.xml or any xml file where this feature should be used, are you the one handling this@fabpot? Can you help me on this?

Apart this last points, the feature is ready from side, only reviews needed now :), thanks!

@javiereguiluz
Copy link
Member

Thanks for working on this. To make this feature completely self-explanatory, I think we should renameredirect-to asredirect-to-route (we already haveredirect-to-url). What do you think?

HeahDude reacted with thumbs up emoji

fabpot added a commit that referenced this pull requestMar 11, 2019
This PR was merged into the 3.4 branch.Discussion----------[Routing] Fixed XML options resolution| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | ~| License       | MIT| Doc PR        | ~Found this bug while adding tests in#30501. I need it to be merged upward so it can get green there.Thanks!Commits-------53a6ff8 [Routing] Fixed XML options resolution
@HeahDudeHeahDude changed the title[FrameworkBundle][Routing] added a RoutingConfigurator to handle template and redirect[FrameworkBundle][Routing] added proper Loader namespace to handle controllersMar 13, 2019
fabpot added a commit that referenced this pull requestMar 20, 2019
… "format" in configuration (Jules Pietri)This PR was merged into the 4.3-dev branch.Discussion----------[Routing] Exposed "utf8" option, defaults "locale" and "format" in configuration| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | ~| License       | MIT| Doc PR        |symfony/symfony-docs#11126A sibling to#30501, everything is in the title :).Commits-------2911490 [Routing] Exposed "utf8" option, defaults "locale" and "format" in configuration
@HeahDude
Copy link
ContributorAuthor

Rebased and squashed here, while I addressed@javiereguiluz's comment.

Also, I was not happy with the xml definition because all attributes are there and it's hard to guess which one to use in which context, so I propose to use dedicated types instead:https://github.com/symfony/symfony/compare/a46c76a8c3a05adb7f984ea4d9cffbd3a446f3f5..7055ba0a5b09cec84d88f5c0447e79b65aa48a85#diff-96d406b9b38189be981bbea1bad334bd.

Tests are passing locally and on travis, but not on windows.. I wonder how this linehttps://github.com/symfony/symfony/pull/30501/files#diff-44f544990841fe196e4ea5c76dcc18dfR8 can be a problem, if anyone has a clue on that topic that would be awesome.

@nicolas-grekas
Copy link
Member

nicolas-grekas commentedDec 8, 2019•
edited
Loading

Did you consider this instead for the PHP-DSL?

$routes->add('template_route','/static')        ->template('static.html.twig')        ->maxAge(300)        ->sharedMaxAge(100)        ->private(true)        ->methods(['GET'])        ->options(['utf8' =>true])        ->condition('abc')    ;$routes->add('redirect_route','/redirect')        ->redirectToRoute('target_route')        ->permanent(true)        ->ignoreAttributes(['attr','ibutes'])        ->keepRequestMethod(true)        ->keepQueryParams(true)        ->schemes(['http'])        ->host('legacy')        ->options(['utf8' =>true])    ;$routes->add('url_redirect_route','/redirect-url')        ->redirectToUrl('/url-target')        ->permanent(true)        ->scheme('http')        ->httpPort(1)        ->httpsPort(2)        ->keepRequestMethod(true)        ->host('legacy')        ->options(['utf8' =>true])
HeahDude, bigfoot90, yceruto, and sstok reacted with heart emoji

@HeahDudeHeahDudeforce-pushed theframework-routing branch 2 times, most recently fromc67da45 todb3fdfdCompareDecember 10, 2019 21:34
@nicolas-grekas
Copy link
Member

Now that#34873 is merged, can you please rebase and take it into account to inject your new ContainerConfigurator?

@nicolas-grekas
Copy link
Member

Thank you@HeahDude.

HeahDude reacted with rocket emoji

nicolas-grekas added a commit that referenced this pull requestFeb 9, 2020
…le template and redirect controllers (HeahDude)This PR was merged into the 5.1-dev branch.Discussion----------[FrameworkBundle][Routing] added Configurators to handle template and redirect controllers| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | let's see| Fixed tickets | partially#24640,#25145| License       | MIT| Doc PR        |symfony/symfony-docs#11120While working onsymfony/symfony-docs#11085, I felt bad about the long notations required for simple [redirects](https://symfony.com/doc/current/routing/redirect_in_config.html) and [templates rendering](https://symfony.com/doc/current/templating/render_without_controller.html) template actions, but I love and use those features since always. Then I gave it a try yesterday night and now I realised I missed#24640 and that#25145 has been closed x).So here we go, here's my WIP. WDYT of this implementation? ping@javiereguiluz?I'm going to open the PR in the docs so we can discuss the DX changes there too, and keep focus on the code here.Cheers!EDIT----This PR now only update PHP-DSL configurators.______________TODO:- [x] gather reviews- ~[x] fix xml schema~- [x] add some tests- ~[ ] handle xsd auto discovery~- [x] rebase on top of#30507- [x] ~add shortcuts for#30514~Commits-------de74794 [FrameworkBundle][Routing] added Configurators to handle template and redirect controllers
@nicolas-grekasnicolas-grekas merged commitde74794 intosymfony:masterFeb 9, 2020
@HeahDudeHeahDude deleted the framework-routing branchFebruary 9, 2020 22:23
nicolas-grekas added a commit that referenced this pull requestFeb 15, 2020
…ude)This PR was merged into the 5.1-dev branch.Discussion----------[Routing] marked configurators traits as internal| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets       |Fix#30501 (comment) <!-- prefix each issue number with "Fix #", if any -->| License       | MIT| Doc PR        | not needed<!--Replace this notice by a short README for your feature/bugfix. This will help peopleunderstand your PR and can be used as a start for the documentation.Additionally (seehttps://symfony.com/releases): - Always add tests and ensure they pass. - Never break backward compatibility (seehttps://symfony.com/bc). - Bug fixes must be submitted against the lowest maintained branch where they apply   (lowest branches are regularly merged to upper ones so they get the fixes too.) - Features and deprecations must be submitted against branch master.-->Commits-------52efec7 [Routing] marked configurators traits as internal
nicolas-grekas added a commit to nicolas-grekas/symfony that referenced this pull requestApr 24, 2020
…urators to handle template and redirect controllers (HeahDude)"This reverts commit477ee19, reversingchanges made to9bfa258.
nicolas-grekas added a commit to nicolas-grekas/symfony that referenced this pull requestApr 24, 2020
…urators to handle template and redirect controllers (HeahDude)"This reverts commit477ee19, reversingchanges made to9bfa258.
fabpot added a commit that referenced this pull requestApr 24, 2020
…d Configurators to handle template and redirect controllers (HeahDude)" (nicolas-grekas)This PR was merged into the 5.1-dev branch.Discussion----------Revert "feature#30501 [FrameworkBundle][Routing] added Configurators to handle template and redirect controllers (HeahDude)"| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -This reverts commit477ee19, reversing changes made to9bfa258.The reverted PR is#30501. Actually, it's a partial revert: changes made to the `Routing` component are not reverted.The reason for this revert is that a discussion is still on-going in#35653 (which provides the same configuration extensions to yaml+xml), and we won't be able to resolve them in time for 5.1.Better postpone for 5.2.@HeahDude I invite you to open a PR after this one, reverting this very PR, which we'll consider again but for 5.2 if you don't mind.Commits-------e4e8945 Revert "feature#30501 [FrameworkBundle][Routing] added Configurators to handle template and redirect controllers (HeahDude)"
vjandrea pushed a commit to vjandrea/symfony that referenced this pull requestApr 25, 2020
…urators to handle template and redirect controllers (HeahDude)"This reverts commit477ee19, reversingchanges made to9bfa258.
@nicolas-grekasnicolas-grekas modified the milestones:next,5.1May 4, 2020
@fabpotfabpot mentioned this pull requestMay 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@TobionTobionTobion left review comments

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

5 participants
@HeahDude@javiereguiluz@nicolas-grekas@Tobion@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp